Better error handling in new exam form, fix Youtube embed height in C…

…MS, peer review updater refactor (#1369)

* Make cms youtube embed styles more resistant to title changes on the IFrame

* Refactor new exam form to handle errors better

* Different way to fix embed styles due to nextjs miscompilation

* Fixes to the history test

- Introduced a new utility function `saveCMSPage` in `cmsUtils.ts` to encapsulate the save action and wait for the success notification.
- Updated `history.spec.ts` to replace direct save button clicks with calls to `saveCMSPage`, improving code readability and maintainability.

* Update message on model solution not to overflow

- Changed height property to min-height in MessageDialogDescription styled component to allow for flexible content display.

* Make exam progress check logic to skip exams with a minimum points threshold of 0. Added a comment to clarify the significance of this threshold in relation to course completion automation.

* Extract processing peer reviews for an course instance to a function

* Fix crash in peer review updater

.eslintrc.js

@@ -234,6 +234,8 @@ module.exports = {
             "JSON.parse",
             "addFilter",
             "removeFilter",
+            "setError",
+            "clearErrors",
           ],
         },
         "object-properties": {

services/cms/src/styles/Gutenberg/editor-styles.scss

@@ -47,23 +47,35 @@
 
   // This height is same as the headless-lms oembed proxy maxheight=440.
   iframe[title="Embedded content from youtube.com"],
+  iframe[title="Embedded content from www.youtube.com"],
   //For youtube shortlink
   iframe[title="Embedded content from youtu"] {
     height: 440px;
   }
 
-  iframe[title="Embedded content from spotify.com"] {
+  iframe[title="Embedded content from spotify.com"],
+  iframe[title="Embedded content from www.spotify.com"] {
     height: 335px;
   }
 
   iframe[title="Embedded content from twitter"],
+  iframe[title="Embedded content from www.twitter.com"],
+  iframe[title="Embedded content from x.com"],
+  iframe[title="Embedded content from www.x.com"],
   iframe[title="Embedded content from imgur"],
+  iframe[title="Embedded content from www.imgur.com"],
   iframe[title="Embedded content from reddit.com"],
+  iframe[title="Embedded content from www.reddit.com"],
   iframe[title="Embedded content from slideshare.net"],
+  iframe[title="Embedded content from www.slideshare.net"],
   iframe[title="Embedded content from ted.com"],
+  iframe[title="Embedded content from www.ted.com"],
   iframe[title="Embedded content from tumblr.com"],
+  iframe[title="Embedded content from www.tumblr.com"],
   iframe[title="Embedded content from menti.com"],
-  iframe[title="Embedded content from mentimeter.com"] {
+  iframe[title="Embedded content from www.menti.com"],
+  iframe[title="Embedded content from mentimeter.com"],
+  iframe[title="Embedded content from www.mentimeter.com"] {
     box-sizing: initial;
     border-top-width: 0px;
     border-right-width: 0px;
@@ -72,13 +84,16 @@
   }
 
   iframe[title="Embedded content from menti.com"],
-  iframe[title="Embedded content from mentimeter.com"] {
+  iframe[title="Embedded content from www.menti.com"],
+  iframe[title="Embedded content from mentimeter.com"],
+  iframe[title="Embedded content from www.mentimeter.com"] {
     html {
       overflow: hidden;
     }
   }
 
-  iframe[title="Embedded content from soundcloud"] {
+  iframe[title="Embedded content from soundcloud"],
+  iframe[title="Embedded content from www.soundcloud.com"] {
     box-sizing: initial;
   }
 

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

@@ -267,6 +267,10 @@ async fn user_has_passed_exam_for_the_course(
     let exam_ids = course_exams::get_exam_ids_by_course_id(conn, course_id).await?;
     for exam_id in exam_ids {
         let exam = exams::get(conn, exam_id).await?;
+        // A minimum points threshold of 0 indicates that the "Related courses can be completed automatically" option has not been enabled by the teacher. If you wish to remove this condition, please first store this information in a separate column in the exams table.
+        if exam.minimum_points_treshold == 0 {
+            continue;
+        }
         if exam.ended_at_or(now, false) {
             let points =
                 user_exercise_states::get_user_total_exam_points(conn, user_id, exam_id).await?;

services/headless-lms/server/src/programs/peer_review_updater.rs

@@ -1,9 +1,88 @@
 use crate::setup_tracing;
 use dotenv::dotenv;
+use headless_lms_models::error::TryToOptional;
 use headless_lms_models::peer_review_queue_entries;
 use sqlx::{Connection, PgConnection};
 use std::env;
 
+async fn process_course_instance(
+    conn: &mut PgConnection,
+    course_instance: &headless_lms_models::course_instances::CourseInstance,
+    now: chrono::DateTime<chrono::Utc>,
+) -> anyhow::Result<(i32, i32)> {
+    let mut moved_to_manual_review = 0;
+    let mut given_full_points = 0;
+
+    let mut earliest_manual_review_cutoff = now - chrono::Duration::days(7 * 3);
+
+    // Process manual review cases
+    let all_exercises_in_course_instance =
+        headless_lms_models::exercises::get_exercises_by_course_instance_id(
+            conn,
+            course_instance.id,
+        )
+        .await?;
+
+    for exercise in all_exercises_in_course_instance.iter() {
+        if !exercise.needs_peer_review {
+            continue;
+        }
+        let course_id = exercise.course_id;
+        if let Some(course_id) = course_id {
+            let exercise_config =
+                headless_lms_models::peer_or_self_review_configs::get_by_exercise_or_course_id(
+                    conn, exercise, course_id,
+                )
+                .await
+                .optional()?;
+
+            if let Some(exercise_config) = exercise_config {
+                let manual_review_cutoff_in_days = exercise_config.manual_review_cutoff_in_days;
+                let timestamp = now - chrono::Duration::days(manual_review_cutoff_in_days.into());
+
+                if timestamp < earliest_manual_review_cutoff {
+                    earliest_manual_review_cutoff = timestamp;
+                }
+
+                let should_be_added_to_manual_review = headless_lms_models::peer_review_queue_entries::get_entries_that_need_reviews_and_are_older_than_with_exercise_id(conn, exercise.id, timestamp).await?;
+                if !should_be_added_to_manual_review.is_empty() {
+                    info!(exercise.id = ?exercise.id, "Found {:?} answers that have been added to the peer review queue before {:?} and have not received enough peer reviews or have not been reviewed manually. Adding them to be manually reviewed by the teachers.", should_be_added_to_manual_review.len(), timestamp);
+
+                    for peer_review_queue_entry in should_be_added_to_manual_review {
+                        peer_review_queue_entries::remove_from_queue_and_add_to_manual_review(
+                            conn,
+                            &peer_review_queue_entry,
+                        )
+                        .await?;
+                        moved_to_manual_review += 1;
+                    }
+                }
+            } else {
+                warn!(exercise.id = ?exercise.id, "No peer review config found for exercise {:?}", exercise.id);
+            }
+        }
+    }
+
+    // After this date, we will assume that the teacher has given up on reviewing answers, so we will consider queue entries older than this to have passed the peer review.
+    let pass_automatically_cutoff = earliest_manual_review_cutoff - chrono::Duration::days(90);
+
+    // Process automatic pass cases
+    let should_pass = headless_lms_models::peer_review_queue_entries::get_entries_that_need_reviews_and_are_older_than(conn, course_instance.id, pass_automatically_cutoff).await?;
+    if !should_pass.is_empty() {
+        info!(course_instance_id = ?course_instance.id, "Found {:?} answers that have been added to the peer review queue before {:?} and have not received enough peer reviews or have not been reviewed manually. Giving them full points.", should_pass.len(), pass_automatically_cutoff);
+        for peer_review_queue_entry in should_pass {
+            let _res = peer_review_queue_entries::remove_from_queue_and_give_full_points(
+                conn,
+                &peer_review_queue_entry,
+            )
+            .await?;
+            given_full_points += 1;
+        }
+    }
+
+    Ok((moved_to_manual_review, given_full_points))
+}
+
 pub async fn main() -> anyhow::Result<()> {
     env::set_var("RUST_LOG", "info,actix_web=info,sqlx=warn");
     dotenv().ok();
@@ -12,9 +91,7 @@ pub async fn main() -> anyhow::Result<()> {
         .unwrap_or_else(|_| "postgres://localhost/headless_lms_dev".to_string());
     let mut conn = PgConnection::connect(&db_url).await?;
     let now = chrono::offset::Utc::now();
-    let manual_review_cutoff = now - chrono::Duration::days(7 * 3);
-    // After this date, we will assume that the teacher has given up on reviewing answers, so we will consider queue entries older than this to have passed the peer review.
-    let pass_automatically_cutoff = manual_review_cutoff - chrono::Duration::days(90);
+
     info!("Peer review updater started");
     // Doing the update in small parts so that we don't end up constructing too heavy queries and so that we can get more frequeent log messages about the progress
     let all_course_instances =
@@ -25,84 +102,29 @@ pub async fn main() -> anyhow::Result<()> {
     );
 
     info!(
-        ?manual_review_cutoff,
+        earliest_manual_review_cutoff = ?now - chrono::Duration::days(7 * 3),
         "Finding answers to move to manual review"
     );
 
-    let mut moved_to_manual_review = 0;
+    let mut total_moved_to_manual_review = 0;
+    let mut total_given_full_points = 0;
 
     for course_instance in all_course_instances.iter() {
-        let all_exercises_in_course_instance =
-            headless_lms_models::exercises::get_exercises_by_course_instance_id(
-                &mut conn,
-                course_instance.id,
-            )
-            .await?;
-        //Go through all exercises in the course instance to check if the exercises have custom manual review cutoff times
-        for exercise in all_exercises_in_course_instance.iter() {
-            if !exercise.needs_peer_review {
-                continue;
-            }
-            let course_id = exercise.course_id;
-            if let Some(course_id) = course_id {
-                let exercise_config =
-                    headless_lms_models::peer_or_self_review_configs::get_by_exercise_or_course_id(
-                        &mut conn, exercise, course_id,
-                    )
-                    .await?;
-
-                let manual_review_cutoff_in_days = exercise_config.manual_review_cutoff_in_days;
+        let (moved_to_manual_review, given_full_points) =
+            process_course_instance(&mut conn, course_instance, now).await?;
 
-                let timestamp = now - chrono::Duration::days(manual_review_cutoff_in_days.into());
-
-                let should_be_added_to_manual_review = headless_lms_models::peer_review_queue_entries::get_entries_that_need_reviews_and_are_older_than_with_exercise_id(&mut conn, exercise.id, timestamp).await?;
-                if should_be_added_to_manual_review.is_empty() {
-                    continue;
-                }
-
-                info!(exercise.id = ?exercise.id, "Found {:?} answers that have been added to the peer review queue before {:?} and have not received enough peer reviews or have not been reviewed manually. Adding them to be manually reviewed by the teachers.", should_be_added_to_manual_review.len(), timestamp);
-
-                for peer_review_queue_entry in should_be_added_to_manual_review {
-                    peer_review_queue_entries::remove_from_queue_and_add_to_manual_review(
-                        &mut conn,
-                        &peer_review_queue_entry,
-                    )
-                    .await?;
-                    moved_to_manual_review += 1
-                }
-            }
-        }
+        total_moved_to_manual_review += moved_to_manual_review;
+        total_given_full_points += given_full_points;
     }
 
     info!(
         "Total answers moved to manual review: {:?}",
-        moved_to_manual_review
+        total_moved_to_manual_review
     );
-
     info!(
-        ?pass_automatically_cutoff,
-        "Finding answers to give full points"
+        "Total answers given full points: {:?}",
+        total_given_full_points
     );
-
-    let mut given_full_points = 0;
-
-    for course_instance in all_course_instances.iter() {
-        let should_pass = headless_lms_models::peer_review_queue_entries::get_entries_that_need_reviews_and_are_older_than(&mut conn, course_instance.id, pass_automatically_cutoff).await?;
-        if should_pass.is_empty() {
-            continue;
-        }
-        info!(course_instance_id = ?course_instance.id, "Found {:?} answers that have been added to the peer review queue before {:?} and have not received enough peer reviews or have not been reviewed manually. Giving them full points.", should_pass.len(), pass_automatically_cutoff);
-        for peer_review_queue_entry in should_pass {
-            let _res = peer_review_queue_entries::remove_from_queue_and_give_full_points(
-                &mut conn,
-                &peer_review_queue_entry,
-            )
-            .await?;
-            given_full_points += 1;
-        }
-    }
-
-    info!("Total answers given full points: {:?}", given_full_points);
     info!("All done!");
     Ok(())
 }