Refactor Academy router

RichDom2185 commented 2 months ago

Description

Create GuardedRoute class for builder pattern
Reformat app router config for readability
Migrate Academy routes to use new Data Loader API
Use lazy loading + code-split Academy routes for performance #2499
Refactor routing logic (assessment configurations, not found behavior)

Please refer to commit history for details.

See #2507 See #2484

TODO:

[x] Fix infinite render update loop when visiting admin panel
[x] Fix tests (tried following https://reactrouter.com/en/main/routers/create-memory-router without luck)
[x] Potentially refactor routerConfig.tsx to use GuardedRoute where possible

Type of change

[ ] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] This change requires a documentation update
[x] Code quality improvements

How to test

Everything should still work.

Checklist

[x] I have tested this code
[ ] I have updated the documentation

coveralls commented 1 month ago

Pull Request Test Coverage Report for Build 8971102719

Details

22 of 116 (18.97%) changed or added relevant lines in 20 files are covered.
3 unchanged lines in 2 files lost coverage.
Overall coverage decreased (-0.08%) to 32.144%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/commons/assessment/Assessment.tsx	3	4	75.0%
src/pages/academy/Academy.tsx	0	1	0.0%
src/pages/academy/adminPanel/AdminPanel.tsx	0	2	0.0%
src/pages/academy/dashboard/Dashboard.tsx	0	2	0.0%
src/pages/academy/gameSimulator/GameSimulator.tsx	0	2	0.0%
src/pages/academy/grading/Grading.tsx	0	2	0.0%
src/pages/academy/groundControl/GroundControlContainer.ts	0	2	0.0%
src/pages/academy/notiPreference/NotiPreference.tsx	0	2	0.0%
src/pages/academy/sourcereel/Sourcereel.tsx	0	2	0.0%
src/pages/academy/teamFormation/TeamFormation.tsx	0	2	0.0%
<!--	Total:	22	116	18.97%	-->

Files with Coverage Reduction	New Missed Lines	%
src/pages/academy/Academy.tsx	1	0.0%
src/commons/application/ApplicationWrapper.tsx	2	0.0%
<!--	Total:	3	-->

Totals
Change from base Build 8960018753:	-0.08%
Covered Lines:	5004
Relevant Lines:	14643

💛 - Coveralls

RichDom2185 commented 1 month ago

[x] Fix infinite render update loop when visiting admin panel

[x] Fix tests (tried following https://reactrouter.com/en/main/routers/create-memory-router without luck)

[x] Potentially refactor routerConfig.tsx to use GuardedRoute where possible

@chownces I've fixed the infinite rerender issue, and also refactored ensureUserAndRole to use GuardedRoute, thereby eliminating the dependency on role. I think in the future we should move towards a static router (thereby removing the need for UPDATE_REACT_ROUTER action) to decrease complexity – just access the store directly via store.getState() within loader functions whenever we need to have dynamic routing logic.

This will reduce the complexity of the implementation.

RichDom2185 commented 1 month ago

There's currently a bug in the academy routes when the route is non-existent. Instead of 404-ing, the page hangs (likely in an infinite loop).

Try something like http://localhost:8000/courses/11/sourcecastt

Somehow the new 404 behaviour seems to be causing the infinite loop. I've restored the original behaviour of rendering <NotFound /> as a catch-all, instead of redirecting first to a /not_found path, then rendering the component there. This fixes the issue.

source-academy / frontend