-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add report fields to QBO/Xero #51321
Conversation
@aimane-chnaif Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
@aimane-chnaif I've shared with you the design doc in case you need more context. |
@allgandalf could you please retest? Video for bug #1 fix: Screen.Recording.2024-10-24.at.14.16.27.movVideo for bug #2 fix: Screen.Recording.2024-10-24.at.14.17.29.mov |
Bug, we do not disable the option to click on the list page for QBO locations: Screen.Recording.2024-10-24.at.5.50.14.PM.mov |
Also test is failing |
Not a blocker but if you change export as from credit card to debit card and go to the list page there we still see as Screen.Recording.2024-10-24.at.6.11.13.PM.mov |
Lint failure :/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going out for dinner, this tests well now, let's hope the pending reassurance test passes.
We are good to merge this 👍 , I verified the following according to the testing doc:
Manual Test
Pre-testing steps
- Make sure that your test account is on the accountingOnNewExpensify beta (Doesn't require beta to test)
- Make sure you have QBO and Xero credentials to initiate a connection
- Make sure you create a new workspace/policy for each test run
- Make sure emails sent to the testing email addresses go to an inbox you can access to validate your account(s) (Internal)
QuickBooks Online
Classes
-
Verify that the
Displayed As
option appears below the toggle. -
Verify that a new page opens with options to select
Tags
orReport Fields
. -
Verify that the selection is saved, and you're navigated back to the Classes configuration page.
Customers/Projects
-
Verify that the
Displayed As
option appears below the toggle. -
Verify that options to select
Tags
orReport Fields
are displayed. -
Verify that the selection is saved, and you return to the configuration page.
Locations
-
Verify there’s no explanatory subtext under
Import
menu item -
Verify that the
Displayed As
option appears below the toggle and there’s no right icon. -
Verify there is an explanatory subtext like here
-
Verify that you see
Check
,Journal entry
,Vendor bill
as the available options. -
Verify you don’t see the explanatory subtext anymore and that you see the right icon for `Displayed as
-
Verify that options to select
Tags
orReport Fields
are displayed. -
Verify that the selection is saved, and you return to the configuration page.
-
Verify that the
Report Fields
option has been correctly persisted after refreshing the page -
Verify that you only see
Journal entry
, as the available option.
Xero
Tracking Categories
- Verify that the
Displayed As
option appears below the toggle. - Verify that options to select
Tags
orReport Fields
are displayed. - Verify that the selection is saved, and you return to the configuration page.
Thank you @allgandalf 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Reviewer Checklist
Screenshots/VideosAndroid: NativeAndroid: mWeb ChromeiOS: NativeiOS: mWeb SafariMacOS: Chrome / SafariMacOS: Desktop |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪 |
🚀 Deployed to staging by https://github.com/lakchote in version: 9.0.55-0 🚀
|
🚀 Deployed to production by https://github.com/Beamanator in version: 9.0.55-10 🚀
|
const selectDisplayedAs = useCallback( | ||
(row: CardListItem) => { | ||
if (row.value !== qboConfig?.syncCustomers) { | ||
QuickbooksOnline.updateQuickbooksOnlineSyncCustomers(policyID, row.value, qboConfig?.syncCustomers); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since Report Fields are a Control feature, we should be showing the Report Fields upgrade RHN. This was caught by #52397
Details
We need to add the report fields option for QBO/Xero to close our product gap between OldDot and NewDot.
Fixed Issues
$ #51320
Tests
Did the tests outlined in the document here
QBO
qbo.mov
Xero
xero.mov
Offline tests
Offline tests are outlined in the document in the section above.
QA Steps
Same as in tests
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps./** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop