[HOLD for payment 2024-03-26] [$500] Category - Unselected subcategories are indented w/o parent when there is <8 categories

lanitochka17 commented 7 months ago

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: 1.4.47-2 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4381052 Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause - Internal Team Slack conversation:

Action Performed:

User is an employee of Collect workspace
The Collect workspace has less than 8 categories and has one parent category with three subcategories
1. Go to staging.new.expensify.com
2. Go to workspace chat
3. Open manual request page
4. Enter amount > Next
5. Click Show more > Category
6. Select a subcategory
7. Click Category

Expected Result:

The selected category will be displayed at the top in Parent: Child format, while the parent category remains for other unselected subcategories

Actual Result:

The parent category moves to the top, and the other two unselected subcategories are indented without parent

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/89c75960-794e-4a78-b7d3-a49d6054aa17

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~011bca94c973f91072
Upwork Job ID: 1767773523777097728
Last Price Increase: 2024-03-13

Issue Owner

Current Issue Owner: @JmillsExpensify

melvin-bot[bot] commented 7 months ago

Triggered auto assignment to @JmillsExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

lanitochka17 commented 7 months ago

@JmillsExpensify 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

lanitochka17 commented 7 months ago

We think that this bug might be related to #wave6 CC @greg-schroeder

gijoe0295 commented 7 months ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

Unselected subcategories are indented w/o parent when there is <8 categories

What is the root cause of that problem?

We build the option tree for < 8 list (small list) without enabling the isOneLine param:

https://github.com/Expensify/App/blob/e3b10f7da5a6935a0fe461f7acf950129d6a04f0/src/libs/OptionsListUtils.ts#L1008

What changes do you think we should make in order to solve the problem?

We have many approaches for this:

Be consistent with large list (>= 8), which means the selected subcategory would show in one line format (Insurance: Health) while the rest are indented:

Large list	Small list

To do this, we need to divided the section into two child sections, all with empty title:

The selected section, calling getCategoryOptionTree with isOneLine=true
And the rest (without isOneLine)

Suggested code change

```javascript if (numberOfVisibleCategories < CONST.CATEGORY_LIST_THRESHOLD) { categorySections.push({ title: '', shouldShow: false, indexOffset, data: getCategoryOptionTree(selectedOptions, true), }); indexOffset += selectedOptions.length; categorySections.push({ title: '', shouldShow: false, indexOffset, data: getCategoryOptionTree(enabledWithoutSelectedCategories), }); return categorySections; } ```

The simple approach: Use one line format for both the selected subcategory and the rest. Just call getCategoryOptionTree with isOneLine=true

We handle those logics inside here.

What alternative solutions did you explore? (Optional)

We can combine those two approaches to follow whichever we want. Let's see what design team weigh in here.

melvin-bot[bot] commented 7 months ago

@JmillsExpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

melvin-bot[bot] commented 7 months ago

@JmillsExpensify 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

melvin-bot[bot] commented 7 months ago

Job added to Upwork: https://www.upwork.com/jobs/~011bca94c973f91072

melvin-bot[bot] commented 7 months ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @c3024 (External)

JmillsExpensify commented 7 months ago

Added the external label. We already have one proposal above.

gijoe0295 commented 7 months ago

Note that I had proposal here https://github.com/Expensify/App/issues/37774#issuecomment-1979335940.

Voloda911 commented 7 months ago

Revise Display Logic: Modify the logic that controls the display of parent and child categories to ensure the selected subcategory appears alongside its parent category in the "Parent: Child" format.

UI Component Update: Update or create a new UI component that accurately handles and displays the hierarchy of categories and subcategories, including clear visual indicators for understanding category structure.

Cross-Platform Testing: Ensure the solution is tested across all supported platforms (Android, iOS, MacOS, etc.) to confirm the issue is resolved universally.

Automated Testing: Add or update existing automated tests to verify the correct display of categories and subcategories, preventing future regressions.

melvin-bot[bot] commented 7 months ago

📣 @Voloda911! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

Make sure you've read and understood the contributing guidelines.
Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Format:
```
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>
```

c3024 commented 7 months ago

I think the smaller list selection should be consistent with the larger list like shown in the picture in approach 1 in this proposal here.

Approach 1 works well for this.

Proposal here by @gijoe0295 looks good to me.

🎀 👀 🎀 C+ Reviewed

melvin-bot[bot] commented 7 months ago

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

amyevans commented 7 months ago

Agreed on the first approach outlined by @gijoe0295

melvin-bot[bot] commented 7 months ago

❌ There was an error making the offer to @c3024 for the Reviewer role. The BZ member will need to manually hire the contributor.

melvin-bot[bot] commented 7 months ago

❌ There was an error making the offer to @gijoe0295 for the Contributor role. The BZ member will need to manually hire the contributor.

melvin-bot[bot] commented 7 months ago

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

melvin-bot[bot] commented 7 months ago

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot[bot] commented 7 months ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 1.4.54-4 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

https://github.com/Expensify/App/pull/38279

If no regressions arise, payment will be issued on 2024-03-26. :confetti_ball:

For reference, here are some details about the assignees on this issue:

@c3024 requires payment (Needs manual offer from BZ)
@gijoe0295 requires payment (Needs manual offer from BZ)

melvin-bot[bot] commented 7 months ago

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:

[ ] [@c3024] The PR that introduced the bug has been identified. Link to the PR:
[ ] [@c3024] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
[ ] [@c3024] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
[ ] [@c3024] Determine if we should create a regression test for this bug.
[ ] [@c3024] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[ ] [@JmillsExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

gijoe0295 commented 7 months ago

Hi @JmillsExpensify Can you please send me a new offer on this job? There're some errors on Upwork site that I cannot accept the existing one.

gijoe0295 commented 6 months ago

@JmillsExpensify We're good for payment. Also please send me a new offer as per ^.

c3024 commented 6 months ago

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:

[X] [@c3024] The PR that introduced the bug has been identified. Link to the PR: #34848
[x] [@c3024] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/App/pull/34848/files#r1542726766
[x] [@c3024] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:Not started because this is an uncommon bug which can not be caught in checklists.
[x] [@c3024] Determine if we should create a regression test for this bug. Yes
[x] [@c3024] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

Create a workspace and enable categories for this workspace.
Create at least two child categories (subcategories) A and B under a (parent) category C for this workspace.
Ensure that total categories are less than 8.
Click on FAB > Request Money > Manual > Input an amount > Next > Select the workspace created above
Click on Show more and click on Category and select the sub-category A
Click on Category again
Verify that selected category A is shown at the top of the list and shown in one line format (C:A) while other unselected child B is indented to its parent category C

👍 or 👎

melvin-bot[bot] commented 6 months ago

@JmillsExpensify, @amyevans, @c3024, @gijoe0295 Huh... This is 4 days overdue. Who can take care of this?

JmillsExpensify commented 6 months ago

Payment summary:

Contributor: $500 @gijoe0295
Contributor+: $500 @c3024

JmillsExpensify commented 6 months ago

Offer sent to contributor. @c3024 please send your Upwork profile and I'll send an offer as well.

c3024 commented 6 months ago

@JmillsExpensify thanks. Here's my profile link

https://www.upwork.com/freelancers/~0105555e2f227dbf47

JmillsExpensify commented 6 months ago

Thanks! Both contributor contracts paid, and regression test request has been created, so I'm closing this one.

Expensify / App