Closed IuliiaHerets closed 6 days ago
Triggered auto assignment to @abekkala (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.
@abekkala FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors
We think that this bug might be related to #wave-control
Import spreadsheet option is available when workspace is connected to QBO
We are not adding any condition here to check this https://github.com/Expensify/App/blob/83865b64cca258299e091cc1b5b16ac12c92e089/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx#L302-L312
We can conditionally push this item only if !hasAccountingConnection https://github.com/Expensify/App/blob/83865b64cca258299e091cc1b5b16ac12c92e089/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx#L302-L312
We always show the import sheet option without checking we connect an accounting or not
We should use the same way we display the add category button
const menuItems = [];
if (!PolicyUtils.hasAccountingConnections(policy)) {
menuItems.push({
icon: Expensicons.Table,
text: translate('spreadsheet.importSpreadsheet'),
onSelected: () => {
if (isOffline) {
Modal.close(() => setIsOfflineModalVisible(true));
return;
}
Navigation.navigate(ROUTES.WORKSPACE_CATEGORIES_IMPORT.getRoute(policyId));
},
})
}
menuItems.push({
icon: Expensicons.Download,
text: translate('spreadsheet.downloadCSV'),
onSelected: () => {
if (isOffline) {
Modal.close(() => setIsOfflineModalVisible(true));
return;
}
Category.downloadCategoriesCSV(policyId);
},
});
If we already connected an accounting, we also need to display the not found page in ImportCategoriesPage
and ImportedCategoriesPage
page. We can display a helpful message that help user know that they already connected an accounting and cannot import category
Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.
Job added to Upwork: https://www.upwork.com/jobs/~021834239004653240865
Triggered auto assignment to Contributor-plus team member for initial proposal review - @thesahindia (External
)
@Nodebrute's proposal was first so I will prefer them!
🎀 👀 🎀 C+ reviewed
Triggered auto assignment to @chiragsalian, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
@thesahindia I think we also need to display the not found page in ImportCategoriesPage
and ImportedCategoriesPage
to prevent users from going to this page by deep link when we already connect an accounting.
Yeah i believe that is correct @nkdengineer. @thesahindia, what are your thoughts?
@abekkala, @chiragsalian, @thesahindia Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
Yeah i believe that is correct @nkdengineer. @thesahindia, what are your thoughts?
Yes, it is correct, but I wasn’t sure if I should consider it for deciding the assignment here, as these things should be addressed in the PR.
I am comfortable with either of them being assigned.
I think this is the thing that we can miss in the PR if we don't address this in the proposal.
The main expected result was that the 'Import Spreadsheet' option should be hidden, which I addressed correctly in my proposal. Additionally, the 'Not Here' page can be easily handled in the PR. I don’t believe I could have overlooked that in the PR. We also have a checklist that includes testing the component with a deep link. https://github.com/Expensify/App/blob/448ca57d9b7aedb7e8dd9b46ce382ec4a1b85e8a/contributingGuides/REVIEWER_CHECKLIST.md?plain=1#L53
cc: @thesahindia @chiragsalian
Hmm okay alright, sorry @nkdengineer. @Nodebrute makes a convincing point and was first so I'll give it over to them. Feel free to create the PR @Nodebrute.
📣 @Nodebrute 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!
Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑💻 Keep in mind: Code of Conduct | Contributing 📖
I’ll raise a PR in a few hours.
@Nodebrute can you link the PR please?
The production deploy automation failed: This should be on [HOLD for Payment 2024-10-07] according to https://github.com/Expensify/App/issues/49855 production deploy checklist, confirmed in https://github.com/Expensify/App/pull/49545#issuecomment-2384302822
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
It was implemented like this. We probably forgot about this case.
We should add a test case. The steps are below:
Precondition: Workspace is connected to QBO.
$250 approved for @thesahindia
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: 9.0.31-14 Reproducible in staging?: Y Reproducible in production?: Y Email or phone of affected tester (no customers): applausetester+kh010901@applause.expensifail.com Issue reported by: Applause Internal Team
Action Performed:
Precondition:
Expected Result:
Import spreadsheet option should be hidden when there is connection in the workspace, as the categories are imported from the connection.
Actual Result:
Import spreadsheet option is available when workspace is connected to QBO
Workaround:
Unknown
Platforms:
Screenshots/Videos
https://github.com/user-attachments/assets/fcc5a939-5393-4521-82b1-bf6faff293dd
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @thesahindia