feat: refactor notices provider to useQuery #992
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/react-query-route-loaders #992 +/- ##
==================================================================
+ Coverage 81.15% 81.25% +0.09%
==================================================================
Files 372 371 -1
Lines 7776 7773 -3
Branches 1889 1888 -1
==================================================================
+ Hits 6311 6316 +5
+ Misses 1407 1401 -6
+ Partials 58 56 -2 ☔ View full report in Codecov by Sentry. |
brobro10000
force-pushed
the
hu/ent-8475
branch
2 times, most recently
from
387c003 to
73b84ed
Compare
| @@ -364,6 +365,33 @@ export async function fetchSubscriptions(enterpriseUuid) { | |||
| }; | |||
| } | |||
|
|
|||
| // Notices | |||
| export const fetchNotices = async () => { | |||
| const authenticatedUser = getAuthenticatedUser(); | |||
There was a problem hiding this comment.
[question] Is this check on whether the user is authenticated necessary given we shouldn't be getting to this code path for unauthenticated users? For example, all the route loaders have
const authenticatedUser = await ensureAuthenticatedUser(requestUrl, params);
// User is not authenticated, so we can't do anything in this loader.
if (!authenticatedUser) {
return null;
}...to shortcut the execution of the remainder of the loader function given the rest of the loader function would fail without authentication.
Given that, could we always assume users are authenticated within this function?
There was a problem hiding this comment.
Good call out, I used the existing notices API without changing it so a lot of these nits are around the original implementation. Fixing it up now 👍🏽
* feat: refactor notices provider to useQuery * chore: merge fix * chore: cleanup * fix: redirects without momentary render * fix: debugging * chore: Testing and cleanup * chore: PR feedback
Refactors the NoticesProvider context to the loader. It conditionally checks whether the loader should include the API call to retrieve notices from platform-plugin-notices before it adds it to the rootLoader to be called.
The redirect is done directly within the fetch call. The reason for the implementation is to prevent the momentary render of the learner portal before redirect.
For all changes
Only if submitting a visual change