From fd221259e389f4066b9368e3aff0a3e9f91f9e70 Mon Sep 17 00:00:00 2001 From: Splines Date: Thu, 22 Aug 2024 17:01:06 +0200 Subject: [PATCH 1/4] Always show annotations visible to teacher (should not be dependent on whether NEW annotations can be posted with the teacher or not) --- app/controllers/annotations_controller.rb | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/app/controllers/annotations_controller.rb b/app/controllers/annotations_controller.rb index 67d579757..6e6ea47b7 100644 --- a/app/controllers/annotations_controller.rb +++ b/app/controllers/annotations_controller.rb @@ -91,17 +91,8 @@ def destroy def update_annotations medium = Medium.find_by(id: params[:mediumId]) - # Get the right annotations - annotations = if medium.annotations_visible?(current_user) - Annotation.where(medium: medium, - visible_for_teacher: true).or( - Annotation.where(medium: medium, - user: current_user) - ) - else - Annotation.where(medium: medium, - user: current_user) - end + annotations = Annotation.where(medium: medium, visible_for_teacher: true) + .or(Annotation.where(medium: medium, user: current_user)) # If annotation is associated to a comment, # the field "comment" is empty -> get it from the commontator comment From 0c921f67a74838c62e296ae427c7fff236d915a5 Mon Sep 17 00:00:00 2001 From: Splines Date: Fri, 23 Aug 2024 00:15:03 +0200 Subject: [PATCH 2/4] Add cypress tests to avoid bug regression --- .../annotations/_annotation_area.html.erb | 2 +- app/views/lectures/edit/_comments.html.erb | 2 +- app/views/lectures/edit/_form.html.erb | 1 + app/views/media/feedback.html.erb | 3 +- spec/cypress/e2e/annotations_spec.cy.js | 69 +++++++++++++++++++ 5 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 spec/cypress/e2e/annotations_spec.cy.js diff --git a/app/views/annotations/_annotation_area.html.erb b/app/views/annotations/_annotation_area.html.erb index 2e0327e89..3bea2edd0 100644 --- a/app/views/annotations/_annotation_area.html.erb +++ b/app/views/annotations/_annotation_area.html.erb @@ -1,4 +1,4 @@ -
+
diff --git a/app/views/lectures/edit/_comments.html.erb b/app/views/lectures/edit/_comments.html.erb index 60af9b03f..ea9e4ecea 100644 --- a/app/views/lectures/edit/_comments.html.erb +++ b/app/views/lectures/edit/_comments.html.erb @@ -36,7 +36,7 @@ -
+
<%= t('admin.lecture.enable_annotation_button') %> <%= helpdesk(t('admin.lecture.enable_annotation_button_helpdesk'), false) %> diff --git a/app/views/lectures/edit/_form.html.erb b/app/views/lectures/edit/_form.html.erb index 00e2608c3..b37145278 100644 --- a/app/views/lectures/edit/_form.html.erb +++ b/app/views/lectures/edit/_form.html.erb @@ -118,6 +118,7 @@
<%= render partial: 'lectures/edit/announcements', diff --git a/app/views/media/feedback.html.erb b/app/views/media/feedback.html.erb index eb92333bf..843c83c2c 100644 --- a/app/views/media/feedback.html.erb +++ b/app/views/media/feedback.html.erb @@ -23,7 +23,8 @@ - + { + FactoryBot.create("valid_lesson", { lecture_id: context.lecture.id }).as("lesson"); + }); + + // Medium + cy.then(() => { + FactoryBot.create("lesson_medium", "with_video", "released", + "with_lesson_by_id", { lesson_id: context.lesson.id, description: "Soil medium" }) + .as("medium"); + }); +} + +describe("Annotations visibility", () => { + context("when teacher disables annotation sharing with teachers", () => { + it("annotations published before that are still visible to the teacher", function () { + cy.createUser("generic").as("user"); + cy.createUserAndLogin("teacher").then(teacher => createLectureLessonMedium(this, teacher)); + + cy.then(() => { + // Create new annotation + FactoryBot.create("annotation", "with_text", "shared_with_teacher", + { medium_id: this.medium.id, user_id: this.user.id }).as("annotation"); + + // Disable annotation sharing in lecture settings + cy.visit(`/lectures/${this.lecture.id}/edit#communication`); + cy.getBySelector("annotation-lecture-settings") + .find("input[value=0]").should("have.length", 1).click(); + + // Click on submit button to save changes + cy.intercept("POST", `/lectures/${this.lecture.id}`).as("lectureUpdate"); + cy.getBySelector("lecture-pane-communication") + .find("input[type=submit]").should("have.length", 1).click(); + cy.wait("@lectureUpdate"); + + // Make sure that changes were really saved + cy.reload(); + cy.getBySelector("annotation-lecture-settings").then(($form) => { + cy.wrap($form).find("input[value=0]").should("be.checked"); + cy.wrap($form).find("input[value=1]").should("not.be.checked"); + }); + }); + + cy.then(() => { + cy.visit(`/media/${this.medium.id}/feedback`); + + // Annotation visible + cy.getBySelector("feedback-markers") + .children().should("have.length", 1) + .click({ force: true }); + + // Annotation can be opened in sidebar + cy.getBySelector("annotation-caption").then(($sideBar) => { + cy.i18n(`admin.annotation.${this.annotation.category}`).then((category) => { + cy.wrap($sideBar).children().first().should("contain", category); + }); + cy.wrap($sideBar).children().eq(1).should("contain", this.annotation.comment); + }); + }); + }); + }); +}); From 3386ea4f1b2481d65adac0833594c4aad55272f2 Mon Sep 17 00:00:00 2001 From: Splines Date: Fri, 23 Aug 2024 00:49:45 +0200 Subject: [PATCH 3/4] Remove unnecessary "with_teacher_by_id" --- spec/cypress/e2e/annotations_spec.cy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/cypress/e2e/annotations_spec.cy.js b/spec/cypress/e2e/annotations_spec.cy.js index 9640153bc..aff9275ed 100644 --- a/spec/cypress/e2e/annotations_spec.cy.js +++ b/spec/cypress/e2e/annotations_spec.cy.js @@ -2,7 +2,7 @@ import FactoryBot from "../support/factorybot"; function createLectureLessonMedium(context, teacher) { // Lecture - FactoryBot.create("lecture_with_sparse_toc", "with_title", "with_teacher_by_id", + FactoryBot.create("lecture_with_sparse_toc", "with_title", { title: "Groundbreaking lecture", teacher_id: teacher.id }).as("lecture"); // Lesson From 36ff03b8d854be36ae94ab3fbd3d5537171d0430 Mon Sep 17 00:00:00 2001 From: Splines Date: Thu, 5 Sep 2024 10:56:15 +0200 Subject: [PATCH 4/4] Make UI visible in the viewport during cypress tests --- spec/cypress/e2e/annotations_spec.cy.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/spec/cypress/e2e/annotations_spec.cy.js b/spec/cypress/e2e/annotations_spec.cy.js index aff9275ed..498f98465 100644 --- a/spec/cypress/e2e/annotations_spec.cy.js +++ b/spec/cypress/e2e/annotations_spec.cy.js @@ -32,6 +32,7 @@ describe("Annotations visibility", () => { // Disable annotation sharing in lecture settings cy.visit(`/lectures/${this.lecture.id}/edit#communication`); cy.getBySelector("annotation-lecture-settings") + .should("be.visible") .find("input[value=0]").should("have.length", 1).click(); // Click on submit button to save changes @@ -42,16 +43,17 @@ describe("Annotations visibility", () => { // Make sure that changes were really saved cy.reload(); - cy.getBySelector("annotation-lecture-settings").then(($form) => { - cy.wrap($form).find("input[value=0]").should("be.checked"); - cy.wrap($form).find("input[value=1]").should("not.be.checked"); - }); + cy.getBySelector("annotation-lecture-settings") + .should("be.visible").then(($form) => { + cy.wrap($form).find("input[value=0]").should("be.checked"); + cy.wrap($form).find("input[value=1]").should("not.be.checked"); + }); }); cy.then(() => { cy.visit(`/media/${this.medium.id}/feedback`); - // Annotation visible + // Annotation is visible cy.getBySelector("feedback-markers") .children().should("have.length", 1) .click({ force: true });