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

fix: Search Page Order #6971

Merged
merged 8 commits into from
Jan 14, 2022
Merged

fix: Search Page Order #6971

merged 8 commits into from
Jan 14, 2022

Conversation

parasharrajat
Copy link
Member

@parasharrajat parasharrajat commented Dec 31, 2021

Details

Fixed Issues

$ #6627
$ #5699

Tests | QA Steps

  1. Open Search Page.

  2. Search for some user name that does not start with #.

  3. Search results should be in the following order:

  4. 1: 1 chat.

  5. group Chat.

  6. Room.

  7. Order will change based on the resemblance between search value and search result. e.g. If the search value matches the room name, the room name would be above. etc.


  1. In an account that has at least 1 workspace
  2. Open search
  3. Search for #announce room
  4. Delete a character and write it again
  5. room name should not duplicate

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web | Desktop

output_file.mp4

Mobile Web

image

iOS

Android

image

@parasharrajat parasharrajat marked this pull request as ready for review January 10, 2022 22:34
@roryabraham
Copy link
Contributor

Sorry for the delay here. Reviewing now...

Copy link
Contributor

@roryabraham roryabraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good – my only suggestion is that we should order rooms so that any (deleted) rooms appear after open rooms.

@roryabraham
Copy link
Contributor

We can do that pretty easily by replacing this line with:

let orderedReports = lodashOrderBy(reports, sortProperty, sortDirection);
orderedReports = _.sortBy(orderedReports, report => ReportUtils.isArchivedRoom(report));

@parasharrajat
Copy link
Member Author

@roryabraham Updated.

@roryabraham roryabraham merged commit 921bad2 into Expensify:main Jan 14, 2022
@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @roryabraham in version: 1.1.29-8 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 failure ❌
🕸 web 🕸 success ✅

@kavimuru
Copy link

@parasharrajat Issue #5699 is still reproduced. This PR is failing.

Screen Shot 2022-01-15 at 2 52 03 PM

@parasharrajat
Copy link
Member Author

parasharrajat commented Jan 15, 2022

@kavimuru Could you please follow these steps for me and share the results?

  1. Open first announce room.
  2. Save the id from URL and send one message to it.
  3. Go back to some other chat but not any of these rooms.
  4. Search for # announce and then open the second one.
  5. Note the id and tell me if you see the same message again.

@parasharrajat
Copy link
Member Author

@mvtglobally Any luck above?

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.1.30-3 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@kavimuru
Copy link

@parasharrajat Sorry for the delay. Issue #5699 is not reproduced anymore. I followed these steps and not seeing the same message.

  1. Open first announce room.
  2. Save the id from URL and send one message to it.
  3. Go back to some other chat but not any of these rooms.
  4. Search for # announce and then open the second one.
  5. Note the id and tell me if you see the same message again.
Recording.153.mp4

@parasharrajat
Copy link
Member Author

parasharrajat commented Jan 19, 2022

Thanks for looking into this @kavimuru. Based on these, I can say that these are two rooms with the same name. I saw the same results but as the message is not visible in the other rooms it means that they are different. Also, the id is different on URL.

It may be possible that these were created during the early development of Rooms so they have the same names.

Another fact is that only rooms with # announce name is shown twice which I suppose are auto-created.

It could be a good question to ask the team if two auto-generated rooms can have the same name. Sound like a bug but not related to this issue?

@jasperhuangg
Copy link
Contributor

Hey! I know this is from a while back, but it looks like this PR is somewhat related to this bug. In addition to updating the indexOffset for that new section that you added, we also need to update the indexOffset for the usersToInvite section. The PR with that fix is here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants