Skip to content

Conversation

@Supun-VinodIT
Copy link

@Supun-VinodIT Supun-VinodIT commented Jan 17, 2026

Description

This PR completes the migration of the Projects Index state from AngularJS to Angular.
The previous implementation relied on an AngularJS state, controller, and template.
As part of the migration effort, I created a new Angular component and moved all
relevant logic, routing, and view behavior into the Angular framework.

I also cleaned up the old AngularJS state by disabling it so that the new Angular
component is now the source of truth for the Projects Index page.

This PR contains only the intended migration files.

Fixes # (issue) N/A

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

How Has This Been Tested?

  1. Starting the frontend application.
  2. Logging in with default credentials.
  3. Navigating to /projects/<validProjectId> and confirming the page loads.
  4. Confirming the Angular component renders correctly.
  5. Verifying that the project and unit data appear as expected.
  6. Checking the browser console for errors — none found.
  7. Confirming that the old AngularJS controller is no longer triggered.

Testing Checklist:

  • Tested in latest Chrome
  • Tested in latest Safari
  • Tested in latest Firefox

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have requested a review

##Screenshots (Evidence)

Screenshot 1 - Projects Index Page Rendering
1  Projects Index Page Rendering

Shows the page loading at /projects/ with correct layout and navigation. Confirms that the new Angular component is rendering successfully and the user interface is visible.

Screenshot 2 - Console Showing No Errors
2  Console Showing No Errors

Displays the browser’s DevTools Console tab after refreshing the page. Confirms that there are no red errors or warnings related to the migrated component. Only expected optimization warnings from legacy modules are present.

Screenshot 3 - Network Tab Showing Successful API Calls
3  Network tab showing successful API calls

Shows the DevTools Network tab after page load. Confirms that getProject and getUnit API calls return 200 OK, verifying that project and unit data are loading correctly.

Screenshot 4 - Angular Component Folder Structure
4  Angular component folder structure

Displays the file explorer in VS Code showing the new Angular component files inside src/app/projects/states/index/. Includes:

  • index.component.ts
  • index.component.html
  • index.component.scss
    Confirms that the migration was scoped and structured correctly.

Screenshot 5 - Routing Configuration
5  Updated routing configuration

Shows the route definition in app-routing.module.ts for projects/:projectId. Confirms that the new Angular component is registered with Angular Router and replaces the legacy AngularJS state.

How the behaviour can be tested / replicated by others

Anyone reviewing this PR can follow the steps below to confirm the migrated Projects Index state works as expected:

  1. Start the frontend application
    Launch the Angular development server using the project’s standard startup command.

  2. Sign in using any valid development credentials
    Once the app loads, log in with the default test credentials provided in the project documentation.

  3. Navigate to the Projects Index route
    In the browser, go to:

    http://localhost:4200/projects/<validProjectId>
    

    Replace <validProjectId> with a project ID that exists in your local database.

  4. Confirm the page renders correctly
    The Projects Index page should load without errors.
    If the user isn’t enrolled in a unit, the “not enrolled” message will appear - this is expected behaviour.

  5. Check the browser console
    Open DevTools → Console and refresh the page.
    There should be no red errors.
    Any AngularJS optimisation warnings are normal and unrelated to this migration.

  6. Check the Network tab
    Open DevTools → Network and refresh the page.
    API calls such as getProject and getUnit should return successful responses (e.g., 200 OK).

  7. Verify routing behaviour

    • A valid project ID should load the Angular component.
    • An invalid project ID should redirect the user to /home.
  8. Confirm the old AngularJS controller is no longer used
    The legacy ProjectsIndexStateCtrl should not appear in logs or be triggered during navigation.

@Supun-VinodIT Supun-VinodIT changed the title feat(projects): restore Projects Index migration files feature/projects-index-state-migration Jan 17, 2026
Copy link

@31Husain31 31Husain31 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reviewed the code changes and have some feedback:

Amazing things I noticed:

  • Really like the proper RxJS patterns with takeUntil for cleanup - good memory management
  • Error handling with the isLoading and hasError states is solid
  • The switchMap pattern for handling route params is the right approach
  • Using OnDestroy to clean up subscriptions properly

Concerns/Questions:

  1. Template Issue (High Priority):
    Looking at index.component.html, I see the <ng-template #loadingOrError> is defined twice (lines 37-41 and 42-45). That's going to cause issues. Should probably remove one of those duplicates.

  2. UI/UX:
    The new component has a "Complete" button for tasks (completeTask(task)) but I don't see that method implemented in the .ts file. Is that missing or is the template from a different version?
    The new UI looks quite different from the old AngularJS version (which reused units/states/index/index.tpl.html). Was this intentional? Should we keep the same UI for consistency?

  3. Service Injection:
    I see you're injecting ProjectService and ListenerService but the ListenerService doesn't seem to be used anywhere in the component. Is that needed?

  4. Type Safety:
    project: any and unit: any - could we create proper interfaces for these? Would make the code more maintainable
    Same with the ViewType.PROJECT - good that you're using an enum, but worth checking if project type aligns with what setView expects

  5. Empty Observable:
    When there's no valid projectId, you're returning new Subject() but never completing it. Would EMPTY from rxjs be cleaner here?

Minor Issues:

The .scss file is empty - do we need it at all?
The old CoffeeScript controller had roleWhitelist: ['Student', 'Tutor', 'Convenor', 'Admin', 'Auditor'] - how is this being handled in the new Angular routing? Route guards?

@Supun-VinodIT
Copy link
Author

I've reviewed the code changes and have some feedback:

Amazing things I noticed:

* Really like the proper RxJS patterns with takeUntil for cleanup - good memory management

* Error handling with the isLoading and hasError states is solid

* The switchMap pattern for handling route params is the right approach

* Using OnDestroy to clean up subscriptions properly

Concerns/Questions:

1. Template Issue (High Priority):
   Looking at index.component.html, I see the <ng-template #loadingOrError> is defined twice (lines 37-41 and 42-45). That's going to cause issues. Should probably remove one of those duplicates.

2. UI/UX:
   The new component has a "Complete" button for tasks (completeTask(task)) but I don't see that method implemented in the .ts file. Is that missing or is the template from a different version?
   The new UI looks quite different from the old AngularJS version (which reused units/states/index/index.tpl.html). Was this intentional? Should we keep the same UI for consistency?

3. Service Injection:
   I see you're injecting ProjectService and ListenerService but the ListenerService doesn't seem to be used anywhere in the component. Is that needed?

4. Type Safety:
   project: any and unit: any - could we create proper interfaces for these? Would make the code more maintainable
   Same with the ViewType.PROJECT - good that you're using an enum, but worth checking if project type aligns with what setView expects

5. Empty Observable:
   When there's no valid projectId, you're returning new Subject() but never completing it. Would EMPTY from rxjs be cleaner here?

Minor Issues:

The .scss file is empty - do we need it at all? The old CoffeeScript controller had roleWhitelist: ['Student', 'Tutor', 'Convenor', 'Admin', 'Auditor'] - how is this being handled in the new Angular routing? Route guards?

Thanks for the detailed review - all high‑priority items have been addressed:

  • Removed the duplicate <ng-template #loadingOrError> block
  • Removed unused ListenerService injection
  • Added minimal Project and Unit interfaces to replace any types
  • Replaced new Subject() with EMPTY for cleaner observable handling
  • Removed the unused “Complete” button from the template to match the original AngularJS UI
  • Cleaned up the template and TypeScript logic accordingly
  • Minor Issues - The old roleWhitelist is handled through Angular route guards in the new architecture.
    This component doesn’t contain its own routing, and I couldn’t find the route definition for ProjectsIndexComponent in this feature folder. Once I confirm where the route is registered, I can add the equivalent canActivate guard with the same roles.

Please let me know if you'd like any further adjustments.

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.

2 participants