Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure lists update when switching between workspaces with the same number of rows #12449

Merged
merged 1 commit into from
Nov 1, 2024

Conversation

richard-cox
Copy link
Member

@richard-cox richard-cox commented Oct 31, 2024

Summary

Fixes #12437

Occurred changes and/or fixed issues

  • Switching between fleet workspaces which contain the same number of resources will fail to update the rows
    • sometimes this would happen anyway, for instance the generation of the resource was updated
  • Caused by change in reactivity with base fn in sortable table pagination pipeline
    • this was fired with stale rows (rows from old workspace) BUT would be cached against the new state (new workspace)
    • whenever the pipeline was triggered again (sans other changes) it would then return the cached stale rows
  • fixed by providing a specific hash to use instead of one generated on demand (which fetched new state)
    • i've limited this to the workspace world to reduce impact / bugs

Technical notes summary

  • this is a quicker fix than re-rewriting the whole thing... or finding out why vue changed behaviour

Areas or cases that should be tested

  • Everything list based

Areas which could experience regressions

  • Everything list based

Screenshot/Video

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

@richard-cox richard-cox added this to the v2.10.0 milestone Oct 31, 2024
@richard-cox richard-cox self-assigned this Oct 31, 2024
@eva-vashkevich
Copy link
Member

Do we need to have a follow up issue to investigate the non-workspace case?

@@ -556,6 +580,7 @@ export default {
:adv-filter-hide-labels-as-cols="advFilterHideLabelsAsCols"
:adv-filter-prevent-filtering-labels="advFilterPreventFilteringLabels"
:key-field="keyField"
:sortGeneration="sortGeneration"
Copy link
Member

Choose a reason for hiding this comment

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

this should be kebab case

@eva-vashkevich
Copy link
Member

eva-vashkevich commented Oct 31, 2024

I've tested the original scenario as well as played with namespaces in other areas of rancher and it seems to work but would want someone else to review as well. Also, there is a typo that we should probably fix before merging

Copy link
Member

@mantis-toboggan-md mantis-toboggan-md left a comment

Choose a reason for hiding this comment

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

Code-wise this looks safe enough and I've tried testing both fleet workspace switching and explorer namespace switching.

This solution seems appropriate given time constraints but I agree with @eva-vashkevich about filing a follow-up issue to re-evaluate. I'm wary of leaving the root cause of this bug undiscovered but agree with the assessment that it'd take substantial time to do.

@eva-vashkevich eva-vashkevich merged commit 3387613 into rancher:master Nov 1, 2024
34 checks passed
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.

As custom user incorrect fleet gitrepos are shown given the workspace filter
3 participants