Skip to content
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

Development: Update to Angular 19, standalone and inject() #10112

Merged
merged 431 commits into from
Jan 13, 2025

Conversation

krusche
Copy link
Member

@krusche krusche commented Jan 6, 2025

Checklist

General

Client

  • Important: I implemented the changes with a very good performance, prevented too many (unnecessary) REST calls and made sure the UI is responsive, even with large data (e.g. using paging).
  • I strictly followed the principle of data economy for all client-server REST calls.
  • I strictly followed the client coding and design guidelines.
  • Following the theming guidelines, I specified colors only in the theming variable files and checked that the changes look consistent in both the light and the dark theme.
  • I added multiple integration tests (Jest) related to the features (with a high test coverage), while following the test guidelines.
  • I added authorities to all new routes and checked the course groups for displaying navigation elements (links, buttons).
  • I documented the TypeScript code using JSDoc style.
  • I added multiple screenshots/screencasts of my UI changes.
  • I translated all newly inserted strings into English and German.

Changes affecting Programming Exercises

  • High priority: I tested all changes and their related features with all corresponding user types on a test server configured with the integrated lifecycle setup (LocalVC and LocalCI).
  • I tested all changes and their related features with all corresponding user types on a test server configured with Gitlab and Jenkins.

Motivation and Context

We want to update our client code the latest and greatest Angular version and profit from new features and optimizations. As a first step, this PR updates to Angular 19, converts all Angular components to standalone and to use inject() instead of constructor injection

Description

  1. I used ng update to migrate the client app to Angular 19
  2. I used https://angular.dev/reference/migrations/standalone to migrate all components to standalone
  3. I used https://angular.dev/reference/migrations/inject-function to migrate everything to inject() instead of constructor injection
  4. I used https://angular.dev/reference/migrations/route-lazy-loading to automatically create lazy routes
  5. I deactivated jhiExtensionPoints for Orion, because they currently do no seem to work.
  6. In between, I fixed all compile and test compile and lint errors, removed unused code and optimized some code places

Right now, many client tests fail, but the main functionality of the client app seems to work, at least some basic testing looked promising. I believe it will be easier to collectively fix the issues in the functionality as well as the client tests in this PR instead of having many small PRs that deal with similar issues during the migration and manually migrate aspects that could be done automatically

What is missing for a complete transition:

However, I suggest to apply those improvements in follow-up PRs to avoid

Steps for Testing

Test everything in the app and make sure it works identical to develop

Testserver States

Note

These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.







Review Progress

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Exam Mode Test

  • Test 1
  • Test 2

@krusche krusche requested a review from a team as a code owner January 6, 2025 20:41
@github-actions github-actions bot added tests client Pull requests that update TypeScript code. (Added Automatically!) labels Jan 6, 2025
Copy link
Contributor

@ole-ve ole-ve left a comment

Choose a reason for hiding this comment

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

reviewed the account and admin components. why are all standalone: true removed? standalone: true is default value in angular 19

@KonstiAnon
Copy link
Contributor

reviewed the account and admin components. why are all standalone: true removed? standalone: true is default value in angular 19

I have noticed that the MockBuilder requires standalone components to be marked with: standalone: true, or it won't recognize them as standalone. I'm not sure why this issue exists, but wanted to point it out in case someone else runs into the same issue.

@krusche
Copy link
Member Author

krusche commented Jan 7, 2025

reviewed the account and admin components. why are all standalone: true removed? standalone: true is default value in angular 19

I have noticed that the MockBuilder requires standalone components to be marked with: standalone: true, or it won't recognize them as standalone. I'm not sure why this issue exists, but wanted to point it out in case someone else runs into the same issue.

I created a temporary patch based on help-me-mom/ng-mocks#10583 which should fix certain test issues related to standalone withng-mocks. Locally, this increased the passed client tests from 2591 (30%) to 5205 (61%).

So please pull the latest changes ;-) and make sure to invoke `npm install?

@magaupp
Copy link
Contributor

magaupp commented Jan 7, 2025

79d2666 removes [ngModelOptions]="{ standalone: true }" from form inputs. This option is not related to standalone components.

See https://angular.dev/api/forms/NgModel#options:

standalone: When set to true, the ngModel will not register itself with its parent form, and acts as if it's not in the form. Defaults to false. If no parent form exists, this option has no effect.

Was this intentional?

@krusche
Copy link
Member Author

krusche commented Jan 7, 2025

79d2666 removes [ngModelOptions]="{ standalone: true }" from form inputs. This option is not related to standalone components.

See https://angular.dev/api/forms/NgModel#options:

standalone: When set to true, the ngModel will not register itself with its parent form, and acts as if it's not in the form. Defaults to false. If no parent form exists, this option has no effect.

Was this intentional?

no, sorry, you can add it again if it makes sense

@krusche
Copy link
Member Author

krusche commented Jan 7, 2025

79d2666 removes [ngModelOptions]="{ standalone: true }" from form inputs. This option is not related to standalone components.
See https://angular.dev/api/forms/NgModel#options:

standalone: When set to true, the ngModel will not register itself with its parent form, and acts as if it's not in the form. Defaults to false. If no parent form exists, this option has no effect.

Was this intentional?

no, sorry, you can add it again if it makes sense

I re-added them

DominikRemo
DominikRemo previously approved these changes Jan 13, 2025
Copy link
Contributor

@DominikRemo DominikRemo left a comment

Choose a reason for hiding this comment

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

Tested in testing session on ts3. Complaints and manual assessments work as they used to.
This approval is limited to complaints and manual assessments.

BBesrour
BBesrour previously approved these changes Jan 13, 2025
Copy link
Member

@BBesrour BBesrour left a comment

Choose a reason for hiding this comment

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

Tested on TS3, tested build overview, build agents, programming exercises creation and participation

Copy link
Contributor

@ole-ve ole-ve left a comment

Choose a reason for hiding this comment

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

Tested on TS3 and TS1 and found a UI glitch regarding the import competency/prerequisite-buttons, described on the Confluence Page.

@ole-ve ole-ve dismissed stale reviews from BBesrour and DominikRemo via 047a3b4 January 13, 2025 18:55
@krusche krusche added this to the 7.9.0 milestone Jan 13, 2025
@krusche
Copy link
Member Author

krusche commented Jan 13, 2025

Tested on TS3 and TS1 and found a UI glitch regarding the import competency/prerequisite-buttons, described on the Confluence Page.

Those are small issues. I would suggest to merge this huge PR now and fix those issues quickly in follow-up PRs before we deploy the next release

Copy link
Contributor

@SimonEntholzer SimonEntholzer left a comment

Choose a reason for hiding this comment

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

Tested programming exercise related pages on Ts3

Copy link
Contributor

@asliayk asliayk left a comment

Choose a reason for hiding this comment

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

tested communication module on ts3

Copy link
Contributor

@coolchock coolchock left a comment

Choose a reason for hiding this comment

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

tested exam mode on ts3 (regular, test, test Run, all exercise types, events, assessment, complaints, grading, import, archive, scores, checklist). No issues found.

Copy link
Contributor

@muradium muradium left a comment

Choose a reason for hiding this comment

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

Tested on TS3 during testing session - text, quiz, modeling and file upload exercises behave as expected

Copy link
Contributor

@PaRangger PaRangger left a comment

Choose a reason for hiding this comment

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

Tested Communication Module in Testing Session.

Copy link
Contributor

@az108 az108 left a comment

Choose a reason for hiding this comment

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

Tested the Tutorial Groups feature on TS3 in the testing session and locally. No major issues found, only one small UI issue which can be fixed in a follow up. I noted this down on the respective confluence page 👍

Copy link
Contributor

@raffifasaro raffifasaro left a comment

Choose a reason for hiding this comment

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

Tested exercise file upload, competencies and learning paths on TS1 in testing session

@krusche krusche merged commit 43d0089 into develop Jan 13, 2025
33 of 40 checks passed
@krusche krusche deleted the chore/update-angular19 branch January 13, 2025 21:52
Copy link

@Cathy0123456789 Cathy0123456789 left a comment

Choose a reason for hiding this comment

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

Tested Communication Module & Quiz in Testing Session

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Pull requests that update TypeScript code. (Added Automatically!) priority:high ready for review tests
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.