-
Notifications
You must be signed in to change notification settings - Fork 313
Exam mode
: Migrate participate module to signals
#10456
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: develop
Are you sure you want to change the base?
Exam mode
: Migrate participate module to signals
#10456
Conversation
End-to-End (E2E) Test Results Summary
|
End-to-End (E2E) Test Results Summary
|
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.
Tested on TS3. Everything went smoothly in the first attempt of an exam. I noticed something odd when attempting an exam the second time. I couldn't submit the programming exercise, I either had a 404 or 500 error. The same thing happened for follow up attempts.
Also the number of attempts is not updated correctly. I have made 2 attempts but the attempts count is still 0.
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.
Please fix the outcommented code in src/main/webapp/app/exam/overview/exam-cover/exam-participation-cover.component.spec.ts
- I think it can be removed?
Could we also align the usage of update
vs set
?
https://angular.dev/guide/signals#writable-signals
Besides that the changes look fine to me
src/main/webapp/app/exam/overview/events/button/exam-live-events-button.component.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/exam/overview/events/overlay/exam-live-events-overlay.component.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/exam/overview/exam-cover/exam-participation-cover.component.spec.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/exam/overview/exam-navigation-sidebar/exam-navigation-sidebar.component.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/exam/overview/exam-navigation-sidebar/exam-navigation-sidebar.component.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/exam/overview/exam-navigation-bar/exam-navigation-bar.component.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/exam/overview/exam-navigation-bar/exam-navigation-bar.component.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/exam/overview/events/button/exam-live-events-button.component.ts
Outdated
Show resolved
Hide resolved
TestBed.runInInjectionContext(() => { | ||
component.exam = input(exam); | ||
component.studentExam = input(studentExam); | ||
}); |
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.
Why don't we do this in the @beforeEach
?
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.
done
...main/webapp/app/exam/overview/general-information/exam-general-information.component.spec.ts
Outdated
Show resolved
Hide resolved
End-to-End (E2E) Test Results Summary
|
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.
Approve code
There are still two outcommented lines of code, but no dealbreaker
src/main/webapp/app/exam/overview/exam-cover/exam-participation-cover.component.spec.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/exam/overview/exam-cover/exam-participation-cover.component.spec.ts
Outdated
Show resolved
Hide resolved
5ec982c
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.
Thanks for the adjustments, the code looks good to me! 😄
End-to-End (E2E) Test Results Summary
|
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.
Checklist
General
Client
Motivation and Context
Description
This is the final pull request that completes the migration of the exam mode to use signals.
Steps for Testing
Prerequisites:
Testserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Performance Review
Code Review
Manual Tests
Summary by CodeRabbit
Refactor
Tests