[HOLD for Payment 2024-09-23][$250] Update UpdatePolicyConnectionConfiguration to be 1:1:1 - Part 1

[dangrous]

Problem

Our UpdatePolicyConnectionConfiguration API command is not 1:1:1 - it is the same API call for all related user actions, when it should be split up for each connection (QBO, Xero, etc.) and configuration setting (autoSyncVendor, enableNewCategories, etc.). At the end of this we should not call that function anywhere.

This is polish for the Collect QBO project.

Solution

When we created the NetSuite commands, we followed the correct pattern, and so need to do so here too.

This requires backend and frontend work in tandem (to create the commands and then to use them), so we're looking for a contributor(s) to handle the frontend. You can see an example PR here - there is some extra prep work there that you can ignore, but adjusting all calls like this is what we're going for:

- Connections.updatePolicyConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.QBO, CONST.QUICK_BOOKS_CONFIG.REIMBURSABLE_EXPENSES_ACCOUNT, row.value);
+ QuickbooksOnline.updateQuickbooksOnlineReimbursableExpensesAccount(policyID, row.value);

Here is a list of the commands handled in this issue:

• UpdateXeroImportCustomers
• UpdateXeroEnableNewCategories
• UpdateXeroExport
• UpdateXeroSync
• UpdateXeroAutoSync

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01df4dbeb24127ed89
• Upwork Job ID: 1824524962582076644
• Last Price Increase: 2024-08-16
• Automatic offers:
• ishpaul777 | Reviewer | 103558840

Issue Owner

Current Issue Owner: @

Issue Owner

Current Issue Owner: @VictoriaExpensify

[melvin-bot]

Triggered auto assignment to @VictoriaExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[VictoriaExpensify]

Hey @dangrous - I notice that I got assigned to this with the bug label, but then you removed the bug label. So I'm not sure if I'm meant to be assigned or what I should be doing with this...?

[dangrous]

Ha yeah sorry I was trying to remove it before Melvin noticed, but I failed.

Nothing to do at the moment - we're going to be assigning this directly to a C+, who will eventually need payment, so might as well stick around... but no action right now!

[mananjadhav]

Picking this up.

[ishpaul777]

Picking this up as C+

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01df4dbeb24127ed89

[melvin-bot]

Current assignee @ishpaul777 is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

📣 @ishpaul777 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[dangrous]

oh hm it didn't register @mananjadhav . @ishpaul777 can you do the 🎀 thing? Maybe it needs that.

[ishpaul777]

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @johnmlee101, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[ishpaul777]

i think thats becuase Manan got paid through New Dot ?

[dangrous]

oh maybe? that makes it easier if so. In any case, @mananjadhav you're assigned!

[dangrous]

FYI it looks like we send export and sync with different setting values as a full object so we'll need to split those up as well - like:

• UpdateXeroExportBillDate
• UpdateXeroExportExporter
• UpdateXeroExportBillStatus
• UpdateXeroSyncInvoiceCollectionsAccountID
• UpdateXeroSyncReimbursementAccountID

etc.

And another note, for Xero we are using updatePolicyXeroConnectionConfig which eventually calls UpdatePolicyConnectionConfiguration

[mananjadhav]

Okay let me know once we have these API on staging I'll start with the PR.

[dangrous]

Backend is merged, not yet deployed, but you can probably start working and it'll be up by the time testing is ready. Here are the relevant new commands, just to confirm:

• UpdateXeroImportCustomers
• UpdateXeroEnableNewCategories
• UpdateXeroAutoSync

Export commands:

• UpdateXeroExportBillStatus
• UpdateXeroExportBillDate
• UpdateXeroExportExporter
• UpdateXeroExportNonReimbursableAccount

Sync commands:

• UpdateXeroSyncInvoiceCollectionsAccountID
• UpdateXeroSyncSyncReimbursedReports
• UpdateXeroSyncReimbursementAccountID

I grouped the sync and export ones that will need to be split up

[dangrous]

Spoke too soon, I guess - this is on staging now, so code away!

[dangrous]

@mananjadhav let us know how things are going!

[mananjadhav]

I should have the PR by tomorrow.

[mananjadhav]

I will raise the draft PR in a few hours.

[mananjadhav]

I had some system issues and now I am on a new system. I'll be redoing some of the work, so will raise a PR by end of day today.

[mananjadhav]

quick question @dangrous, I can see we have added prepareXeroOptimisticData which is specific to xero.config.

App/src/libs/actions/connections/Xero.ts

Lines 65 to 70 in 6d00670

	function prepareXeroOptimisticData<TSettingName extends keyof Connections['xero']['config']>(
	policyID: string,
	settingName: TSettingName,
	settingValue: Partial<Connections['xero']['config'][TSettingName]>,
	oldSettingValue?: Partial<Connections['xero']['config'][TSettingName]> | null,
	) {

Now for my commands UpdateXeroExportBillStatus, UpdateXeroExportBillDate, etc. are the keys from xero.config.export, which makes it unusable as the keys are nested. Do we create a separate method for prepareXeroExportOptimisticData or should I just duplicate the code in each method to set the keys.

In turn we're also calling createXeroPendingFields and createXeroErrorFields which would have to be defined for export, advanced etc.

[dangrous]

I think prepareXeroExportOptimisticData makes sense since we're going to be using it a couple times!

[mananjadhav]

Thanks for confirming. We'll need to do the same for sync and the error/pending fields methods.

[melvin-bot]

@mananjadhav, @dangrous, @VictoriaExpensify, @ishpaul777 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[ishpaul777]

Update: @mananjadhav has a draft PR which is almost 90% ready for review just pending checklist and some conflicts, i'll review it over the weekend

[mananjadhav]

@dangrous I am getting an issue while executing UpdateXeroSyncSyncReimbursedReports. It returns false as string and the switch resets. Can you please check this at your end? The other commands are working fine.

web-sync-reimbursed-reports.mov

[dangrous]

Sorry for the delay on this @ishpaul777 ! I've found the issue and am raising a BE PR to fix momentarily (https://github.com/Expensify/Web-Expensify/pull/43460)

[dangrous]

@mananjadhav that issue should be fixed on staging!

[aldo-expensify]

Is there anything else left to do here?

[ishpaul777]

This is on staging once deployed to prod then hold for 7 day regression period

[dangrous]

This was deployed to prod on 9/16. Stupid automation.

[VictoriaExpensify]

It looks like this is ready for payment then 🤑

[VictoriaExpensify]

Payment summary:
Contributor: @mananjadhav owed $250 via NewDot
Contributor+: @ishpaul777 paid $250 via Upwork

[garrettmknight]

$250 approved for @mananjadhav