Skip to content

Conversation

foot
Copy link
Collaborator

@foot foot commented Sep 13, 2022

  • Refactors Clusters and Templates contexts into hooks
    • As the selectedClusters state (which clusters to delete) is no longer in a common context between the deleteClusterDialog and the clustersList we do a small refactor and clear the selectedClusters onDialogClose instead of having the Dialog do that itself.

Fixes #1422

To test:

Pre-reqs:

Test

  • Login to https://35.188.40.143:30080/ with OIDC + gitlab, navigate to apps/sources, wait ~20s, see the error toast about no gitopscluster/capitemplates permissions.
  • Checkout this branch locally and run: PROXY_HOST=https://35.188.40.143:30080/ yarn start
  • Login with OIDC + gitlab, The redirect will be incorrect and needs correction, when you see an error in the browser, edit the URL manually, replacing https://35.188.40.143:30080/ with http://localhost:3000 and hit enter and you should be logged in locally.
  • Notice no more errors on the apps/sources page.

@foot foot added the bug Something isn't working label Sep 13, 2022
@foot foot changed the title Don't show clusters and templates permission erros if viewing apps/sources Don't show clusters and templates permission errors if viewing apps/sources Sep 13, 2022
@foot foot requested a review from ahussein3 September 13, 2022 11:49
@ahussein3
Copy link
Contributor

LGTM. Well done now we have reduced the API calls by 2
💯

@ahussein3
Copy link
Contributor

@foot the only tiny thing I would suggest is to find better name for onRequestClose,handleRequestClose for maybe something like ResetDialogOnClose. as RequestClose is like confusing as there is no actual request. just my opinion.

@foot
Copy link
Collaborator Author

foot commented Sep 14, 2022

Great point, I was trying to be consistent w/ mui but got that wrong 😅 . They just call it onClose https://v4.mui.com/api/dialog/ will do that.

@foot foot merged commit b2cecc5 into main Sep 14, 2022
@foot foot deleted the fixes-errors branch September 14, 2022 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dashboard keeps on showing unnecessary errors for resources a tenant has no permissions
2 participants