Skip to content

Conversation

@MartinSchoeler
Copy link
Member

@MartinSchoeler MartinSchoeler commented Dec 24, 2025

Proposed changes (including videos or screenshots)

Issue(s)

ABAC-113

Steps to test or reproduce

Further comments

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Fixed the initial state of attribute values in the ABAC attributes editor when editing existing attributes. Previously, an unnecessary empty entry was pre-populated in the form.

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

@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Dec 24, 2025

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

  • This PR is missing the 'stat: QA assured' label

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 Dec 24, 2025

⚠️ No Changeset found

Latest commit: 1e382cd

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 24, 2025

Walkthrough

A form initialization fix in AttributesContextualBar that changes the default attributeValues from a single empty object to an empty array when editing existing attributes, preventing automatic addition of unused empty value fields in the UI.

Changes

Cohort / File(s) Summary
Form initialization fix
apps/meteor/client/views/admin/ABAC/ABACAttributesTab/AttributesContextualBar.tsx
Modified defaultValues for attributeValues to initialize as empty array instead of array with single empty object when attributeData exists, eliminating unwanted pre-populated empty value entry during attribute edit mode

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • #37782 — Modifies form mode and related save handling in the same AttributesContextualBar component
  • #37952 — Changes query invalidation logic in the same component alongside form initialization behavior
  • #37872 — Modifies cache invalidation in onSettled within the same AttributesContextualBar file

Suggested labels

stat: ready to merge, stat: QA assured

Suggested reviewers

  • tassoevan
  • KevLehman

Poem

🐰 An empty array hops in with glee,
No phantom values vex thee!
Edit mode flows clean and bright,
One small fix makes forms just right. ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the regression and the specific fix: preventing an unwanted new field from appearing when editing ABAC attributes.
Linked Issues check ✅ Passed The code change directly addresses ABAC-113 by modifying attributeValues initialization from a single empty object to an empty array when editing existing attributes, eliminating the extra empty field.
Out of Scope Changes check ✅ Passed The change is narrowly scoped to fixing the specific regression in AttributesContextualBar, with no unrelated modifications to other components or functionality.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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 regression-abac-fields

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.

@MartinSchoeler MartinSchoeler added this to the 8.0.0 milestone Dec 24, 2025
@codecov
Copy link

codecov bot commented Dec 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.62%. Comparing base (0dddad6) to head (1e382cd).
⚠️ Report is 4 commits behind head on release-8.0.0.

Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                @@
##           release-8.0.0   #37966      +/-   ##
=================================================
+ Coverage          70.60%   70.62%   +0.01%     
=================================================
  Files               3146     3146              
  Lines             108690   108690              
  Branches           19523    19495      -28     
=================================================
+ Hits               76738    76757      +19     
+ Misses             29947    29937      -10     
+ Partials            2005     1996       -9     
Flag Coverage Δ
e2e 60.16% <ø> (+0.04%) ⬆️
e2e-api 47.43% <ø> (-0.07%) ⬇️
unit 71.75% <ø> (+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.

@MartinSchoeler MartinSchoeler marked this pull request as ready for review December 26, 2025 14:18
@MartinSchoeler MartinSchoeler requested a review from a team as a code owner December 26, 2025 14:18
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 1 file

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 0dddad6 and 1e382cd.

📒 Files selected for processing (1)
  • apps/meteor/client/views/admin/ABAC/ABACAttributesTab/AttributesContextualBar.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation

Files:

  • apps/meteor/client/views/admin/ABAC/ABACAttributesTab/AttributesContextualBar.tsx
🧠 Learnings (3)
📓 Common learnings
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37244
File: apps/meteor/client/views/admin/ABAC/AdminABACRoomAttributesForm.spec.tsx:125-146
Timestamp: 2025-10-30T19:30:46.541Z
Learning: In the AdminABACRoomAttributesForm component (apps/meteor/client/views/admin/ABAC/AdminABACRoomAttributesForm.tsx), the first attribute value field is mandatory and does not have a Remove button. Only additional values beyond the first have Remove buttons. This means trashButtons[0] corresponds to the second value's Remove button, not the first value's.
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
📚 Learning: 2025-10-30T19:30:46.541Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37244
File: apps/meteor/client/views/admin/ABAC/AdminABACRoomAttributesForm.spec.tsx:125-146
Timestamp: 2025-10-30T19:30:46.541Z
Learning: In the AdminABACRoomAttributesForm component (apps/meteor/client/views/admin/ABAC/AdminABACRoomAttributesForm.tsx), the first attribute value field is mandatory and does not have a Remove button. Only additional values beyond the first have Remove buttons. This means trashButtons[0] corresponds to the second value's Remove button, not the first value's.

Applied to files:

  • apps/meteor/client/views/admin/ABAC/ABACAttributesTab/AttributesContextualBar.tsx
📚 Learning: 2025-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.

Applied to files:

  • apps/meteor/client/views/admin/ABAC/ABACAttributesTab/AttributesContextualBar.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: 📦 Build Packages
  • GitHub Check: CodeQL-Build
  • GitHub Check: CodeQL-Build

@github-actions
Copy link
Contributor

📦 Docker Image Size Report

📈 Changes

Service Current Baseline Change Percent
sum of all images 1.1GiB 1.1GiB +11MiB
rocketchat 355MiB 345MiB +11MiB
omnichannel-transcript-service 132MiB 132MiB -412B
queue-worker-service 132MiB 132MiB -629B
ddp-streamer-service 126MiB 126MiB +929B
account-service 113MiB 113MiB +638B
authorization-service 111MiB 111MiB -1.2KiB
presence-service 111MiB 111MiB +819B

📊 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/16 01:28", "11/17 23:50", "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/26 14:37 (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.11]
  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.11]
  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.12]
  line "omnichannel-transcript-service" [0.14, 0.14, 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]
  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.11]
  line "queue-worker-service" [0.14, 0.14, 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]
  line "rocketchat" [0.36, 0.35, 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.35]
Loading

Statistics (last 30 days):

  • 📊 Average: 1.5GiB
  • ⬇️ Minimum: 1.2GiB
  • ⬆️ Maximum: 1.6GiB
  • 🎯 Current PR: 1.1GiB
ℹ️ About this report

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

  • Tag: pr-37966
  • Baseline: develop
  • Timestamp: 2025-12-26 14:37:07 UTC
  • Historical data points: 30

Updated: Fri, 26 Dec 2025 14:37:08 GMT

@tassoevan tassoevan merged commit 586a88a into release-8.0.0 Dec 27, 2025
46 checks passed
@tassoevan tassoevan deleted the regression-abac-fields branch December 27, 2025 18:43
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.

3 participants