Skip to content

fix(ui): show disabled-source warning in model catalog preview - #3192

Merged
google-oss-prow[bot] merged 5 commits into
kubeflow:mainfrom
ConorOM1:preview_fix
Sep 17, 2026
Merged

fix(ui): show disabled-source warning in model catalog preview#3192
google-oss-prow[bot] merged 5 commits into
kubeflow:mainfrom
ConorOM1:preview_fix

Conversation

@ConorOM1

@ConorOM1 ConorOM1 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Description

When a catalog source was disabled, the Model catalog preview still showed all models in the "Models included" tab. That made it look like disabling the source had no effect.

  • Enable source checkbox is moved above Model visibility
  • Preview still shows filter results so users can configure model visibility before enabling a source
  • A warning alert is shown above the preview when the source is disabled and a preview has been loaded:
  • Source disabled.
  • Models from this source will not appear in the model catalog until the source is enabled.
  • Toggling Enable source only shows/hides the warning — it does not trigger the "Refresh preview" alert
  • No changes to preview API behavior (enabled is not included in preview requests)
  • UI change: Warning alert appears in the preview panel when previewing a disabled source. Models remain visible in the list.

How Has This Been Tested?

  • npm run test:lint — passed
  • npm run test:type-check — passed
  • npm run test:unit — model catalog settings + PreviewPanel (63 tests) — passed
  • New unit tests in PreviewPanel.spec.tsx for warning visibility (shown when disabled + previewed, hidden when enabled, hidden before preview)
  • New Cypress test: should show source disabled warning after preview when source is disabled

Manual:

  1. Go to Model catalog settings → Add/Manage source
  2. Fill required fields, leave Enable source unchecked, click Preview
  3. Confirm warning appears and models are still listed
  4. Check Enable source — warning should disappear without a refresh prompt
  5. Open a saved disabled source — warning should appear after auto-preview loads

source enabled:
image

source disabled:
image

Merge criteria:

  • All the commits have been signed-off (To pass the DCO check)
  • The commits have meaningful messages
  • Automated tests are provided as part of the PR for major new functionalities; testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work.
  • Code changes follow the kubeflow contribution guidelines.
  • For first time contributors: Please reach out to the Reviewers to ensure all tests are being run, ensuring the label ok-to-test has been added to the PR.

If you have UI changes

  • The developer has added tests or explained why testing cannot be added.
  • Included any necessary screenshots or gifs if it was a UI change.
  • Verify that UI/UX changes conform the UX guidelines for Kubeflow.

@manaswinidas

manaswinidas commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

/ok-to-test

Philip-Carneiro
Philip-Carneiro approved these changes Sep 15, 2026
View reviewed changes

@Philip-Carneiro Philip-Carneiro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

/lgtm
works fine, but will conflict with other PR to add warning msgs in the preview as well.

@google-oss-prow google-oss-prow Bot added the lgtm label Sep 15, 2026
@ConorOM1
update model preview
763d614 
Signed-off-by: Conor O'Malley <conormomalley@gmail.com>
@ConorOM1
update test
b0063e8 
Signed-off-by: Conor O'Malley <conormomalley@gmail.com>
@google-oss-prow google-oss-prow Bot removed the lgtm label Sep 16, 2026
@ConorOM1

ConorOM1 commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

@Philip-Carneiro thanks for reviewing, rebased and retested:
image

Philip-Carneiro
Philip-Carneiro suggested changes Sep 16, 2026
View reviewed changes
Comment thread clients/ui/frontend/src/app/pages/modelCatalogSettings/constants.tsx Outdated
Comment thread clients/ui/frontend/src/app/pages/modelCatalogSettings/constants.tsx Outdated
@ConorOM1
address comments
03481df 
Signed-off-by: Conor O'Malley <conormomalley@gmail.com>
@Philip-Carneiro

Philip-Carneiro commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

/lgtm
Perfect o/

@google-oss-prow google-oss-prow Bot added the lgtm label Sep 16, 2026
@Philip-Carneiro

Philip-Carneiro commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

/approve

@ConorOM1
microcopy update
d36ff2f 
Signed-off-by: Conor O'Malley <conormomalley@gmail.com>
@google-oss-prow google-oss-prow Bot removed the lgtm label Sep 16, 2026
@Philip-Carneiro

Philip-Carneiro commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

/lgtm

@google-oss-prow google-oss-prow Bot added the lgtm label Sep 16, 2026
ppadti
ppadti suggested changes Sep 17, 2026
View reviewed changes

@ppadti ppadti left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One minor thing

Comment thread clients/ui/frontend/src/app/pages/modelCatalogSettings/components/PreviewPanel.tsx Outdated
const { isLoadingInitial, isLoadingMore, activeTab, summary, tabStates, error } = previewState;
const { items, hasMore } = tabStates[activeTab];
const previewError = error;
const showSourceDisabledWarning = !isSourceEnabled && !!summary;

@ppadti ppadti Sep 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we also check on error state here - so that we won't show this when we get a error in preview response?
something like this? and a test case for this in ppreviewPanel.spec.tsx?

Suggested change
const showSourceDisabledWarning = !isSourceEnabled && !!summary;
const showSourceDisabledWarning = !isSourceEnabled && !!summary && !previewError;

@google-oss-prow google-oss-prow Bot removed the lgtm label Sep 17, 2026
@ConorOM1 ConorOM1 mentioned this pull request Sep 17, 2026
Merged
8 tasks
@ConorOM1
suppress for error state
a4a365b 
Signed-off-by: Conor O'Malley <conormomalley@gmail.com>
ppadti
ppadti approved these changes Sep 17, 2026
View reviewed changes

@ppadti ppadti left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @ConorOM1
/lgtm
/approve

@google-oss-prow google-oss-prow Bot added the lgtm label Sep 17, 2026
@google-oss-prow

google-oss-prow Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Philip-Carneiro, ppadti

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

The pull request process is described 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

@google-oss-prow
google-oss-prow Bot merged commit 7292a43 into kubeflow:main Sep 17, 2026
27 of 28 checks passed
@ppadti ppadti mentioned this pull request Sep 17, 2026
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fege fege Awaiting requested review from fege
2 more reviewers
@Philip-Carneiro Philip-Carneiro Philip-Carneiro requested changes
@ppadti ppadti ppadti approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@Philip-Carneiro Philip-Carneiro

@ppadti ppadti

Labels

approved Area/UI lgtm ok-to-test size/L

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ConorOM1 @manaswinidas @Philip-Carneiro @ppadti