Skip to content

Fix(RHOAIENG-81200): Model Registry empty states and access messaging show wrong content for admin vs non-admin users - #9143

Open
Philip-Carneiro wants to merge 6 commits into
opendatahub-io:mainfrom
Philip-Carneiro:fix/admin-register-tooltip-messaging
Open

Fix(RHOAIENG-81200): Model Registry empty states and access messaging show wrong content for admin vs non-admin users#9143
Philip-Carneiro wants to merge 6 commits into
opendatahub-io:mainfrom
Philip-Carneiro:fix/admin-register-tooltip-messaging

Conversation

@Philip-Carneiro

@Philip-Carneiro Philip-Carneiro commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

https://redhat.atlassian.net/browse/RHOAIENG-81200

Description

Model Registry empty states and disabled-action messaging did not correctly reflect the user's privilege level. Admin users saw "contact your administrator" messaging and non-admin "request access" copy in places where they should see role-appropriate guidance.

This PR fixes the remaining bug (BUG 3 from the ticket) by making two components admin-aware using the existing AdminCheck extension pattern:

Register button tooltip (ModelDetailsPage):

  • Admin with no registries: "No model registries available — To create a new model registry, go to Model registry settings."
  • Non-admin (unchanged): "Request access to a model registry — contact your administrator."

"Need another registry?" popover (AdminHelpAction):

  • Admin: Popover with link to Model registry settings page
  • Non-admin (unchanged): "Who's my administrator?" popover

How Has This Been Tested?

  • 9 unit tests added (4 for OdhModelDetailsPage, 5 for AdminHelpAction) covering admin, non-admin, unloaded, no-provider, and MUI theme scenarios
  • All tests verify both presence of correct content AND absence of wrong content (anti-tautological)
  • Full test suite passes: 952/952 tests, lint clean, TypeScript clean
  • Manual testing on PSI-16 cluster with admin (cluster-admin) and non-admin (regularuser2) users

Test Impact

  • OdhModelDetailsPage.spec.tsx — 4 tests covering admin/non-admin register button rendering
  • AdminHelpAction.spec.tsx — 5 tests covering admin/non-admin popover content, MUI theme fallback
  • Tests follow the same pattern as existing OdhUnavailableModelRegistry.spec.tsx

Request review criteria:

Self checklist (all need to be checked):

  • The developer has manually tested the changes and verified that the changes work
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has added tests or explained why testing cannot be added (unit or cypress tests for related changes)
  • The code follows our Best Practices (React coding standards, PatternFly usage, performance considerations)

If you have UI changes:

  • Included any necessary screenshots or gifs if it was a UI change.
  • Included tags to the UX team if it was a UI/UX change.

After the PR is posted & before it merges:

  • The developer has tested their solution on a cluster by using the image produced by the PR to main

@openshift-ci

openshift-ci Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign griffin-sullivan for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (9)
  • packages/model-registry/upstream/frontend/src/app/pages/modelCatalog/ModelCatalogRoutes.tsx is excluded by !**/upstream/**
  • packages/model-registry/upstream/frontend/src/app/pages/modelCatalog/screens/ModelDetailsPage.tsx is excluded by !**/upstream/**
  • packages/model-registry/upstream/frontend/src/app/pages/modelRegistry/screens/components/AdminHelpAction.tsx is excluded by !**/upstream/**
  • packages/model-registry/upstream/frontend/src/app/pages/modelRegistry/screens/components/__tests__/AdminHelpAction.spec.tsx is excluded by !**/upstream/**
  • packages/model-registry/upstream/frontend/src/odh/components/OdhModelCatalogCoreLoader.tsx is excluded by !**/upstream/**
  • packages/model-registry/upstream/frontend/src/odh/components/OdhModelDetailsPage.tsx is excluded by !**/upstream/**
  • packages/model-registry/upstream/frontend/src/odh/components/OdhModelRegistryCoreLoader.tsx is excluded by !**/upstream/**
  • packages/model-registry/upstream/frontend/src/odh/components/__tests__/OdhModelDetailsPage.spec.tsx is excluded by !**/upstream/**
  • packages/model-registry/upstream/frontend/src/odh/context/AdminStatusContext.tsx is excluded by !**/upstream/**

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: fef08a58-4891-4bf8-a1af-725abdafb189

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@rhods-ci-bot

Copy link
Copy Markdown

/early-gate-build

@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.66667% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.51%. Comparing base (c460e76) to head (e111a02).

Files with missing lines Patch % Lines
...d/src/odh/components/OdhModelCatalogCoreLoader.tsx 77.77% 2 Missing ⚠️
...pp/pages/modelCatalog/screens/ModelDetailsPage.tsx 75.00% 1 Missing ⚠️
...rontend/src/odh/components/OdhModelDetailsPage.tsx 83.33% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #9143      +/-   ##
==========================================
+ Coverage   59.50%   59.51%   +0.01%     
==========================================
  Files        3101     3103       +2     
  Lines       96084    96108      +24     
  Branches    25198    25206       +8     
==========================================
+ Hits        57176    57202      +26     
+ Misses      38908    38906       -2     
Flag Coverage Δ
cypress-mock 69.37% <86.66%> (+0.01%) ⬆️
unit 29.29% <ø> (ø)

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

Files with missing lines Coverage Δ
.../src/app/pages/modelCatalog/ModelCatalogRoutes.tsx 100.00% <100.00%> (ø)
...delRegistry/screens/components/AdminHelpAction.tsx 66.66% <100.00%> (+8.33%) ⬆️
.../src/odh/components/OdhModelRegistryCoreLoader.tsx 86.11% <ø> (ø)
...am/frontend/src/odh/context/AdminStatusContext.tsx 100.00% <100.00%> (ø)
...pp/pages/modelCatalog/screens/ModelDetailsPage.tsx 72.72% <75.00%> (-4.64%) ⬇️
...rontend/src/odh/components/OdhModelDetailsPage.tsx 83.33% <83.33%> (ø)
...d/src/odh/components/OdhModelCatalogCoreLoader.tsx 86.20% <77.77%> (-0.75%) ⬇️

... and 17 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c460e76...e111a02. Read the comment docs.

🚀 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.

…nt for admin vs non-admin users

Signed-off-by: Philip Colares Carneiro <philip.colares@gmail.com>
@Philip-Carneiro
Philip-Carneiro force-pushed the fix/admin-register-tooltip-messaging branch from 8cefaaf to 56a2916 Compare August 10, 2026 06:55
@rhods-ci-bot

Copy link
Copy Markdown

/early-gate-build

@Philip-Carneiro

Copy link
Copy Markdown
Contributor Author

/retest

@rhods-ci-bot

Copy link
Copy Markdown

/early-gate-build

@rhods-ci-bot

Copy link
Copy Markdown

/early-gate-build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants