WIP

services/headless-lms/models/src/course_instances.rs

@@ -965,7 +965,7 @@ mod test {
     #[tokio::test]
     async fn gets_course_average_duration_with_empty_database() {
         insert_data!(:tx, :user, :org, :course, :instance);
-        let duration = get_course_average_duration(tx.as_mut(), course.id)
+        let duration = get_course_average_duration(tx.as_mut(), course)
             .await
             .unwrap();
         assert!(duration.is_none())

services/headless-lms/models/src/course_module_completions.rs

@@ -249,7 +249,7 @@ WHERE settings.current_course_instance_id = $1
 
 /// Gets all module completions for the user on a course. There can be multiple modules
 /// in a single course, so the result is a `Vec`.
-pub async fn get_all_by_course_and_user_id(
+pub async fn get_all_by_course_id_and_user_id(
     conn: &mut PgConnection,
     course_id: Uuid,
     user_id: Uuid,

services/headless-lms/models/src/exercise_slide_submissions.rs

@@ -14,7 +14,7 @@ use crate::{
     exercises::{self, Exercise, GradingProgress},
     prelude::*,
     teacher_grading_decisions::{self, TeacherGradingDecision},
-    user_exercise_states::{self, CourseOrExamId, UserExerciseState},
+    user_exercise_states::{self, UserExerciseState},
 };
 
 #[derive(Debug, Serialize, Deserialize, PartialEq, Clone)]

services/headless-lms/models/src/exercise_task_gradings.rs

@@ -14,7 +14,6 @@ use crate::{
     library::custom_view_exercises::CustomViewExerciseTaskGrading,
     prelude::*,
     user_exercise_states::UserExerciseState,
-    CourseOrExamId,
 };
 
 #[derive(Debug, Serialize, Deserialize, PartialEq, Clone)]
@@ -238,7 +237,10 @@ where id = $1
     )
     .fetch_one(conn)
     .await?;
-    CourseOrExamId::from(res.course_id, res.exam_id)
+    Ok(CourseOrExamId::from_course_and_exam_ids(
+        res.course_id,
+        res.exam_id,
+    )?)
 }
 
 pub async fn new_grading(

services/headless-lms/models/src/exercise_task_submissions.rs

@@ -239,7 +239,10 @@ WHERE ets.id = $1
     )
     .fetch_one(conn)
     .await?;
-    CourseOrExamId::from(res.course_id, res.exam_id)
+    Ok(CourseOrExamId::from_course_and_exam_ids(
+        res.course_id,
+        res.exam_id,
+    )?)
 }
 
 pub async fn get_peer_reviews_received(

services/headless-lms/models/src/exercise_tasks.rs

@@ -13,7 +13,7 @@ use crate::{
     exercise_task_submissions::{self, ExerciseTaskSubmission},
     library::custom_view_exercises::CustomViewExerciseTaskSpec,
     prelude::*,
-    user_exercise_states::{self, CourseOrExamId},
+    user_exercise_states::{self},
 };
 
 /// Information necessary for the frontend to render an exercise task

services/headless-lms/models/src/exercises.rs

@@ -20,7 +20,7 @@ use crate::{
     teacher_grading_decisions::{TeacherDecisionType, TeacherGradingDecision},
     user_course_exercise_service_variables::UserCourseExerciseServiceVariable,
     user_course_settings,
-    user_exercise_states::{self, CourseOrExamId, ReviewingStage, UserExerciseState},
+    user_exercise_states::{self, ReviewingStage, UserExerciseState},
 };
 use std::collections::HashMap;
 
@@ -915,7 +915,7 @@ mod test {
             tx.as_mut(),
             user_id,
             exercise_id,
-            CourseOrExamId::Instance(course_instance.id),
+            CourseOrExamId::Course(course_id),
         )
         .await
         .unwrap();
@@ -943,7 +943,7 @@ mod test {
             tx.as_mut(),
             user_id,
             exercise_id,
-            CourseOrExamId::Instance(course_instance.id),
+            CourseOrExamId::Course(course_id),
         )
         .await
         .unwrap();

services/headless-lms/models/src/lib.rs

@@ -99,8 +99,10 @@ pub mod prelude;
 #[cfg(test)]
 pub mod test_helper;
 
+use exercises::Exercise;
 use futures::future::BoxFuture;
 use url::Url;
+use user_exercise_states::UserExerciseState;
 use uuid::Uuid;
 
 pub use self::error::{ModelError, ModelErrorType, ModelResult};
@@ -254,66 +256,97 @@ impl PKeyPolicy<Uuid> {
     }
 }
 
-/// Many database tables are related to either a course or an exam
-#[derive(Clone, Copy)]
+/// A "trait alias" so this `for<'a>` ... string doesn't need to be repeated everywhere
+/// Arguments:
+///   `Url`: The URL that the request is sent to (the exercise service's endpoint)
+///   `&str`: Exercise type/service slug
+///   `Option<Value>`: The Json for the request, for example the private spec in a public spec request
+pub trait SpecFetcher:
+    for<'a> Fn(
+    Url,
+    &'a str,
+    Option<&'a serde_json::Value>,
+) -> BoxFuture<'a, ModelResult<serde_json::Value>>
+{
+}
+
+impl<
+        T: for<'a> Fn(
+            Url,
+            &'a str,
+            Option<&'a serde_json::Value>,
+        ) -> BoxFuture<'a, ModelResult<serde_json::Value>>,
+    > SpecFetcher for T
+{
+}
+
+/// Either a course or exam id.
+///
+/// Exercises can either be part of courses or exams. Many user-related actions need to differentiate
+/// between two, so `CourseOrExamId` helps when handling these separate scenarios.
+#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Copy)]
 pub enum CourseOrExamId {
     Course(Uuid),
     Exam(Uuid),
 }
 
 impl CourseOrExamId {
-    pub fn from(course_id: Option<Uuid>, exam_id: Option<Uuid>) -> ModelResult<Self> {
+    pub fn from_course_and_exam_ids(
+        course_id: Option<Uuid>,
+        exam_id: Option<Uuid>,
+    ) -> ModelResult<Self> {
         match (course_id, exam_id) {
-            (Some(course_id), None) => Ok(Self::Course(course_id)),
-            (None, Some(exam_id)) => Ok(Self::Exam(exam_id)),
-            (Some(_), Some(_)) => Err(ModelError::new(
+            (None, None) => Err(ModelError::new(
                 ModelErrorType::Generic,
-                "Database row had both a course id and an exam id".to_string(),
+                "Expected either course or exam id, but neither were provided.",
                 None,
             )),
-            (None, None) => Err(ModelError::new(
+            (Some(course_id), None) => Ok(Self::Course(course_id)),
+            (None, Some(exam_id)) => Ok(Self::Exam(exam_id)),
+            (Some(_), Some(_)) => Err(ModelError::new(
                 ModelErrorType::Generic,
-                "Database row did not have a course id or an exam id".to_string(),
+                "Expected either course or exam id, but both were provided.",
                 None,
             )),
         }
     }
 
-    pub fn to_course_and_exam_ids(self) -> (Option<Uuid>, Option<Uuid>) {
+    pub fn to_course_and_exam_ids(&self) -> (Option<Uuid>, Option<Uuid>) {
         match self {
-            Self::Course(instance_id) => (Some(instance_id), None),
-            Self::Exam(exam_id) => (None, Some(exam_id)),
+            CourseOrExamId::Course(course_id) => (Some(*course_id), None),
+            CourseOrExamId::Exam(exam_id) => (None, Some(*exam_id)),
         }
     }
-    pub fn exam_id(self) -> Option<Uuid> {
-        if let CourseOrExamId::Exam(id) = self {
-            Some(id)
-        } else {
-            None
-        }
+}
+
+impl TryFrom<UserExerciseState> for CourseOrExamId {
+    type Error = ModelError;
+
+    fn try_from(user_exercise_state: UserExerciseState) -> Result<Self, Self::Error> {
+        Self::from_course_and_exam_ids(user_exercise_state.course_id, user_exercise_state.exam_id)
     }
 }
 
-/// A "trait alias" so this `for<'a>` ... string doesn't need to be repeated everywhere
-/// Arguments:
-///   `Url`: The URL that the request is sent to (the exercise service's endpoint)
-///   `&str`: Exercise type/service slug
-///   `Option<Value>`: The Json for the request, for example the private spec in a public spec request
-pub trait SpecFetcher:
-    for<'a> Fn(
-    Url,
-    &'a str,
-    Option<&'a serde_json::Value>,
-) -> BoxFuture<'a, ModelResult<serde_json::Value>>
-{
+impl TryFrom<&UserExerciseState> for CourseOrExamId {
+    type Error = ModelError;
+
+    fn try_from(user_exercise_state: &UserExerciseState) -> Result<Self, Self::Error> {
+        Self::from_course_and_exam_ids(user_exercise_state.course_id, user_exercise_state.exam_id)
+    }
 }
 
-impl<
-        T: for<'a> Fn(
-            Url,
-            &'a str,
-            Option<&'a serde_json::Value>,
-        ) -> BoxFuture<'a, ModelResult<serde_json::Value>>,
-    > SpecFetcher for T
-{
+impl TryFrom<Exercise> for CourseOrExamId {
+    type Error = ModelError;
+
+    fn try_from(exercise: Exercise) -> Result<Self, Self::Error> {
+        Self::from_course_and_exam_ids(exercise.course_id, exercise.exam_id)
+    }
+}
+
+impl TryFrom<&Exercise> for CourseOrExamId {
+    type Error = ModelError;
+
+    fn try_from(exercise: &Exercise) -> Result<Self, Self::Error> {
+        Self::from_course_and_exam_ids(exercise.course_id, exercise.exam_id)
+    }
 }

services/headless-lms/models/src/library/grading.rs

@@ -22,7 +22,7 @@ use crate::{
     regradings,
     user_course_exercise_service_variables::UserCourseExerciseServiceVariable,
     user_exercise_slide_states::{self, UserExerciseSlideState},
-    user_exercise_states::{self, CourseOrExamId, ExerciseWithUserState, UserExerciseState},
+    user_exercise_states::{self, ExerciseWithUserState, UserExerciseState},
     user_exercise_task_states,
 };
 

services/headless-lms/models/src/library/peer_or_self_reviewing.rs

@@ -17,7 +17,7 @@ use crate::{
     peer_or_self_review_submissions,
     peer_review_queue_entries::{self, PeerReviewQueueEntry},
     prelude::*,
-    user_exercise_states::{self, CourseOrExamId, ReviewingStage, UserExerciseState},
+    user_exercise_states::{self, ReviewingStage, UserExerciseState},
 };
 
 use super::user_exercise_state_updater::{

services/headless-lms/models/src/library/progressing.rs

@@ -294,7 +294,8 @@ async fn update_module_completion_prerequisite_statuses_for_user(
 ) -> ModelResult<()> {
     let default_course_module = course_modules::get_default_by_course_id(conn, course_id).await?;
     let course_module_completions =
-        course_module_completions::get_all_by_course_and_user_id(conn, course_id, user_id).await?;
+        course_module_completions::get_all_by_course_id_and_user_id(conn, course_id, user_id)
+            .await?;
     let default_module_is_completed = course_module_completions
         .iter()
         .any(|x| x.course_module_id == default_course_module.id);
@@ -720,7 +721,7 @@ pub async fn get_user_module_completion_statuses_for_course(
     let course_modules = course_modules::get_by_course_id(conn, course_id).await?;
     let course_module_ids = course_modules.iter().map(|x| x.id).collect::<Vec<_>>();
     let course_module_completions: HashMap<Uuid, CourseModuleCompletion> =
-        course_module_completions::get_all_by_course_and_user_id(conn, course_id, user_id)
+        course_module_completions::get_all_by_course_id_and_user_id(conn, course_id, user_id)
             .await?
             .into_iter()
             .map(|x| (x.course_module_id, x))

services/headless-lms/models/src/pages.rs

@@ -410,7 +410,10 @@ WHERE id = $1
     )
     .fetch_one(conn)
     .await?;
-    CourseOrExamId::from(res.course_id, res.exam_id)
+    Ok(CourseOrExamId::from_course_and_exam_ids(
+        res.course_id,
+        res.exam_id,
+    )?)
 }
 
 pub enum PageVisibility {

services/headless-lms/models/src/peer_review_queue_entries.rs

@@ -457,7 +457,7 @@ pub async fn remove_from_queue_and_add_to_manual_review(
     let _ues = user_exercise_states::update_reviewing_stage(
         &mut tx,
         peer_review_queue_entry.user_id,
-        user_exercise_states::CourseOrExamId::Course(peer_review_queue_entry.course_id),
+        CourseOrExamId::Course(peer_review_queue_entry.course_id),
         peer_review_queue_entry.exercise_id,
         ReviewingStage::WaitingForManualGrading,
     )
@@ -477,7 +477,7 @@ pub async fn remove_from_queue_and_give_full_points(
         &mut tx,
         peer_review_queue_entry.user_id,
         peer_review_queue_entry.exercise_id,
-        user_exercise_states::CourseOrExamId::Course(peer_review_queue_entry.course_id),
+        CourseOrExamId::Course(peer_review_queue_entry.course_id),
     )
     .await?;
     if let Some(user_exercise_state) = user_exercise_state {

services/headless-lms/models/src/user_course_exercise_service_variables.rs

@@ -1,10 +1,6 @@
 use std::collections::HashMap;
 
-use crate::{
-    exercise_tasks::ExerciseTask,
-    prelude::*,
-    user_exercise_states::{CourseOrExamId, UserExerciseState},
-};
+use crate::{exercise_tasks::ExerciseTask, prelude::*, user_exercise_states::UserExerciseState};
 
 #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
 #[cfg_attr(feature = "ts_rs", derive(TS))]

services/headless-lms/models/src/user_exercise_states.rs

@@ -98,77 +98,6 @@ pub struct UserExerciseStateUpdate {
     pub grading_progress: GradingProgress,
 }
 
-/// Either a course or exam id.
-///
-/// Exercises can either be part of courses or exams. Many user-related actions need to differentiate
-/// between two, so `CourseOrExamId` helps when handling these separate scenarios.
-#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Copy)]
-pub enum CourseOrExamId {
-    Course(Uuid),
-    Exam(Uuid),
-}
-
-impl CourseOrExamId {
-    pub fn from_course_and_exam_ids(
-        course_id: Option<Uuid>,
-        exam_id: Option<Uuid>,
-    ) -> ModelResult<Self> {
-        match (course_id, exam_id) {
-            (None, None) => Err(ModelError::new(
-                ModelErrorType::Generic,
-                "Expected either course or exam id, but neither were provided.",
-                None,
-            )),
-            (Some(course_id), None) => Ok(Self::Course(course_id)),
-            (None, Some(exam_id)) => Ok(Self::Exam(exam_id)),
-            (Some(_), Some(_)) => Err(ModelError::new(
-                ModelErrorType::Generic,
-                "Expected either course or exam id, but both were provided.",
-                None,
-            )),
-        }
-    }
-
-    pub fn to_course_and_exam_ids(&self) -> (Option<Uuid>, Option<Uuid>) {
-        match self {
-            CourseOrExamId::Course(course_id) => (Some(*course_id), None),
-            CourseOrExamId::Exam(exam_id) => (None, Some(*exam_id)),
-        }
-    }
-}
-
-impl TryFrom<UserExerciseState> for CourseOrExamId {
-    type Error = ModelError;
-
-    fn try_from(user_exercise_state: UserExerciseState) -> Result<Self, Self::Error> {
-        Self::from_course_and_exam_ids(user_exercise_state.course_id, user_exercise_state.exam_id)
-    }
-}
-
-impl TryFrom<&UserExerciseState> for CourseOrExamId {
-    type Error = ModelError;
-
-    fn try_from(user_exercise_state: &UserExerciseState) -> Result<Self, Self::Error> {
-        Self::from_course_and_exam_ids(user_exercise_state.course_id, user_exercise_state.exam_id)
-    }
-}
-
-impl TryFrom<Exercise> for CourseOrExamId {
-    type Error = ModelError;
-
-    fn try_from(exercise: Exercise) -> Result<Self, Self::Error> {
-        Self::from_course_and_exam_ids(exercise.course_id, exercise.exam_id)
-    }
-}
-
-impl TryFrom<&Exercise> for CourseOrExamId {
-    type Error = ModelError;
-
-    fn try_from(exercise: &Exercise) -> Result<Self, Self::Error> {
-        Self::from_course_and_exam_ids(exercise.course_id, exercise.exam_id)
-    }
-}
-
 #[derive(Debug, Serialize, Deserialize, FromRow, PartialEq, Clone)]
 #[cfg_attr(feature = "ts_rs", derive(TS))]
 pub struct UserCourseProgress {