Skip to content

Comments

fix: sort parameter validation on /api/v1/audit.settings endpoint#38366

Merged
kodiakhq[bot] merged 2 commits intodevelopfrom
fix/auditsettings-sort
Jan 28, 2026
Merged

fix: sort parameter validation on /api/v1/audit.settings endpoint#38366
kodiakhq[bot] merged 2 commits intodevelopfrom
fix/auditsettings-sort

Conversation

@ricardogarim
Copy link
Contributor

@ricardogarim ricardogarim commented Jan 27, 2026

Proposed changes (including videos or screenshots)

The sort parameter on the /api/v1/audit.settings endpoint was returning a 400 error when using the standard string format (sort={"ts":1}).

The root cause is that AJV validation schema defined sort as an object type, but parseJsonQuery() only handles string format. This mismatch caused string format to fail validation.

To fix, changed the schema to use type: 'string' for the sort parameter, aligning with all other endpoints in the codebase and the official API documentation.

Issue(s)

Steps to test or reproduce

Further comments

Summary by CodeRabbit

  • Bug Fixes

    • Fixed sort parameter validation on the audit settings API endpoint to accept string format.
  • Tests

    • Added test coverage for timestamp sorting in audit settings, verifying both ascending and descending sort orders as well as default sorting behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Jan 27, 2026

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is targeting the wrong base branch. It should target 8.2.0, but it targets 8.1.0

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Jan 27, 2026

🦋 Changeset detected

Latest commit: 3b7a9a7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 40 packages
Name Type
@rocket.chat/rest-typings Patch
@rocket.chat/meteor Patch
@rocket.chat/api-client Patch
@rocket.chat/core-services Patch
@rocket.chat/ddp-client Patch
@rocket.chat/http-router Patch
@rocket.chat/models Patch
@rocket.chat/ui-contexts Patch
@rocket.chat/web-ui-registration Patch
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/federation-matrix Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/abac Patch
@rocket.chat/network-broker Patch
@rocket.chat/omni-core-ee Patch
@rocket.chat/livechat Patch
@rocket.chat/mock-providers Patch
@rocket.chat/cron Patch
@rocket.chat/instance-status Patch
@rocket.chat/omni-core Patch
@rocket.chat/ui-client Patch
@rocket.chat/media-calls Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/fuselage-ui-kit Patch
@rocket.chat/gazzodown Patch
@rocket.chat/ui-avatar Patch
@rocket.chat/ui-video-conf Patch
@rocket.chat/ui-voip Patch
@rocket.chat/core-typings Patch
@rocket.chat/apps Patch
@rocket.chat/model-typings Patch
@rocket.chat/license Patch
@rocket.chat/pdf-worker Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 27, 2026

Walkthrough

The PR fixes the sort parameter validation on the /api/v1/audit.settings endpoint by changing its schema from an object format to a string format, accompanied by type definition updates and comprehensive end-to-end tests for sorting behavior.

Changes

Cohort / File(s) Summary
Documentation
.changeset/odd-colts-doubt.md
Changelog entry documenting patch version bumps for @rocket.chat/rest-typings and @rocket.chat/meteor, with a fix note for sort parameter validation on the audit.settings API.
Type Definitions
packages/rest-typings/src/v1/server-events/ServerEventsAuditSettingsParamsGET.ts
Updated ServerEventsAuditSettingsParamsGetSchema to change sort property from an object with optional ts numeric field to a simple optional string, simplifying the validation schema.
End-to-End Tests
apps/meteor/tests/end-to-end/api/settings.ts
Added "Sorting" test suite with helper predicates (isSortedAscending, isSortedDescending) and three test cases covering ascending sort (ts: 1), descending sort (ts: -1), and default sorting behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Suggested labels

stat: ready to merge, stat: QA assured

Suggested reviewers

  • ggazzo

Poem

🐰 A sort parameter fixed at last,
From objects to strings, the die is cast!
Tests now validate the ts flow so bright,
Ascending, descending, all feels right! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing sort parameter validation on the /api/v1/audit.settings endpoint, which is the core focus of the changeset.
Linked Issues check ✅ Passed The PR implements the required fix for SUP-974 by changing the sort schema from object to string, ensuring the endpoint accepts standard string-formatted sort parameters without returning HTTP 400.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing sort parameter validation: schema update, test coverage for sorting behavior, and changeset documentation. No unrelated modifications are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/auditsettings-sort

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ricardogarim ricardogarim added this to the 8.2.0 milestone Jan 27, 2026
@ricardogarim ricardogarim marked this pull request as ready for review January 27, 2026 12:45
@ricardogarim ricardogarim requested review from a team as code owners January 27, 2026 12:45
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 3 files

@codecov
Copy link

codecov bot commented Jan 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.80%. Comparing base (31d0c01) to head (3b7a9a7).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #38366      +/-   ##
===========================================
+ Coverage    70.78%   70.80%   +0.01%     
===========================================
  Files         3159     3159              
  Lines       109397   109397              
  Branches     19669    19652      -17     
===========================================
+ Hits         77437    77455      +18     
+ Misses       29933    29910      -23     
- Partials      2027     2032       +5     
Flag Coverage Δ
e2e 60.33% <ø> (+<0.01%) ⬆️
e2e-api 48.89% <ø> (+1.03%) ⬆️
unit 71.97% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 27, 2026

📦 Docker Image Size Report

➡️ Changes

Service Current Baseline Change Percent
sum of all images 0B 0B 0B
account-service 0B 0B 0B
authorization-service 0B 0B 0B
ddp-streamer-service 0B 0B 0B
omnichannel-transcript-service 0B 0B 0B
presence-service 0B 0B 0B
queue-worker-service 0B 0B 0B
rocketchat 0B 0B 0B

📊 Historical Trend

---
config:
  theme: "dark"
  xyChart:
    width: 900
    height: 400
---
xychart
  title "Image Size Evolution by Service (Last 30 Days + This PR)"
  x-axis ["11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 21:57", "12/03 21:00", "12/04 18:17", "12/05 21:56", "12/08 20:15", "12/09 22:17", "12/10 23:26", "12/11 21:56", "12/12 22:45", "12/13 01:34", "12/15 22:31", "12/16 22:18", "12/17 21:04", "12/18 23:12", "12/19 23:27", "12/20 21:03", "12/22 18:54", "12/23 16:16", "12/24 19:38", "12/25 17:51", "12/26 13:18", "12/29 19:01", "12/30 20:52", "01/28 06:46 (PR)"]
  y-axis "Size (GB)" 0 --> 0.5
  line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
  line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
  line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.00]
  line "omnichannel-transcript-service" [0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
  line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
  line "queue-worker-service" [0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
  line "rocketchat" [0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.00]
Loading

Statistics (last 30 days):

  • 📊 Average: 1.5GiB
  • ⬇️ Minimum: 1.4GiB
  • ⬆️ Maximum: 1.6GiB
  • 🎯 Current PR: 0B
ℹ️ About this report

This report compares Docker image sizes from this build against the develop baseline.

  • Tag: pr-38366
  • Baseline: develop
  • Timestamp: 2026-01-28 06:46:50 UTC
  • Historical data points: 30

Updated: Wed, 28 Jan 2026 06:46:51 GMT

@tassoevan tassoevan added the stat: QA assured Means it has been tested and approved by a company insider label Jan 28, 2026
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Jan 28, 2026
@kodiakhq kodiakhq bot merged commit 75d089c into develop Jan 28, 2026
44 checks passed
@kodiakhq kodiakhq bot deleted the fix/auditsettings-sort branch January 28, 2026 07:07
vanshitahujaa added a commit to vanshitahujaa/Rocket.Chat that referenced this pull request Jan 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants