Skip to content

Commit

Permalink
Store rejected submissions for helping to debug why they were rejecte…
Browse files Browse the repository at this point in the history
…d. (#1276)

* WIP migration

* Add table, and prepare code

* Store rejected submissions for debugging purposes

* Clippy fixes

* Save rejected submissions for debugging purposes
  • Loading branch information
nygrenh authored May 23, 2024
1 parent 3f56ff9 commit a92cc04
Show file tree
Hide file tree
Showing 13 changed files with 171 additions and 13 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
DROP TABLE rejected_exercise_task_submissions;
DROP TABLE rejected_exercise_slide_submissions;
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
CREATE TABLE rejected_exercise_slide_submissions (
id UUID DEFAULT uuid_generate_v4() PRIMARY KEY,
created_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(),
updated_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(),
deleted_at TIMESTAMP WITH TIME ZONE,
user_id UUID NOT NULL REFERENCES users(id),
exercise_slide_id UUID NOT NULL REFERENCES exercise_slides(id)
);
CREATE TRIGGER set_timestamp BEFORE
UPDATE ON rejected_exercise_slide_submissions FOR EACH ROW EXECUTE PROCEDURE trigger_set_timestamp();
COMMENT ON TABLE rejected_exercise_slide_submissions IS 'If an exercise slide submission is rejected, the submission is stored here. Rejections happen usually when there is some bug in the exercise service. This data if used for diagnosing problems.';
COMMENT ON COLUMN rejected_exercise_slide_submissions.id IS 'A unique, stable identifier for the record.';
COMMENT ON COLUMN rejected_exercise_slide_submissions.created_at IS 'Timestamp when the record was created.';
COMMENT ON COLUMN rejected_exercise_slide_submissions.updated_at IS 'Timestamp when the record was last updated. The field is updated automatically by the set_timestamp trigger.';
COMMENT ON COLUMN rejected_exercise_slide_submissions.deleted_at IS 'Timestamp when the record was deleted. If null, the record is not deleted.';
COMMENT ON COLUMN rejected_exercise_slide_submissions.user_id IS 'The user who submitted the exercise';
COMMENT ON COLUMN rejected_exercise_slide_submissions.exercise_slide_id IS 'The exercise slide that was submitted';
CREATE TABLE rejected_exercise_task_submissions (
id UUID DEFAULT uuid_generate_v4() PRIMARY KEY,
created_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(),
updated_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(),
deleted_at TIMESTAMP WITH TIME ZONE,
rejected_exercise_slide_submission_id UUID NOT NULL REFERENCES rejected_exercise_slide_submissions(id),
data_json JSONB NOT NULL
);
CREATE TRIGGER set_timestamp BEFORE
UPDATE ON rejected_exercise_task_submissions FOR EACH ROW EXECUTE PROCEDURE trigger_set_timestamp();
COMMENT ON TABLE rejected_exercise_task_submissions IS 'If an exercise task submission is rejected, the submission is stored here. Rejections happen usually when there is some bug in the exercise service. This data if used for diagnosing problems.';
COMMENT ON COLUMN rejected_exercise_task_submissions.id IS 'A unique, stable identifier for the record.';
COMMENT ON COLUMN rejected_exercise_task_submissions.created_at IS 'Timestamp when the record was created.';
COMMENT ON COLUMN rejected_exercise_task_submissions.updated_at IS 'Timestamp when the record was last updated. The field is updated automatically by the set_timestamp trigger.';
COMMENT ON COLUMN rejected_exercise_task_submissions.deleted_at IS 'Timestamp when the record was deleted. If null, the record is not deleted.';
COMMENT ON COLUMN rejected_exercise_task_submissions.rejected_exercise_slide_submission_id IS 'The rejected exercise slide submission that this task submission is related to';
COMMENT ON COLUMN rejected_exercise_task_submissions.data_json IS 'The contents of the failed sumbission';

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ pub async fn insert(
exercise_slide_submission_id: Uuid,
exercise_slide_id: Uuid,
exercise_task_id: Uuid,
data_json: Value,
data_json: &Value,
) -> ModelResult<Uuid> {
let res = sqlx::query!(
"
Expand Down
1 change: 1 addition & 0 deletions services/headless-lms/models/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ pub mod playground_examples;
pub mod proposed_block_edits;
pub mod proposed_page_edits;
pub mod regradings;
pub mod rejected_exercise_slide_submissions;
pub mod repository_exercises;
pub mod research_forms;
pub mod roles;
Expand Down
8 changes: 4 additions & 4 deletions services/headless-lms/models/src/library/grading.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ pub struct ExerciseStateUpdateNeedToUpdatePeerReviewStatusWithThis {
pub async fn create_user_exercise_slide_submission(
conn: &mut PgConnection,
exercise_with_user_state: &ExerciseWithUserState,
user_exercise_slide_submission: StudentExerciseSlideSubmission,
user_exercise_slide_submission: &StudentExerciseSlideSubmission,
) -> ModelResult<ExerciseSlideSubmissionWithTasks> {
let selected_exercise_slide_id = exercise_with_user_state
.user_exercise_state()
Expand Down Expand Up @@ -143,7 +143,7 @@ pub async fn create_user_exercise_slide_submission(
},
)
.await?;
let user_exercise_task_submissions = user_exercise_slide_submission.exercise_task_submissions;
let user_exercise_task_submissions = &user_exercise_slide_submission.exercise_task_submissions;
let mut exercise_slide_submission_tasks =
Vec::with_capacity(user_exercise_task_submissions.len());
for task_submission in user_exercise_task_submissions {
Expand All @@ -162,7 +162,7 @@ pub async fn create_user_exercise_slide_submission(
exercise_slide_submission.id,
exercise_task.exercise_slide_id,
exercise_task.id,
task_submission.data_json,
&task_submission.data_json,
)
.await?;
let submission = exercise_task_submissions::get_by_id(&mut tx, submission_id).await?;
Expand Down Expand Up @@ -232,7 +232,7 @@ pub enum GradingPolicy {
pub async fn grade_user_submission(
conn: &mut PgConnection,
exercise_with_user_state: &mut ExerciseWithUserState,
user_exercise_slide_submission: StudentExerciseSlideSubmission,
user_exercise_slide_submission: &StudentExerciseSlideSubmission,
grading_policy: GradingPolicy,
fetch_service_info: impl Fn(Url) -> BoxFuture<'static, ModelResult<ExerciseServiceInfoApi>>,
send_grading_request: impl Fn(
Expand Down
2 changes: 1 addition & 1 deletion services/headless-lms/models/src/library/regrading.rs
Original file line number Diff line number Diff line change
Expand Up @@ -955,7 +955,7 @@ mod test {
let grading = crate::library::grading::grade_user_submission(
conn,
&mut exercise_with_user_state,
submission,
&submission,
GradingPolicy::Fixed(mock_results),
|_| unimplemented!(),
|_, _, _| unimplemented!(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
use crate::{
library::grading::{StudentExerciseSlideSubmission, StudentExerciseTaskSubmission},
prelude::*,
};

#[derive(Clone, PartialEq, Deserialize, Serialize)]
pub struct RejectedExerciseSlideSubmission {
pub id: Uuid,
pub created_at: DateTime<Utc>,
pub updated_at: DateTime<Utc>,
pub deleted_at: Option<DateTime<Utc>>,
pub user_id: Uuid,
pub exercise_slide_id: Uuid,
}

pub async fn insert_rejected_exercise_slide_submission(
conn: &mut PgConnection,
rejected_submission: &StudentExerciseSlideSubmission,
user_id: Uuid,
) -> ModelResult<Uuid> {
let mut tx = conn.begin().await?;
let res = sqlx::query!(
"
INSERT INTO rejected_exercise_slide_submissions (user_id, exercise_slide_id)
VALUES ($1, $2)
RETURNING id
",
user_id,
rejected_submission.exercise_slide_id,
)
.fetch_one(&mut *tx)
.await?;

for task in &rejected_submission.exercise_task_submissions {
insert_rejected_exercise_task_submission(&mut tx, task, res.id).await?;
}

tx.commit().await?;
Ok(res.id)
}

/// Used internally only by the `insert_rejected_exercise_slide_submission` function.
async fn insert_rejected_exercise_task_submission(
conn: &mut PgConnection,
rejected_submission: &StudentExerciseTaskSubmission,
exercise_slide_submission_id: Uuid,
) -> ModelResult<Uuid> {
let res = sqlx::query!(
"
INSERT INTO rejected_exercise_task_submissions (rejected_exercise_slide_submission_id, data_json)
VALUES ($1, $2)
RETURNING id
",
exercise_slide_submission_id,
rejected_submission.data_json,
)
.fetch_one(&mut *conn)
.await?;
Ok(res.id)
}
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ async fn post_submission(
payload: web::Json<StudentExerciseSlideSubmission>,
user: AuthUser,
) -> ControllerResult<web::Json<StudentExerciseSlideSubmissionResult>> {
let submission = payload.0;
let mut conn = pool.acquire().await?;
let exercise = models::exercises::get_by_id(&mut conn, *exercise_id).await?;
let token = authorize(
Expand All @@ -214,12 +215,36 @@ async fn post_submission(
let result = domain::exercises::process_submission(
&mut conn,
user.id,
exercise,
payload.0,
exercise.clone(),
&submission,
jwt_key.into_inner(),
)
.await?;
token.authorized_ok(web::Json(result))
.await;
return match result {
Ok(res) => token.authorized_ok(web::Json(res)),
Err(err) => {
match models::rejected_exercise_slide_submissions::insert_rejected_exercise_slide_submission(
&mut conn,
&submission,
user.id,
)
.await {
Ok(_) => {
warn!(
"Submission was rejected but it was saved for debugging purposes. User id: {}, Exercise id: {}",
user.id, exercise.id
);
},
Err(_) => {
error!(
"Submission was rejected and saving it for debugging purposes failed. User id: {}, Exercise id: {}",
user.id, exercise.id
);
},
}
Err(err)
}
};
}

/**
Expand Down
2 changes: 1 addition & 1 deletion services/headless-lms/server/src/controllers/langs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ async fn submit_exercise(
&mut conn,
user.id,
exercise,
StudentExerciseSlideSubmission {
&StudentExerciseSlideSubmission {
exercise_slide_id: submission.exercise_slide_id,
exercise_task_submissions: vec![StudentExerciseTaskSubmission {
exercise_task_id: submission.exercise_task_id,
Expand Down
2 changes: 1 addition & 1 deletion services/headless-lms/server/src/domain/csv_export/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ mod test {
headless_lms_models::library::grading::grade_user_submission(
tx,
&mut exercise_with_user_state,
StudentExerciseSlideSubmission {
&StudentExerciseSlideSubmission {
exercise_slide_id: ex_slide,
exercise_task_submissions: vec![StudentExerciseTaskSubmission {
exercise_task_id: et,
Expand Down
2 changes: 1 addition & 1 deletion services/headless-lms/server/src/domain/exercises.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub async fn process_submission(
conn: &mut PgConnection,
user_id: Uuid,
exercise: Exercise,
submission: StudentExerciseSlideSubmission,
submission: &StudentExerciseSlideSubmission,
jwt_key: Arc<JwtKey>,
) -> Result<StudentExerciseSlideSubmissionResult, ControllerError> {
enforce_deadline(conn, &exercise).await?;
Expand Down

0 comments on commit a92cc04

Please sign in to comment.