-
Notifications
You must be signed in to change notification settings - Fork 77
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
Add live feedback storage and statistics #7497
base: master
Are you sure you want to change the base?
Conversation
889de1f
to
389bdac
Compare
389bdac
to
063e585
Compare
302abf2
to
fb7f80f
Compare
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.
Hi @syoopie good job on your PR! It's quite well made..
I have left some comments inside this PR, please attend to them and apply the changes accordingly. If you have any further inquiries, please reply to my comments and if you don't understand how to apply those changes, please don't hesitate to reach out to me
Other than that, here are some advise for your future PRs:
- please properly name your commit. We have the convention written in our Commit Guideline. Not sure if you have read this before, but please learn about this
- avoid creating a new commit that are actually merely fixing the commit you just made previously that are still within the same PR. If you have committed several more commits then realising your mistake, instead do
git rebase
and thenedit
the respective commit.. Please take a look at the following git rebase guideline to know how to use git rebase properly
Please let me know when you're finished polishing your code, and then request for my re-review for me to look more into your PR
app/views/course/statistics/assessments/main_statistics.json.jbuilder
Outdated
Show resolved
Hide resolved
...les/course/assessment/pages/AssessmentStatistics/LiveFeedbackHistory/LiveFeedbackDetails.tsx
Outdated
Show resolved
Hide resolved
...course/assessment/pages/AssessmentStatistics/LiveFeedbackHistory/LiveFeedbackHistoryPage.tsx
Outdated
Show resolved
Hide resolved
0f76c0b
to
d5e7640
Compare
app/services/course/assessment/answer/programming_codaveri_async_feedback_service.rb
Show resolved
Hide resolved
client/app/bundles/course/assessment/pages/AssessmentStatistics/LiveHelpStatisticsTable.tsx
Outdated
Show resolved
Hide resolved
bd8c12a
to
85bdef9
Compare
client/app/bundles/course/assessment/pages/AssessmentStatistics/LiveHelpStatisticsTable.tsx
Outdated
Show resolved
Hide resolved
client/app/bundles/course/assessment/pages/AssessmentStatistics/LiveHelpStatisticsTable.tsx
Outdated
Show resolved
Hide resolved
client/app/bundles/course/assessment/pages/AssessmentStatistics/LiveHelpStatisticsTable.tsx
Outdated
Show resolved
Hide resolved
85bdef9
to
e1cb9b4
Compare
@answer.actable.files | ||
) | ||
|
||
if response_status == 200 |
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.
@syoopie please attend to this review. Btw, just now I tried to comment out that line save_live_feedback
and the API in generating live feedback still runs as expected (when you try to generate live feedback, that generated feedback will be saved accordingly to our DB and the count is increased by 1)
Therefore, please investigate once more if the line 116 is needed here.
@@ -318,6 +321,11 @@ export function fetchLiveFeedback({ | |||
.fetchLiveFeedback(feedbackUrl, feedbackToken) | |||
.then((response) => { | |||
if (response.status === 200) { | |||
CourseAPI.assessment.submissions.saveLiveFeedback( |
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.
I mean this one also calls that controller function save_live_feedback
, so with having that save_live_feedback
on line 116 of course/assessment/submission/submissions_controller.rb
, technically this function is called twice. I don't see the reason why this should happen
On the other hand, deactivating this API call while keeping that line 116 intact makes the Get Help count to be stagnant (not increased) when Get Help is clicked. Please investigate further and explain why this occurs
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.
Backend only does the saving when the feedback is immediately returned from codaveri.
The only case when frontend does saving is when it needs to poll codaveri. In this scenario backend does not save the feedback (response status will be 201)
client/app/bundles/course/assessment/pages/AssessmentStatistics/AssessmentStatisticsPage.tsx
Outdated
Show resolved
Hide resolved
client/app/bundles/course/assessment/pages/AssessmentStatistics/utils.js
Outdated
Show resolved
Hide resolved
0776582
to
39254a6
Compare
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.
Okay nice! @cysjonathan I think from me, the review's done for this PR. Please review it once more and once you approve this, should be good to merge
43e0ae0
to
c85a081
Compare
New Tables: 1. course_assessment_live_feedbacks 2. course_assessment_live_feedback_code 3. course_assessment_live_feedback_comments Related models have also been updated. These include: users course_assessments course_assessment_questions
add new route for getting live feedback stats add new live_feedback_concern refactored some code in assessments_controller as it was used for live feedback stats as well add new json return format for live feedback stats
refactored for a wider range of usability fix erroneous comments add new function to get class for live feedback cells previously cells were coloured linearly based on a max number new colouring only applies this gradient to the lower 75% of the data upper 25% is all coloured the most intense shade of red
add new tab to assessment statistics if live help is enabled for assessment add new types for live feedback statistics add call to new route for getting live feedback statistics refactor functionality for getting jointGroupsName refactor functionality for translating status both refactors have their commented out typescript equivalent left inside in file
this commit used to be to fix a breaking change from codaveri regarding categories, but the change has been made by another commit that is already merged the change has been removed from this commit during rebase and only other random changes are left remove some random commented code in codaveri-async-feedback-service
add frontend logic for returning live feedback once retrieved add backend logic and routes for saving live feedback
individual live feedback histories need this information to be retrieved
add dialog to show live feedback history on the live feedback statistics page add routes and backend for retrieving live feedback history
Change assessments factory to give unique question weights when creating questions This is to assist in live feedback tests as weights are needed to order the questions
abstract out the translations of some assessment stsatistics files add translations and localizations for the associated translations
- conform to new standard, all user facing "verbs" are "Get Help" - "Get Help" will describe the act of getting live feedback - all other usage of the terms "Live Help" and "Get Help" will instead be "Live Feedback"
add custom slider with more visible marks and larger height
- sql queries drop from 99 -> ~30
c85a081
to
4035cd4
Compare
Add support for saving live feedback and displaying it in the assessment statistics page.
Workflow for saving:
course_assessment_live_feedbacks
is created and current code states (course_assessment_live_feedback_code
) are savedcourse_assessment_live_feedback_comments
)course_assessment_live_feedback_comments
)Schema Changes: Added 3 new tables
feedback_id
under thecourse_assessments_live_feedbacks
table corresponds to thetransaction_id
provided by codaveri. This is to ensure traceability.The schema is separated as such so that each feedback request can accomadate multiple different code files, and each code file can have multiple comments.
New Live Feedback statistics page:
New Live Feedback History Dialog
Other Notes:
This PR also standardises the terminology surrounding the live feedback function.