Skip to content

Commit

Permalink
Fix annotation feedback not shown when share feature is disabled (#686)
Browse files Browse the repository at this point in the history
* Always show annotations visible to teacher

(should not be dependent on whether NEW annotations can be posted
with the teacher or not)

* Add cypress tests to avoid bug regression

* Remove unnecessary "with_teacher_by_id"

* Make UI visible in the viewport during cypress tests
  • Loading branch information
Splines authored Sep 19, 2024
1 parent 5caf307 commit 73b0fc8
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 14 deletions.
13 changes: 2 additions & 11 deletions app/controllers/annotations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/views/annotations/_annotation_area.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<figcaption id="annotation-caption">
<figcaption id="annotation-caption" data-cy="annotation-caption">
<div id="annotation-infobar">
<!--- ANNOTATION CATEGORY -->
</div>
Expand Down
2 changes: 1 addition & 1 deletion app/views/lectures/edit/_comments.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
</div>

<!--- Annotation button related setting -->
<div class="pt-3 form-group">
<div class="pt-3 form-group" data-cy="annotation-lecture-settings">
<%= t('admin.lecture.enable_annotation_button') %>
<%= helpdesk(t('admin.lecture.enable_annotation_button_helpdesk'), false) %>

Expand Down
1 change: 1 addition & 0 deletions app/views/lectures/edit/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@

<!-- Communication -->
<div class="tab-pane fade" id="lecture-pane-communication"
data-cy="lecture-pane-communication"
role="tabpanel" aria-labelledby="lecture-nav-communication" tabindex="0">
<div class="container small-width lecture-pane">
<%= render partial: 'lectures/edit/announcements',
Expand Down
3 changes: 2 additions & 1 deletion app/views/media/feedback.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
<span id="heatmap">
<!-- JavaScript inserts heatmap here -->
</span>
<span id="feedback-markers" class="annotation-marker">
<span id="feedback-markers" data-cy="feedback-markers"
class="annotation-marker">
<!-- JavaScript inserts markers here -->
</span>
<input type="range" id="seek-bar"
Expand Down
71 changes: 71 additions & 0 deletions spec/cypress/e2e/annotations_spec.cy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import FactoryBot from "../support/factorybot";

function createLectureLessonMedium(context, teacher) {
// Lecture
FactoryBot.create("lecture_with_sparse_toc", "with_title",
{ title: "Groundbreaking lecture", teacher_id: teacher.id }).as("lecture");

// Lesson
cy.then(() => {
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")
.should("be.visible")
.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")
.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 is 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);
});
});
});
});
});

0 comments on commit 73b0fc8

Please sign in to comment.