-
Notifications
You must be signed in to change notification settings - Fork 11
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
feature: add new event ResultTestVariablesAfterTransmissionEvent #2425
feature: add new event ResultTestVariablesAfterTransmissionEvent #2425
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #2425 +/- ##
=============================================
- Coverage 16.94% 16.88% -0.06%
- Complexity 3203 3222 +19
=============================================
Files 208 209 +1
Lines 11542 11594 +52
=============================================
+ Hits 1956 1958 +2
- Misses 9586 9636 +50 ☔ View full report in Codecov by Sentry. |
Can you please add test coverage to the new code created? |
Front-end summary Node 18
|
…ssionEventHandler
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.
- New code is covered by tests (if applicable)
- Tests are running successfully (old and new ones) on my local machine (if applicable)
- New code is respecting code style rules
- New code is respecting best practices
- New code is not subject to concurrency issues (if applicable)
- Feature is working correctly on my local machine (if applicable)
- Acceptance criteria are respected
- Pull request title and description are meaningful
models/classes/eventHandler/ResultTransmissionEventHandler/ResultTransmissionEventHandler.php
Outdated
Show resolved
Hide resolved
models/classes/eventHandler/ResultTransmissionEventHandler/ResultTransmissionEventHandler.php
Show resolved
Hide resolved
Version
There are 0 BREAKING CHANGE, 3 features, 4 fixes |
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.
- New code is covered by tests (if applicable)
- Tests are running successfully (old and new ones) on my local machine (if applicable)
- New code is respecting code style rules
- New code is respecting best practices
- New code is not subject to concurrency issues (if applicable)
- Feature is working correctly on my local machine (if applicable)
- Acceptance criteria are respected
- Pull request title and description are meaningful
- Pull request's target is not
master
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.
LGTM, but not tested.
* feature: add new event ResultTestVariablesAfterTransmissionEvent * feature: rewrite LtiAgsListener * fix: codestyle * fix: codestyle * fix: code review * feature: change to TestVariablesRecorded, and use it in ResultTransmissionEventHandler * fix: code review fixes (cherry picked from commit e67ac34)
feature: add new event ResultTestVariablesAfterTransmissionEvent (#2425)
* feature: add new event ResultTestVariablesAfterTransmissionEvent * feature: rewrite LtiAgsListener * fix: codestyle * fix: codestyle * fix: code review * feature: change to TestVariablesRecorded, and use it in ResultTransmissionEventHandler * fix: code review fixes (cherry picked from commit e67ac34)
Ticket: https://oat-sa.atlassian.net/browse/INF-235
What's Changed
Screenshare.-.2023-12-06.10_31_04.AM.mp4