-
Notifications
You must be signed in to change notification settings - Fork 137
refactor(units): migrate units index to Angular component #429
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: development
Are you sure you want to change the base?
refactor(units): migrate units index to Angular component #429
Conversation
YG-GOV
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had a look and this looks good to me. The migration keeps the same loading and routing behaviour and the downgrade setup makes sense.
|
I reviewed the changes in this pull request with a focus on the AngularJS → Angular migration pattern, module wiring, and whether routing/loading behaviour is preserved for the Units “index” state. What changed: This PR migrates the legacy Units index controller/template to a new Angular component (IndexComponent) while keeping the existing UI-Router nested state structure. The Angular component is registered in doubtfire-angular.module.ts as UnitsIndexComponent, ensuring it is declared within the Angular application module. The component is then bridged into the AngularJS layer via doubtfire-angularjs.module.ts by registering an AngularJS directive (fUnitsIndex) using downgradeComponent, which matches the established hybrid migration approach in this codebase. Behaviour preservation: The new Angular template (index.component.html) keeps the same UX flow by displaying a loading notice until unit and (unitRole || project) are available, and then rendering so nested routed content continues to display as before. Logic equivalence / integration: In index.component.ts, the component reads unitId from $stateParams, redirects to home if missing or if the user does not have a role for the unit, waits for GlobalStateService to load, sets the global view to UNIT, and loads unit details and students using existing services (newUnitService, newProjectService). Error handling uses alertService and then redirects back to home, which is consistent with safe failure behaviour for this route. Legacy handoff: The AngularJS template index.tpl.html now includes , which cleanly hands off rendering to the downgraded Angular component without changing the overall routing structure. Testing: A basic creation test (index.component.spec.ts) is included as a scaffold to ensure the component instantiates under Angular testing tools. Overall, it's all good from side. |
|
Hey @JeffySam , there’s an empty file in your commit, and why there’s still a .coffee file. The migration doesn’t look correct to me. Please review your work again, get two more team members to review you changes - please include screesnshots when do code review |
Migrated the Units index view from AngularJS to an Angular component.