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.

Sign up for GitHub

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

Jump to bottom

[IMPROVE] Add pagination to search messages And Fix the search for encrypted room #3212

Merged
merged 26 commits into from
Oct 5, 2021
Merged

[IMPROVE] Add pagination to search messages And Fix the search for encrypted room #3212

merged 26 commits into from
Oct 5, 2021

Conversation

reinaldonetof
Copy link
Contributor

@reinaldonetof reinaldonetof commented Jun 15, 2021 •
edited
Loading

Proposed changes

Pagination on SearchMessageView

Issue(s)

There isn't pagination when search

How to test or reproduce

For normal rooms:

  • Create a room
  • write a lot, if you wanna test with a, repeat more than 50 times for the pagination work
  • To search you will need to write /a/ in input to search for a in any position of the sentence, this will catch the "a" and the "cat" or the "123a1"

For encrypted rooms:

  • Need to activate your E2E encryption
  • Then, activate encrypt to the room
  • and do the same as before:
  • write a lot, if you wanna test with a, repeat more than 50 times for the pagination work
  • To search you will need to write /a/ in input to search for a in any position of the sentence, this will catch the "a" and the "cat" or the "123a1"

Screenshots

Before After

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

@lgtm-com
Copy link

lgtm-com bot commented Jun 15, 2021

This pull request introduces 1 alert when merging 6f40f09 into c744672 - view on LGTM.com

new alerts:

  • 1 for Potentially inconsistent state update

@lgtm-com
Copy link

lgtm-com bot commented Jun 28, 2021

This pull request introduces 1 alert when merging 838562e into a6ded95 - view on LGTM.com

new alerts:

  • 1 for Potentially inconsistent state update

@lgtm-com
Copy link

lgtm-com bot commented Jun 28, 2021

This pull request introduces 1 alert when merging 41cb198 into a6ded95 - view on LGTM.com

new alerts:

  • 1 for Potentially inconsistent state update

gerzonc
gerzonc approved these changes Jun 28, 2021
View reviewed changes
Copy link
Contributor

@gerzonc gerzonc left a comment

Choose a reason for hiding this comment

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

LGTM

@lgtm-com
Copy link

lgtm-com bot commented Jun 28, 2021

This pull request introduces 1 alert when merging 52f6390 into a6ded95 - view on LGTM.com

new alerts:

  • 1 for Potentially inconsistent state update

@gerzonc gerzonc changed the title [FIX] Pagination in SearchMessage by the javascript [FIX] JS Pagination in SearchMessageView Jun 28, 2021
@gerzonc gerzonc changed the title [FIX] JS Pagination in SearchMessageView [FIX] JS Pagination in SearchMessagesView Jun 28, 2021
diegolmello
diegolmello requested changes Jul 2, 2021
View reviewed changes
app/views/SearchMessagesView/index.js Outdated Show resolved Hide resolved
app/views/SearchMessagesView/index.js Outdated Show resolved Hide resolved
app/views/SearchMessagesView/index.js Outdated Show resolved Hide resolved
app/views/SearchMessagesView/index.js Outdated Show resolved Hide resolved
@diegolmello diegolmello changed the title [FIX] JS Pagination in SearchMessagesView [IMPROVE] Add pagination to search messages Jul 22, 2021
diegolmello
diegolmello requested changes Jul 22, 2021
View reviewed changes
Copy link
Member

@diegolmello diegolmello left a comment

Choose a reason for hiding this comment

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

Can you add a test plan for both normal and encrypted rooms?

app/views/SearchMessagesView/index.js Outdated Show resolved Hide resolved
app/views/SearchMessagesView/index.js Outdated Show resolved Hide resolved
app/views/SearchMessagesView/index.js Outdated Show resolved Hide resolved
@reinaldonetof reinaldonetof changed the title [IMPROVE] Add pagination to search messages [IMPROVE] Add pagination to search messages And Fix the search for encrypted room Jul 22, 2021
@CLAassistant
Copy link

CLAassistant commented Jul 29, 2021 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

AlexAlexandre
AlexAlexandre approved these changes Aug 23, 2021
View reviewed changes
Copy link
Contributor

@AlexAlexandre AlexAlexandre left a comment

Choose a reason for hiding this comment

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

LGTM

AlexAlexandre
AlexAlexandre approved these changes Aug 31, 2021
View reviewed changes
Copy link
Contributor

@AlexAlexandre AlexAlexandre left a comment

Choose a reason for hiding this comment

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

LGTM

AlexAlexandre
AlexAlexandre approved these changes Oct 3, 2021
View reviewed changes
Copy link
Contributor

@AlexAlexandre AlexAlexandre left a comment

Choose a reason for hiding this comment

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

LGTM :)

@diegolmello diegolmello merged commit 2a19054 into develop Oct 5, 2021
@diegolmello diegolmello deleted the fix.pagination-search-message-view branch October 5, 2021 13:58
ivnxyz pushed a commit to NextiaDev/Rocket.Chat.ReactNative that referenced this pull request May 26, 2023
@reinaldonetof @gerzonc
@diegolmello @LevyCosta @AlexAlexandre
[IMPROVE] Add pagination to search messages (RocketChat#3212)
2ac4db9 
* [FIX] Pagination in SearchMessage through the javascript

* Minor tweak

* Remove unnecessary state update

* Fix inconsistent value update

* Minor change

* Fixed searchMessages to work with new value of count

* minor tweak

* minor tweak

* minor tweak

* Fix encrypted search

* Added Offset to lib/rocketchat and fixed the search

* fixed the debounce in  search message view

* Needed to compare server version to lower than 3.17.0

Co-authored-by: Gerzon Z <gerzonc@icloud.com>
Co-authored-by: Gerzon Z <gerzonzcanario@gmail.com>
Co-authored-by: Diego Mello <diegolmello@gmail.com>
Co-authored-by: Levy Costa <levycosta471@gmail.com>
Co-authored-by: AlexAlexandre <alexalexandrejr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@diegolmello diegolmello diegolmello requested changes

@AlexAlexandre AlexAlexandre AlexAlexandre approved these changes

@gerzonc gerzonc Awaiting requested review from gerzonc

Assignees
No one assigned
Labels
🐛 bug 🛠 fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@reinaldonetof @CLAassistant @diegolmello @AlexAlexandre @gerzonc @LevyCosta