Implementation and Tests for Sorting Pinned Messages
Proposed changes (including videos or screenshots)
Description of the changes made:
- Added functionality to sort pinned messages in the
GET /api/v1/chat.getPinnedMessagesendpoint. - Implemented the
sortparameter to sort messages based on thetimestamp (ts)field. - Added automated tests to ensure that messages are correctly sorted based on the
sortparameter. - Refactored code for better clarity and readability.
This change allows the /api/v1/chat.getPinnedMessages query to sort pinned messages according to the sort parameter value. Messages can be sorted by timestamp either in ascending or descending order depending on the parameter's value.
Issue(s)
This PR resolves the following issue:
- Adds the ability to sort pinned messages in the
chat.getPinnedMessagesendpoint based on thesortparameter.
If this PR addresses a specific issue or bug, reference the issue number here:
- Fixes the bug related to sorting pinned messages (#36087 ).
Steps to test or reproduce
To test the functionality, follow these steps:
- Send a
GETrequest to the endpoint/api/v1/chat.getPinnedMessages?roomId=room123&sort={"ts":-1}. - Check that the response contains messages sorted by the
ts(timestamp) field in descending order (most recent first). - Also verify that if the
sortparameter is invalid, theerror-invalid-sorterror is returned. - Automated tests can also be run with the command
yarn mocha --require ts-node/register apps/meteor/app/api/server/test/**/*.test.ts.
Make sure sorting works correctly with other sort parameter values (e.g., {"ts":1} for ascending order).
Further comments
The solution was chosen based on the need to allow sorting of pinned messages, which will improve the user experience when querying pinned messages in channels. Considerations:
- The
sortparameter was validated to ensure only valid JSON values are accepted. - Other values may be added in the future to sort by other fields. Alternatives considered:
- Using more complex database aggregations for sorting, but for simplicity and flexibility, the current implementation meets the use case.
⚠️ No Changeset found
Latest commit: 8c2c48fbdcd45b383beaeb6cb7d0ad55acc61921
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
Looks like this PR is not ready to merge, because of the following issues:
- This PR has conflicts, please resolve them before merging
- This PR is missing the 'stat: QA assured' label
- This PR is not mergeable
- This PR is missing the required milestone or project
- This PR has an invalid title
Please fix the issues and try again
If you have any trouble, please check the PR guidelines
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 2 committers have signed the CLA.
:x: Juhvitoria4
:x: CaioPacheco
You have signed the CLA already but the status is still pending? Let us recheck it.
Note: This review was focused on code quality, consistency, and adherence to existing codebase standards rather than technical implementation details.