-
Notifications
You must be signed in to change notification settings - Fork 293
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
: Reduce for-dashboard
payload significantly when participation results exist
#6984
Conversation
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.
Code LGTM 👍🏻
Locally it works: no console errors, stats same as on develop
❌ Unable to deploy to test servers ❌The docker build needs to run through before deploying. |
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.
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 TS5. The course statistics page works as before 👍. There are no Typescript console errors related to this PR.
Code also LGTM.
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.
Aprove code
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.
Code looks good to me. This will most likely also require changes within our native clients.
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 ts5.
No typescript errors are shown in the console and statistics are correct.
@terlan98 Do we need changes directly in Themis or only in the artemis-ios-core-modules? |
Themis is not affected by the changes on this PR. I'll just apply this change on ArtemisCore. |
…ation results exist (#6984)
Checklist
General
Server
Client
Motivation and Context
For each participation result, the
for-dashboard
REST call includes the whole data again, because of the chain "result -> participation -> exercise -> course -> exercises -> studentParticipations -> submissions -> results". In the worst case, this could result in a 80x larger payload in case a student participated in 80 course exercises.Description
I created a new DTO because basically just 3 values are needed in the client: score, rated, participationId
Steps for Testing
develop
)Review Progress
Performance Review
Code Review
Manual Tests