Skip to content

Conversation

@momesgin
Copy link
Member

@momesgin momesgin commented Jan 5, 2026

Summary

Fixes #14774

Occurred changes and/or fixed issues

Removed the caching logic for chart card data. This change ensures chart statuses (e.g., "Installed") update immediately, fixing a side-effect where a page refresh was required. The caching was originally implemented for performance, but it is no longer necessary due to several subsequent optimizations made to the charts page:

Technical notes summary

Removed the caching logic in:

  • shell/pages/c/_cluster/apps/charts/index.vue
  • shell/models/chart.js

Areas or cases that should be tested

Go to the charts page and install a version of a chart that's not the latest, then come back to the charts page without refreshing the page, you should be able to see the installed and upgradeable icons on the card.

Areas which could experience regressions

The primary focus of testing should be to verify that these changes do not introduce a performance regression on the charts page.

  1. Add a repository that hosts a large number of charts. The Bitnami repo is a good candidate: https://charts.bitnami.com/bitnami

  2. Navigate to the charts page and assess performance while:

  • Scrolling rapidly through the entire list of charts.
  • Applying and clearing various filters from the side panel.
  • Typing a search query into the filter box.

The page should remain responsive. Scrolling should be smooth, and filters should apply without a noticeable lag. The overall performance should be on par with the master branch.

Screenshot/Video

cache.mov

Checklist

  • The PR is linked to an issue and the linked issue has a Milestone, or no issue is needed
  • The PR has a Milestone
  • The PR template has been filled out
  • The PR has been self reviewed
  • The PR has a reviewer assigned
  • The PR has automated tests or clear instructions for manual tests and the linked issue has appropriate QA labels, or tests are not needed
  • The PR has reviewed with UX and tested in light and dark mode, or there are no UX changes
  • The PR has been reviewed in terms of Accessibility
  • The PR has considered, and if applicable tested with, the three Global Roles Admin, Standard User and User Base

@momesgin momesgin added this to the v2.14.0 milestone Jan 5, 2026
@momesgin momesgin self-assigned this Jan 5, 2026
@momesgin momesgin added the QA/manual-test Indicates issue requires manually testing label Jan 5, 2026
@momesgin momesgin requested a review from Copilot January 5, 2026 21:50
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an issue where chart status indicators (e.g., "Installed", "Upgradeable") did not update immediately after installing or upgrading a chart, requiring a page refresh. The fix removes caching logic that was originally added for performance but is no longer necessary due to subsequent optimizations (tooltips, lazy loading, and LazyImage component improvements).

Key Changes:

  • Removed the appCardsCache object and associated caching logic from the charts page component
  • Removed the _cardContent caching in the Chart model's cardContent getter to enable reactive status updates
  • Simplified the code by directly computing card data on each access rather than maintaining a cache

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
shell/pages/c/_cluster/apps/charts/index.vue Removed appCardsCache from component data and simplified the appChartCards computed property to directly map charts without caching
shell/models/chart.js Removed caching logic from the cardContent getter to compute statuses dynamically, allowing immediate updates when charts are installed or upgraded

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@momesgin momesgin requested a review from rak-phillip January 5, 2026 23:05
Copy link
Member

@rak-phillip rak-phillip left a comment

Choose a reason for hiding this comment

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

LGTM - I gave this a quick test with the steps outlined above. The issue looks to be resolved and I didn't encounter any aforementioned performance issues.

@momesgin momesgin merged commit b5535d4 into rancher:master Jan 6, 2026
101 of 107 checks passed
@momesgin momesgin deleted the 14774-remove-caching-charts branch January 6, 2026 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

QA/manual-test Indicates issue requires manually testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Chart's app state does not update once apps are fetched

2 participants