Remove repo secrets from redux state, fetch on mount #4028
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
antgamdia
approved these changes
Jan 5, 2022
| @@ -75,6 +75,7 @@ | |||
| "devDependencies": { | |||
| "@formatjs/cli": "^4.7.0", | |||
| "@improbable-eng/grpc-web-fake-transport": "^0.15.0", | |||
| "@testing-library/react": "^12.1.2", | |||
There was a problem hiding this comment.
Adding the testing library seems to be the first step towards getting rid of Enzyme: https://testing-library.com/docs/react-testing-library/migrate-from-enzyme :P
| wrapper = mountWrapper(defaultStore, <AppRepoForm {...defaultProps} repo={repo} />); | ||
| }); | ||
|
|
||
| await waitFor(() => { |
There was a problem hiding this comment.
Way simpler than the tricky approach with jest.runAllTimers() we still have in some places.
Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
absoludity
force-pushed
the
3922-app-repo-secrets-workaround-2
branch
from
772011e
to
ef86572
Compare
This was referenced Jan 6, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of the change
Following on from #4027, this PR updates the AppRepoForm component so that it no longer receives a related secret as a prop (from the redux state?), but rather fetches the secret when mounting and uses the local component state.
Benefits
The new endpoint for fetching a secret for a repo's secret is used (rather than the k8s API), and it avoids the need to update the redux state when a repo is updated (since we always fetch the secret when loading the form). Also avoids storing all repo secrets in the redux state (and removes the related actions/reducers).
Possible drawbacks
Applicable issues
Additional information
As always, testing the combination of an async react hook (useEffect) turned out to be painful to realise, but got there after a lot of head scratching.
I still need to test this IRL locally before landing (once CI passes etc.)