ROX-30334: Rely on context instead of redux for usePermissions hook#16182
Merged
ROX-30334: Rely on context instead of redux for usePermissions hook#16182
Conversation
Contributor
Author
|
This change is part of the following stack: Change managed by git-spice. |
|
Skipping CI for Draft Pull Request. |
Contributor
|
Images are ready for the commit at 4c6195f. To use with deploy scripts, first |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #16182 +/- ##
==========================================
- Coverage 48.71% 48.71% -0.01%
==========================================
Files 2606 2606
Lines 191771 191771
==========================================
- Hits 93418 93415 -3
- Misses 91024 91025 +1
- Partials 7329 7331 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
9 tasks
5ecd4b9 to
1c986d8
Compare
e2497b3 to
286e5b2
Compare
9 tasks
Contributor
There was a problem hiding this comment.
Hey @dvail - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `ui/apps/platform/src/Containers/ReduxUserPermissionProvider.tsx:18` </location>
<code_context>
+});
+
+export default function ReduxUserPermissionProvider({ children }: { children: React.ReactNode }) {
+ const { userRolePermissions, isLoadingPermissions } = useSelector(stateSelector);
+
+ return (
</code_context>
<issue_to_address>
No memoization for context value may cause unnecessary re-renders.
Consider using `useMemo` to memoize the context value and prevent unnecessary re-renders of consumers, especially if the provider wraps large parts of the app.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
sachaudh
approved these changes
Jul 28, 2025
Contributor
sachaudh
left a comment
There was a problem hiding this comment.
LGTM 👍🏼 Minor comments about thoughts and suggestions.
ui/apps/platform/src/Containers/ReduxUserPermissionProvider.tsx
Outdated
Show resolved
Hide resolved
b57a431 to
323f0cf
Compare
286e5b2 to
4c6195f
Compare
Base automatically changed from
dv/ROX-29844-add-configuration-of-base-url-and-e2e-requests
to
master
July 31, 2025 11:54
9 tasks
Stringy
pushed a commit
that referenced
this pull request
Aug 15, 2025
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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
This replaces the direct usage of redux
selectorsin theusePermissions()hook with a call to a context instead. A context provider is added at the top level of the application that will use redux and sagas to handle permissions as they were previously handled in theusePermissions()hook.This will allow us to easily have a separate context provider in the OCP plugin that has user permission information without needing to integrate with redux.
User-facing documentation
Testing and quality
Automated testing
How I validated my change
Smoke test and manual verification that
mypermissionsendpoint is only ever called once when navigating around the application.CI e2e tests should be sufficient to catch edge cases.