Skip to content
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

[HOLD for payment 2023-12-15] [$500] Web - Search Term in WorkspaceMembersPage lost when clicking on Invite #22534

Closed
1 of 6 tasks
kbecciv opened this issue Jul 9, 2023 · 80 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.

Comments

@kbecciv
Copy link

kbecciv commented Jul 9, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Go to settings
  2. Click on workspaces
  3. Click on Members
  4. Search for a string till no results are found
  5. Click on invite to invite this user

Expected Result:

The TextInput now in InviteUser page should have the string searched in WorkspaceMembersPage because once no results are found for a search string in WorkspaceMembersPage the search string should be there in Invite Page so that it should not be typed again from beginning.

Actual Result:

Search string in TextInput of InvitePage is empty

Workaround:

Unknown

Platforms:

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

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.3.38-3
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2023-07-09.at.3.07.10.PM.1.mov
Recording.3520.mp4

Expensify/Expensify Issue URL:
Issue reported by: @c3024
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1688895491063239

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~013d2f1bf2a057285a
  • Upwork Job ID: 1717179788878622720
  • Last Price Increase: 2023-11-24
  • Automatic offers:
    • aimane-chnaif | Reviewer | 27905086
    • DylanDylann | Contributor | 27905088
@kbecciv kbecciv added Daily KSv2 NewFeature Something to build that is a new item. Bug Something is broken. Auto assigns a BugZero manager. labels Jul 9, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 9, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jul 9, 2023

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 Daily KSv2 labels Jul 9, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 9, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@melvin-bot
Copy link

melvin-bot bot commented Jul 9, 2023

Triggered auto assignment to Design team member for new feature review - @shawnborton (NewFeature)

@kbecciv
Copy link
Author

kbecciv commented Jul 9, 2023

Proposal

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

Search string is lost when clicking on Invite members from WorkspaceMembersPage

What is the root cause of that problem?

We are not passing the search string to Invite page
Presently the route for invite page in ROUTES is as follows:

getWorkspaceInviteRoute: (policyID, search) => `workspace/${policyID}/invite`

The Invite route is not receiving search string from WorkspaceMembersPage

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

We should change this to

getWorkspaceInviteRoute: (policyID, search) => `workspace/${policyID}/invite/?search=${search}`

so that search string is captured in route
and navigation route in inviteUser in WorkspaceMembersPage to

Navigation.navigate(ROUTES.getWorkspaceInviteRoute(props.route.params.policyID, searchValue));

and we initialize searchTerm in state of WorkspaceInvitePage to

searchTerm: props.route.params.search ||  '',

Similar changes can be made if we want to use the search string in invite page in WorkspaceMembersPage

What alternative solutions did you explore? (Optional)

@melvin-bot melvin-bot bot added the Daily KSv2 label Jul 9, 2023
@samh-nl
Copy link
Contributor

samh-nl commented Jul 9, 2023

Proposal

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

The search term is not passed onto the next screen when clicking Invite in WorkspaceMembersPage .

What is the root cause of that problem?

No mechanism is currently in place for communicating the search term to the next screen and using this as the default value of the search input.

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

The search term can be stored in Onyx upon clicking the Invite button, after which it can be retrieved in WorkspaceInvitePage and used for initializing the search term value.

What alternative solutions did you explore? (Optional)

N/A

@shawnborton
Copy link
Contributor

Just want to comment that this is definitely not a bug, this would be a feature request. I do think it's a clever idea to maintain the search term between the manage members page and the invite page, but we should not be paying out a bounty for this one.

@c3024
Copy link
Contributor

c3024 commented Jul 10, 2023

Which one is preferred in Expensify design philosophy for these kind of features? Send as search params or saving to and retrieving from Onyx?

@shawnborton
Copy link
Contributor

That's a good question for our engineers, so maybe once we start taking proposals, we can sort that out.

@c3024
Copy link
Contributor

c3024 commented Jul 10, 2023

There are two proposals here already, one with search params and one with Onyx 😆

@situchan
Copy link
Contributor

It was intentional to clear search in #15661. But now the behavior is reversed?

@shawnborton
Copy link
Contributor

Oh, if that's the case then we can just close this out and consider it not a bug/feature request.

@c3024
Copy link
Contributor

c3024 commented Jul 10, 2023

I think the behaviour mentioned in that issue is better.

I always type in the text box of members page even for new members without much thought. Then click on invite to find the search term missing. I think others also do the same frequently.

I think workspace members -> invite page is common and the search string should remain in invite page.

The behaviour in that earlier issue appears correct for me.

@samh-nl
Copy link
Contributor

samh-nl commented Jul 10, 2023

There's already precedence for storing form values (which this effectively is) in Onyx using the formID prop and using the keys defined in ONYXKEYS.FORMS.

@stephanieelliott stephanieelliott removed their assignment Jul 11, 2023
@sakluger
Copy link
Contributor

I'm going to close this one since it looks like we already decided to consistently clear the value.

@mallenexpensify
Copy link
Contributor

Dropping here for reference since I just posted in #expensify-open-source

Not really a bug nor feature request but everytime I end up on this RHP modal, I think I can add someone to the room by typing in the name/email then clicking the green invite button to invite them. (edited)

image

Thanks @c3024 for sharing this link.

Should we reopen this as a feature request instead of a bug? I have a feeling many/most people think similarly to my thoughts above. I can take a poll in #expensify-open-source if it's be helpful.

@neonbhai
Copy link
Contributor

This could use a redesign, as Remove is a button that is an action on the list below, but Invite is a link to a new page. Very similar elements, very different behaviour.

@rlinoz
Copy link
Contributor

rlinoz commented Dec 4, 2023

I think we can demote this to weekly and wait for @DylanDylann updates.

@melvin-bot melvin-bot bot removed the Overdue label Dec 4, 2023
@rlinoz rlinoz added Weekly KSv2 and removed Daily KSv2 labels Dec 4, 2023
@mallenexpensify
Copy link
Contributor

@DylanDylann , it's been 5 days since you were assigned, when can we expect a PR?

@DylanDylann
Copy link
Contributor

@mallenexpensify I am working on it. Will open the PR today

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Dec 6, 2023
@melvin-bot melvin-bot bot changed the title [$500] Web - Search Term in WorkspaceMembersPage lost when clicking on Invite [HOLD for payment 2023-12-15] [$500] Web - Search Term in WorkspaceMembersPage lost when clicking on Invite Dec 8, 2023
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Dec 8, 2023
Copy link

melvin-bot bot commented Dec 8, 2023

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

Copy link

melvin-bot bot commented Dec 8, 2023

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

If no regressions arise, payment will be issued on 2023-12-15. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

  • External issue reporter
  • Contributor that fixed the issue
  • Contributor+ that helped on the issue and/or PR

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

Copy link

melvin-bot bot commented Dec 8, 2023

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@ArekChr / @aimane-chnaif] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
  • [@mallenexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Dec 14, 2023
@aimane-chnaif
Copy link
Contributor

Regression Test Proposal

  1. Go to Settings > Workspaces
  2. Select any workspace > Members
  3. Search for a string till no results are found
  4. Click Invite button to invite this user
  5. Verify that the text input now in invite page has the same string searched in members page

@mallenexpensify
Copy link
Contributor

mallenexpensify commented Dec 18, 2023

Reporter: @c3024 paid $250 via Upwork (bug was reported in July)
Contributor: @DylanDylann paid $500 via Upwork
Contributor+: @aimane-chnaif paid $500 via Upwork

@c3024 , can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~013d2f1bf2a057285a

Regression test GH
https://github.com/Expensify/Expensify/issues/351498

@melvin-bot melvin-bot bot removed the Overdue label Dec 18, 2023
@c3024
Copy link
Contributor

c3024 commented Dec 18, 2023

Accepted. Thanks. @mallenexpensify

@mallenexpensify
Copy link
Contributor

Should be all set! @c3024 paid and comment above has been updated to reflect payment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.
Projects
None yet
Development

No branches or pull requests