Skip to content

Conversation

@JeffySam
Copy link

Migrated the Units index view from AngularJS to an Angular component.

  • Replaced legacy index controller/template with Angular component
  • Wired component into hybrid AngularJS/Angular module using downgradeComponent
  • Preserved routing and loading behaviour
  • Verified locally via npm start and unit navigation

Copy link

@YG-GOV YG-GOV left a 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.

@ishika021
Copy link

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.

@BrianDangDev
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants