Skip to content

Conversation

@MartinSchoeler
Copy link
Member

@MartinSchoeler MartinSchoeler commented Nov 24, 2025

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

Summary by CodeRabbit

  • Bug Fixes

    • Removed unnecessary loading indicators from ABAC settings interface, improving UI clarity and responsiveness.
  • Refactor

    • Simplified availability state handling and boolean logic across ABAC admin controls for better maintainability.

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

@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Nov 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
  • This PR is missing the required milestone or project

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

⚠️ No Changeset found

Latest commit: 774ab7b

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

Caution

Review failed

The pull request is closed.

Walkthrough

Simplified ABAC state management by removing loading states from the useIsABACAvailable hook and standardizing boolean checks across ABAC admin components. Updated hook consumption pattern to destructure the data property from useHasLicenseModule with fallback defaults instead of direct boolean comparisons.

Changes

Cohort / File(s) Summary
Hook Return Type Refactor
apps/meteor/client/views/admin/ABAC/hooks/useIsABACAvailable.tsx
Changed return type from 'loading' | boolean to boolean; removed loading state handling; now always returns hasABAC && isABACSettingEnabled
useHasLicenseModule Consumption Updates
apps/meteor/client/views/admin/ABAC/AdminABACRoute.tsx, apps/meteor/client/views/admin/ABAC/AdminABACSettings.tsx
Updated from direct boolean checks to destructuring pattern: const { data: hasABAC = false } = useHasLicenseModule('abac')
Component Rendering Simplification
apps/meteor/client/views/admin/ABAC/AdminABACPage.tsx
Removed ContextualbarSkeletonBody import and usage; replaced three-condition rendering (loading/new/edit) with two-condition logic; removed explicit loading state paths
Boolean Check Normalization
apps/meteor/client/views/admin/ABAC/AdminABACRoomAttributes.tsx, apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx
Simplified disabled conditions from disabled={isABACAvailable !== true} to disabled={!isABACAvailable}

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify that removing the loading state from useIsABACAvailable doesn't break any dependent UI or logic that previously handled loading states
  • Confirm that the destructuring pattern with { data: hasABAC = false } correctly handles undefined data scenarios and maintains expected behavior
  • Review AdminABACPage changes to ensure the removal of ContextualbarSkeletonBody doesn't degrade UX during async operations
  • Check that simplified boolean checks (!isABACAvailable vs !== true) are semantically equivalent in all contexts

Possibly related PRs

Suggested labels

stat: ready to merge, stat: QA assured

Suggested reviewers

  • aleksandernsilva
  • tassoevan

Poem

🐰 Loading states trimmed clean and neat,
Boolean checks made pure and sweet,
Hooks restructured, logic refined,
ABAC controls now redesigned! ✨

✨ 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-module-ui

📜 Recent review details

Configuration used: CodeRabbit 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 edf2f69 and 774ab7b.

📒 Files selected for processing (6)
  • apps/meteor/client/views/admin/ABAC/AdminABACPage.tsx (2 hunks)
  • apps/meteor/client/views/admin/ABAC/AdminABACRoomAttributes.tsx (1 hunks)
  • apps/meteor/client/views/admin/ABAC/AdminABACRoute.tsx (1 hunks)
  • apps/meteor/client/views/admin/ABAC/AdminABACSettings.tsx (1 hunks)
  • apps/meteor/client/views/admin/ABAC/hooks/useIsABACAvailable.tsx (1 hunks)
  • apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx (1 hunks)

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.

@github-actions
Copy link
Contributor

📦 Docker Image Size Report

📈 Changes

Service Current Baseline Change Percent
sum of all images 1.2GiB 1.2GiB +12MiB
rocketchat 358MiB 347MiB +12MiB
omnichannel-transcript-service 132MiB 132MiB +10KiB
queue-worker-service 132MiB 132MiB +9.8KiB
ddp-streamer-service 127MiB 127MiB +7.0KiB
account-service 114MiB 114MiB +8.7KiB
authorization-service 111MiB 111MiB +58KiB
stream-hub-service 111MiB 111MiB +8.2KiB
presence-service 111MiB 111MiB +8.4KiB

📊 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/15 22:28", "11/16 01:28", "11/17 23:50", "11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 16:27", "11/24 16:49 (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]
  line "authorization-service" [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]
  line "omnichannel-transcript-service" [0.14, 0.14, 0.14, 0.14, 0.14, 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]
  line "queue-worker-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13]
  line "rocketchat" [0.36, 0.36, 0.35, 0.35, 0.35, 0.34, 0.34, 0.35]
  line "stream-hub-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
Loading

Statistics (last 7 days):

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

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

  • Tag: pr-37596
  • Baseline: develop
  • Timestamp: 2025-11-24 16:49:46 UTC
  • Historical data points: 7

Updated: Mon, 24 Nov 2025 16:49:46 GMT

@KevLehman KevLehman marked this pull request as ready for review November 24, 2025 17:12
@KevLehman KevLehman requested a review from a team as a code owner November 24, 2025 17:12
@KevLehman KevLehman merged commit 009716e into feat/abac Nov 24, 2025
44 of 46 checks passed
@KevLehman KevLehman deleted the fix-module-ui branch November 24, 2025 17:12
This was referenced Dec 1, 2025
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