Skip to content

Commit

Permalink
add live feedback saving
Browse files Browse the repository at this point in the history
add frontend logic for returning live feedback once retrieved
add backend logic and routes for saving live feedback
  • Loading branch information
syoopie committed Aug 20, 2024
1 parent a0f4c55 commit 063e585
Show file tree
Hide file tree
Showing 11 changed files with 79 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ def initialize_student_hash(students)
students.to_h { |student| [student, nil] }
end

def fetch_hash_for_live_feedback_assessment(submissions, live_feedback_codes, students)
def fetch_hash_for_live_feedback_assessment(submissions, assessment_live_feedbacks, students)
student_hash = initialize_student_hash(students)

Check warning on line 10 in app/controllers/concerns/course/statistics/live_feedback_concern.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/concerns/course/statistics/live_feedback_concern.rb#L10

Added line #L10 was not covered by tests

populate_hash(submissions, student_hash, live_feedback_codes)
populate_hash(submissions, student_hash, assessment_live_feedbacks)
student_hash

Check warning on line 13 in app/controllers/concerns/course/statistics/live_feedback_concern.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/concerns/course/statistics/live_feedback_concern.rb#L12-L13

Added lines #L12 - L13 were not covered by tests
end

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# frozen_string_literal: true
class Course::Assessment::Submission::SubmissionsController < \
class Course::Assessment::Submission::SubmissionsController <
Course::Assessment::Submission::Controller
include Course::Assessment::Submission::SubmissionsControllerServiceConcern
include Signals::EmissionConcern
Expand Down Expand Up @@ -102,9 +102,42 @@ def generate_live_feedback
response_status, response_body = @answer.generate_live_feedback
response_body['feedbackUrl'] = ENV.fetch('CODAVERI_URL')

course_user = CourseUser.find_by(course_id: @assessment.course_id, user_id: @submission.creator_id)
live_feedback = Course::Assessment::LiveFeedback.create_with_codes(

Check warning on line 106 in app/controllers/course/assessment/submission/submissions_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/course/assessment/submission/submissions_controller.rb#L105-L106

Added lines #L105 - L106 were not covered by tests
@submission.assessment_id,
@answer.question_id,
course_user.id,
response_body['transactionId'],
@answer.actable.files
)

if response_status == 200
params[:live_feedback_id] = live_feedback.id
params[:feedback_files] = response_body['data']['feedbackFiles']
save_live_feedback

Check warning on line 117 in app/controllers/course/assessment/submission/submissions_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/course/assessment/submission/submissions_controller.rb#L114-L117

Added lines #L114 - L117 were not covered by tests
end

response_body['liveFeedbackId'] = live_feedback.id

Check warning on line 120 in app/controllers/course/assessment/submission/submissions_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/course/assessment/submission/submissions_controller.rb#L120

Added line #L120 was not covered by tests
render json: response_body, status: response_status
end

def save_live_feedback
live_feedback = Course::Assessment::LiveFeedback.find_by(id: params[:live_feedback_id])
return head :bad_request if live_feedback.nil?

Check warning on line 126 in app/controllers/course/assessment/submission/submissions_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/course/assessment/submission/submissions_controller.rb#L125-L126

Added lines #L125 - L126 were not covered by tests

feedback_files = params[:feedback_files]
feedback_files.each do |file|
filename = file[:path]
file[:feedbackLines].each do |feedback_line|
Course::Assessment::LiveFeedbackComment.create(

Check warning on line 132 in app/controllers/course/assessment/submission/submissions_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/course/assessment/submission/submissions_controller.rb#L128-L132

Added lines #L128 - L132 were not covered by tests
code_id: live_feedback.code.find_by(filename: filename).id,
line_number: feedback_line[:linenum],
comment: feedback_line[:feedback]
)
end
end
end

# Reload the current answer or reset it, depending on parameters.
# current_answer has the most recent copy of the answer.
def reload_answer
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/course/statistics/assessments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,15 @@ def live_feedback_statistics
preload(course: :course_users).first
submissions = Course::Assessment::Submission.where(assessment_id: assessment_params[:id]).

Check warning on line 43 in app/controllers/course/statistics/assessments_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/course/statistics/assessments_controller.rb#L43

Added line #L43 was not covered by tests
preload(creator: :course_users)
live_feedback_codes = Course::Assessment::LiveFeedbackCode.where(assessment_id: assessment_params[:id])
assessment_live_feedbacks = Course::Assessment::LiveFeedback.where(assessment_id: assessment_params[:id])

Check warning on line 45 in app/controllers/course/statistics/assessments_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/course/statistics/assessments_controller.rb#L45

Added line #L45 was not covered by tests

@course_users_hash = preload_course_users_hash(@assessment.course)

Check warning on line 47 in app/controllers/course/statistics/assessments_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/course/statistics/assessments_controller.rb#L47

Added line #L47 was not covered by tests

load_course_user_students_info
create_question_order_hash

Check warning on line 50 in app/controllers/course/statistics/assessments_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/course/statistics/assessments_controller.rb#L49-L50

Added lines #L49 - L50 were not covered by tests

@student_live_feedback_hash = fetch_hash_for_live_feedback_assessment(submissions,

Check warning on line 52 in app/controllers/course/statistics/assessments_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/course/statistics/assessments_controller.rb#L52

Added line #L52 was not covered by tests
live_feedback_codes,
assessment_live_feedbacks,
@all_students)
end

Expand Down
10 changes: 10 additions & 0 deletions client/app/api/course/Assessment/Submissions.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,16 @@ export default class SubmissionsAPI extends BaseAssessmentAPI {
});
}

saveLiveFeedback(submissionId, liveFeedbackId, feedbackFiles) {
return this.client.post(
`${this.#urlPrefix}/${submissionId}/save_live_feedback`,
{
live_feedback_id: liveFeedbackId,
feedback_files: feedbackFiles,
},
);
}

createProgrammingAnnotation(submissionId, answerId, fileId, params) {
const url = `${this.#urlPrefix}/${submissionId}/answers/${answerId}/programming/files/${fileId}/annotations`;
return this.client.post(url, params);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,14 @@ const LiveHelpStatisticsTable: FC<Props> = (props) => {
.sort((a, b) => a.courseUser.name.localeCompare(b.courseUser.name))
.sort(
(a, b) =>
b.liveFeedbackCount.reduce((sum, count) => sum + (count || 0), 0) -
a.liveFeedbackCount.reduce((sum, count) => sum + (count || 0), 0),
(b.liveFeedbackCount?.reduce(
(sum, count) => sum + (typeof count === 'number' ? count : 0),
0,
) ?? 0) -
(a.liveFeedbackCount?.reduce(
(sum, count) => sum + (typeof count === 'number' ? count : 0),
0,
) ?? 0),
)
.sort(
(a, b) =>
Expand Down Expand Up @@ -161,13 +167,9 @@ const LiveHelpStatisticsTable: FC<Props> = (props) => {
},
title: t(translations.questionIndex, { index: index + 1 }),
cell: (datum): ReactNode => {
return typeof datum.liveFeedbackCount?.[index] === 'number' ? (
renderNonNullAttemptCountCell(datum.liveFeedbackCount?.[index])
) : (
<div className="bg-gray-300 p-[1rem]">
<Box>-</Box>
</div>
);
return typeof datum.liveFeedbackCount?.[index] === 'number'
? renderNonNullAttemptCountCell(datum.liveFeedbackCount?.[index])
: null;
},
sortable: true,
csvDownloadable: true,
Expand Down Expand Up @@ -196,7 +198,7 @@ const LiveHelpStatisticsTable: FC<Props> = (props) => {
cell: (datum): ReactNode => {
const totalFeedbackCount = datum.liveFeedbackCount
? datum.liveFeedbackCount.reduce((sum, count) => sum + (count || 0), 0)
: 0;
: null;
return (
<div className="p-[1rem]">
<Box>{totalFeedbackCount}</Box>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { green } from 'theme/colors';
import { AttemptInfo } from 'types/course/statistics/assessmentStatistics';

const redBackgroundColorClassName = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ export function generateLiveFeedback({
type: actionTypes.LIVE_FEEDBACK_REQUEST,
payload: {
questionId,
liveFeedbackId: response.data?.liveFeedbackId,
feedbackUrl: response.data?.feedbackUrl,
token: response.data?.data?.token,
},
Expand All @@ -306,9 +307,11 @@ export function generateLiveFeedback({
}

export function fetchLiveFeedback({
submissionId,
answerId,
questionId,
feedbackUrl,
liveFeedbackId,
feedbackToken,
successMessage,
noFeedbackMessage,
Expand All @@ -318,6 +321,11 @@ export function fetchLiveFeedback({
.fetchLiveFeedback(feedbackUrl, feedbackToken)
.then((response) => {
if (response.status === 200) {
CourseAPI.assessment.submissions.saveLiveFeedback(
submissionId,
liveFeedbackId,
response.data?.data?.feedbackFiles ?? [],
);
handleFeedbackOKResponse({
dispatch,
response,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,11 @@ class VisibleSubmissionEditIndex extends Component {
dispatch,
liveFeedback,
assessment: { questionIds },
match: { params },
} = this.props;

const liveFeedbackId =
liveFeedback?.feedbackByQuestion?.[questionId].liveFeedbackId;
const feedbackToken =
liveFeedback?.feedbackByQuestion?.[questionId].pendingFeedbackToken;
const questionIndex = questionIds.findIndex((id) => id === questionId) + 1;
Expand All @@ -224,9 +227,11 @@ class VisibleSubmissionEditIndex extends Component {
);
dispatch(
fetchLiveFeedback({
submissionId: params.submissionId,
answerId,
questionId,
feedbackUrl: liveFeedback?.feedbackUrl,
liveFeedbackId,
feedbackToken,
successMessage,
noFeedbackMessage,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export default function (state = initialState, action) {
isRequestingLiveFeedback: true,
pendingFeedbackToken: null,
answerId: null,
liveFeedbackId: null,
feedbackFiles: {},
};
} else {
Expand All @@ -27,18 +28,20 @@ export default function (state = initialState, action) {
});
}
case actions.LIVE_FEEDBACK_REQUEST: {
const { token, questionId, feedbackUrl } = action.payload;
const { token, questionId, liveFeedbackId, feedbackUrl } = action.payload;
return produce(state, (draft) => {
draft.feedbackUrl ??= feedbackUrl;
if (!(questionId in draft)) {
draft.feedbackByQuestion[questionId] = {
isRequestingLiveFeedback: false,
liveFeedbackId,
pendingFeedbackToken: token,
};
} else {
draft.feedbackByQuestion[questionId] = {
isRequestingLiveFeedback: false,
...draft.feedbackByQuestion[questionId],
liveFeedbackId,
pendingFeedbackToken: token,
};
}
Expand Down
2 changes: 1 addition & 1 deletion client/app/types/course/statistics/assessmentStatistics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,5 +83,5 @@ export interface AssessmentLiveFeedbackStatistics {
courseUser: StudentInfo;
groups: { name: string }[];
workflowState?: WorkflowState;
liveFeedbackCount: number[]; // Will already be ordered by question
liveFeedbackCount?: number[]; // Will already be ordered by question
}
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@
post :generate_feedback, on: :member
get :fetch_submitted_feedback, on: :member
post :generate_live_feedback, on: :member
post :save_live_feedback, on: :member
get :download_all, on: :collection
get :download_statistics, on: :collection
patch :publish_all, on: :collection
Expand Down

0 comments on commit 063e585

Please sign in to comment.