[HOLD for payment 2024-03-07] [HOLD][$500] [MEDIUM] Tags: All tags are revealed when searching for hidden tag and clearing the search field

[kbecciv]

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: v1.4.44-0
Reproducible in staging?: y
Reproducible in production?: n
Issue reported by: Applause - Internal Team

Action Performed:

Precondition:

• User is an employee of Collect workspace.
• The Collect workspace has more than 500 tags.

1. Go to workspace chat.
2. Open money request page.
3. Proceed to confirmation page.
4. Click Show more > Tag.
5. Search for a term that is originally hidden behind "Show more" button.
   (If there are 700 tags, the last 200 will be hidden behind "Show more". In this case, we can search for the 700th tag.)
6. Clear the search field.
7. Scroll down.

Expected Result:

Only the first 500 tags will appear and the remaining will be hidden with "Show more" button.

Actual Result:

All the tags are revealed and there is no "Show more" button.

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6394409_1709057092251.20240227-182403_QJFQ1NX9.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0171a76cd580844a9b
• Upwork Job ID: 1762547976110538752
• Last Price Increase: 2024-02-27

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0171a76cd580844a9b

[melvin-bot]

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

[kbecciv]

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

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @NikkiWines (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[eucool]

Proposal

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

All tags are revealed when searching for hidden tag and clearing the search field

What is the root cause of that problem?

We multiply by extra paginationPage variable

App/src/components/OptionsSelector/BaseOptionsSelector.js

Line 523 in 928d346

const shouldShowShowMoreButton = allOptions.length > CONST.MAX_OPTIONS_SELECTOR_PAGE_LENGTH * paginationPage;

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

Remove the paginationPage variable,

What alternative solutions did you explore? (Optional)

N/A

[nexarvo]

Proposal

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

All tags are revealed when searching for hidden tag and clearing the search field

What is the root cause of that problem?

We are using paginationPage to decide whether to use show more button or not here:

App/src/components/OptionsSelector/BaseOptionsSelector.js

Line 523 in fa8051c

const shouldShowShowMoreButton = allOptions.length > CONST.MAX_OPTIONS_SELECTOR_PAGE_LENGTH * paginationPage;

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

We need to remove paginationPage from Line 523 and it should solve the issue but paginationPage is used a couple of times in this file and it does not seems to have any use as we are not using pagination in tags page. Ideally we should remove the paginationPage reference to avoid any confusion in future. like this:

App/src/components/OptionsSelector/BaseOptionsSelector.js

Lines 600 to 605 in fa8051c

	<ShowMoreButton
	containerStyle={{...themeStyles.mt2, ...themeStyles.mb5}}
	currentCount={CONST.MAX_OPTIONS_SELECTOR_PAGE_LENGTH * paginationPage}
	totalCount={allOptions.length}
	onPress={incrementPage}
	/>

What alternative solutions did you explore? (Optional)

[NikkiWines]

Looks like this is a regression from #31606, which has several other regressions tied to it as well. cc: @Piotrfj since it looks like you're handling the regressions that arose from that PR

[dragnoir]

@NikkiWines ther's an ongoing refactor and a cleanup for the BaseOptionsSelector #37353 , I think this should be onhold for now

[NikkiWines]

Yeah, that PR came about as a result of the multiple regressions from #31606. You're right though that this issue will be handled by that PR (unless we ultimately decide to revert #31606). Will put it on hold 👍

[roryabraham]

ended up closing #37353 in favor of the revert of #31606. I'll closely review the 2nd attempt to migrate this component in hopes that most or all of the simplifications from #37353 make it into the migration round 2.

[Piotrfj]

Update: re-refactoring is ready to be tested, it fixes all the regression issues I was aware of. Please test it (especially the tags, because I could not yet replicate the scenario on my site).
The fix is based on the @roryabraham solution, so special thanks for him!
Link to PR: #37494

[melvin-bot]

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

[melvin-bot]

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

• Revert 31606 #37454

If no regressions arise, payment will be issued on 2024-03-07. 🎊

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

• @fedirjh requires payment (Needs manual offer from BZ)

[greg-schroeder]

Okay @ishpaul777 you were assigned on the other issue for payment i believe (#37256) so I don't think this one actually requires payment based on what happened, as @fedirjh didn't actually do the review on the linked PR

[greg-schroeder]

We may be able to close in that case

[ishpaul777]

yes this can be closed 👍

[trjExpensify]

Let's close it then! :)