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] Sort RecordList items in REVERSE chronological order, not FORWARD #27201

Conversation

nmagedman
Copy link
Contributor

@nmagedman nmagedman commented Nov 6, 2022

Proposed changes (including videos or screenshots)

Sort items in the Files sidebar in reverse chronological order (newest at top, oldest at bottom), as it did before RC v3.11.0.

Issue(s)

Fixes #27185

Steps to test or reproduce

See Issue

Further comments

RecordList is a generic widget for use in several parts of the codebase. It remains to be seen whether Date DESC is the correct sort order for all of them, although it does seem reasonable that it ought to be.

@nmagedman nmagedman requested a review from a team as a code owner November 6, 2022 17:27
@nmagedman
Copy link
Contributor Author

@tassoevan This relates to commit 4bfa63c (client/lib/lists/RecordList.ts:36). Please confirm whether "_updatedAt DESC" was the intended sort, or whether you really do want "_updatedAt ASC".

@gabriellsh gabriellsh self-assigned this Nov 8, 2022
gabriellsh
gabriellsh previously approved these changes Nov 8, 2022
@gabriellsh gabriellsh added this to the 5.4.0 milestone Nov 8, 2022
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: needs QA labels Nov 11, 2022
@nmagedman nmagedman force-pushed the noach_seekingalpha_sort_RecordList_items_by_date_DESC branch from 788030e to 3bfdc6c Compare November 15, 2022 11:55
@nmagedman
Copy link
Contributor Author

test-ee has failed twice, with /licenses.isEnterprise returning false instead of the expected true.

From https://github.com/RocketChat/Rocket.Chat/actions/runs/3470198854/jobs/5798656544

  1) licenses
       [/licenses.isEnterprise]
         should pass if user has user role:

      AssertionError: expected { isEnterprise: false, success: true } to have property 'isEnterprise' of true, but got false
      + expected - actual

      -false
      +true

How can we move this PR forward?

@gabriellsh
Copy link
Member

Hey @nmagedman don't worry about that. We are aware of the ee tests situation, and I already added your PR to the milestone, so it should be good to go from your side.

Btw, thanks for the contribution :) !

@ggazzo ggazzo merged commit dfea02d into RocketChat:develop Nov 17, 2022
gabriellsh added a commit that referenced this pull request Nov 21, 2022
…age-ignore-reactivity

* 'develop' of github.com:RocketChat/Rocket.Chat: (23 commits)
  Chore: removing useEndpointData from license api (#26634)
  Chore: Create unique index for `E2EKey` field (#27301)
  [FIX] LDAP "Sync Roles" option doesn't work for custom roles (#26842)
  Chore: UserAvatar wrapper missing key in RoomForeword (#27300)
  Chore: Add deprecation warning to settings (#27295)
  Chore: Refactor LeaveTeam to Typescript (#27197)
  [FIX] Sidebar context menu in searchList (#23830)
  [NEW] Federation events coverage expansion (#27119)
  [FIX] Message Parser version upgrade (#27284)
  [FIX] Replace regex not compatible with safari (#27294)
  [NEW] Emphasis Elements (italic, strike and bold) in Message Parser Components (#27003)
  [FIX] Set default value "false" for global search. (#25568)
  Regression: Custom fields form not showing in user profile nor admin (#27244)
  [FIX] Sort RecordList items in REVERSE chronological order, not FORWARD (#27201)
  Chore: Remove unused css (#27289)
  Chore: also send thread to channel translations (#27242)
  [FIX] Marketplace app details page back-button behavior (#27062)
  Chore: Add tests for omni-jobs & add more context in system messages for jobs (#27048)
  Chore: Fix missing license for forks (#27258)
  i18n: Language update from LingoHub 🤖 on 2022-11-14Z (#27255)
  ...
@ggazzo ggazzo mentioned this pull request Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
communityPR stat: QA skipped stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Files pane sorted by date ASC, but infinity scrolls them by date DESC
5 participants