Skip to content

Commit

Permalink
Add annotations overview page & improve Cypress commands (#668)
Browse files Browse the repository at this point in the history
* Scaffold annotation overview page

* Show all annotations in card list

* Only show annotations of current user

* Add links to annotations and fix empty comments

* Fix annotation caching issue by fixing initialization

This might fix #637

* Open annotation automatically on link click

* Group annotations by lecture

* Fix accordion header class naming

* Link to blog when no annotation is created yet

* Delete unnecessary whitespace

* Localize text in annotation overview

* Refactor annotations overview controller

* Init view to see students annotations as teacher/editor

* Use existing methods to grab media

* Improve description of student annotations

* Use links to feedback player for shared annotations

* Use same bootstrap button for "Create annotation"

* Only show students annotations if current user is teacher or editor

* Fix grammar mistake

* Refactor annotations overview controller

* Refactor accordion label assignment

* Color cards shared by students according to category

* Rename js file to "url_annotation_opener"

* Clarify color choices in user-facing text

* Get rid of turbolinks:load and apply directly

This is to avoid caching issues.

* Fix initial state of accordion buttons

* Show medium title and date

* Expose MaMpf to another port to avoid clashing with dev

* Add an endpoint to skip validation when creating factory

* Use local file system also in test environment

* Enable reloading in cypress tests

* Wrap value in `createUserAndLogin` (cypress)

* Init annotations factory for usage in cypress tests

* Customize title of lectures/lessons

* Test appearance of annotation overview cards

* Use parameterized tests and test timestamp

* Add back missing index variable in test

* Test annotation border color

* Add factorybot to known words

* Add i18n retrieval in Cypress tests

* Test annotation sections & improve i18n controller

* Test student annotation cards & more properties of cards

* Move annotation section test to the top

* Move "grouped by lecture" test up

* Test rendering of math content

* Move overview controller to existing annotation controller

(it is now called "index")

* Move annotation-retrieval to user model

* Move annotation_open_link to helper

* Make extraction method private in controller

Also removed the TODO note regarding nil values

* Remove TODO note as "Won't be done"

* Remove unnecessary long url helper calling

* Pass in medium instead of medium id

* Fix spelling mistake in media helper

* Use link_to Rails helper

* Don't use custom route name for annotations overview

It is now just the "index" and the route is called
"/annotations"

* Sort according to lecture.updated_at

* Extract "annotations by lecture" retrieval to own method

This is to avoid duplicated code.

* Add comment explaining transform keys

* Rename partial view to index_accordion

* Extract border color to helper method

* Open annotations with video in new tab

* Add number of annotations in accordion header

* Add default title to lecture factory with_title

* Fix ordering in cypress tests & sort according to creation date

* Fix cypress tests when anchor link opens new tab

* Fix ordering in math content test

* Fix ordering and new tab in feedback video test

* Fix border color test order

* Change default cypress viewport dimensions

* Place :index ability in other code block

* Use params instead of manually constructing url

* Use URI helper to construct annotation id link

* Use flat_map instead of map(...).flatten
  • Loading branch information
Splines authored Aug 13, 2024
1 parent b2d587f commit 8e44e6f
Show file tree
Hide file tree
Showing 38 changed files with 715 additions and 34 deletions.
3 changes: 2 additions & 1 deletion .config/.cypress.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
module.exports = {
e2e: {
// Add configuration here
// Base URL is set via Docker environment variable
viewportHeight: 1000,
viewportWidth: 1400,
},
};
2 changes: 2 additions & 0 deletions .config/eslint.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ const customGlobals = {

// KaTeX
renderMathInElement: "readable",

openAnnotationIfSpecifiedInUrl: "readable",
};

export default [
Expand Down
2 changes: 2 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,9 @@
//////////////////////////////////////
"cSpell.words": [
"commontator",
"factorybot",
"helpdesk",
"katex",
"turbolinks"
]
}
3 changes: 2 additions & 1 deletion app/abilities/annotation_ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ def initialize(user)
annotation.user == user
end

can [:new, :create, :update_annotations, :num_nearby_posted_mistake_annotations], Annotation
can [:index, :new, :create, :update_annotations, :num_nearby_posted_mistake_annotations],
Annotation
end
end
13 changes: 13 additions & 0 deletions app/assets/javascripts/annotations_overview.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
function colorAnnotationCardsSharedByStudents() {
const annotationCards = $("[data-annotation-card-category]");
for (let card of annotationCards) {
const category = card.dataset.annotationCardCategory;
if (!category) {
continue;
}
const color = Category.getByName(category).color;
card.style.borderColor = color;
}
}

colorAnnotationCardsSharedByStudents();
1 change: 1 addition & 0 deletions app/assets/javascripts/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@
//= require thyme/metadata_manager
//= require thyme/resizer
//= require thyme/utility
//= require thyme/annotations/url_annotation_opener
//= require thyme/thyme_player
//= require thyme/thyme_editor
//= require thyme/thyme_feedback
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class AnnotationManager {
}

/*
Updates the markers on the timeline, i.e. the visual represention of the annotations.
Updates the markers on the timeline, i.e. the visual representation of the annotations.
This method is e.g. used for rearranging the markers when the window is being resized.
Don't mix up with updateAnnotatons() which sends an AJAX request and checks for changes
in the database.
Expand Down Expand Up @@ -82,7 +82,7 @@ class AnnotationManager {
This method is e.g. used when a new annotation is being created.
Don't mix up with updateMarkers() which just updates the position of the markers!
onSucess = A function that is triggered when the annotations have been
onSuccess = A function that is triggered when the annotations have been
successfully updated.
onSuccess = A function that is triggered when the annotations have been
successfully updated.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// eslint-disable-next-line no-unused-vars
function openAnnotationIfSpecifiedInUrl() {
const annotationValue = new URLSearchParams(window.location.search).get("ann");
if (!annotationValue) {
return;
}
const annotationId = Number(annotationValue);
thymeAttributes.annotationArea.showAnnotationWithId(annotationId);
}
2 changes: 1 addition & 1 deletion app/assets/javascripts/thyme/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const thymeAttributes = {
(which it is not for users who aren't signed in). */
annotationFeatureActive: false,

/* When callig the updateMarkers() method this will be used to save an
/* When calling the updateMarkers() method this will be used to save an
array containing all annotations. */
annotations: null,

Expand Down
8 changes: 6 additions & 2 deletions app/assets/javascripts/thyme/thyme_feedback.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ $(document).on("turbolinks:load", function () {
(new TimeButton("minus-ten", -10)).add();
// Sliders
(new VolumeBar("volume-bar")).add();
seekBar = new SeekBar("seek-bar");
seekBar.add();
(new SeekBar("seek-bar")).add();

// heatmap
const heatmap = new Heatmap("heatmap");
Expand Down Expand Up @@ -84,6 +83,11 @@ $(document).on("turbolinks:load", function () {
thymeAttributes.annotationManager = annotationManager;
thymeAttributes.annotationFeatureActive = true;

// update annotations manually once as initialization
annotationManager.updateAnnotations(() => {
openAnnotationIfSpecifiedInUrl();
});

/*
KEYBOARD SHORTCUTS
*/
Expand Down
5 changes: 5 additions & 0 deletions app/assets/javascripts/thyme/thyme_player.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ $(document).on("turbolinks:load", function () {
onClick, onUpdate, isValid);
thymeAttributes.annotationManager = annotationManager;

// update annotations manually once as initialization
annotationManager.updateAnnotations(() => {
openAnnotationIfSpecifiedInUrl();
});

// Update annotations after deleting an annotation
const ANNOTATION_DELETE_SELECTOR = "#annotation-delete-button";
$(document).on("click", ANNOTATION_DELETE_SELECTOR, function () {
Expand Down
8 changes: 0 additions & 8 deletions app/assets/stylesheets/annotations.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,6 @@
font-size: 1.3rem;
padding: 2px 8px;

i {
&::before {
color: transparent;
background-clip: text;
background-image: radial-gradient(at bottom right, rgb(30, 82, 141) 0%, rgb(237, 152, 189) 100%);
}
}

transition: filter 60ms ease-in-out;

&:hover {
Expand Down
15 changes: 15 additions & 0 deletions app/assets/stylesheets/annotations_overview.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
.subtle-background {
background-image: linear-gradient(30deg, #fafdff 12%, transparent 12.5%, transparent 87%, #fafdff 87.5%, #fafdff), linear-gradient(150deg, #fafdff 12%, transparent 12.5%, transparent 87%, #fafdff 87.5%, #fafdff), linear-gradient(30deg, #fafdff 12%, transparent 12.5%, transparent 87%, #fafdff 87.5%, #fafdff), linear-gradient(150deg, #fafdff 12%, transparent 12.5%, transparent 87%, #fafdff 87.5%, #fafdff), linear-gradient(60deg, #fafdff77 25%, transparent 25.5%, transparent 75%, #fafdff77 75%, #fafdff77), linear-gradient(60deg, #fafdff77 25%, transparent 25.5%, transparent 75%, #fafdff77 75%, #fafdff77);
background-size: 42px 74px;
background-position: 0 0, 0 0, 21px 37px, 21px 37px, 0 0, 21px 37px;
}

.annotation-overview-item {
border-width: 1.5px;
cursor: pointer;
transition: box-shadow 120ms cubic-bezier(0.33, 1, 0.68, 1);

&:hover {
box-shadow: rgba(0, 0, 0, 0.23) 1px 2px 8px -2px;
}
}
37 changes: 37 additions & 0 deletions app/controllers/annotations_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,18 @@
class AnnotationsController < ApplicationController
authorize_resource

def index
@annotations_by_lecture = annotations_by_lecture(current_user.own_annotations)

@show_students_annotations = current_user.teachable_editor_or_teacher?
if @show_students_annotations
@student_annotations_by_lecture =
annotations_by_lecture(current_user.students_annotations, is_shared: true)
end

render layout: "application_no_sidebar_with_background"
end

def show
@annotation = Annotation.find(params[:id])
end
Expand Down Expand Up @@ -208,4 +220,29 @@ def commontator_comment(annotation)

annotation.public_comment_id = comment.id
end

def annotations_by_lecture(annotations, is_shared: false)
annotations
.map { |a| extract_annotation_overview_information(a, is_shared) }
.group_by { |annotation| annotation[:lecture] }
.sort_by { |lecture, _annotations| lecture.updated_at }.reverse.to_h
# Instead of the full lecture object, we only need its title,
# and also not as value anymore for individual annotations.
.transform_keys(&:title)
.transform_values { |annos| annos.map { |a| a.except(:lecture) } }
.transform_values { |annos| annos.sort_by { |a| a[:created_at] }.reverse }
end

def extract_annotation_overview_information(annotation, is_shared)
{
category: annotation.category,
text: annotation.comment_optional,
color: annotation.color,
updated_at: annotation.updated_at,
lecture: annotation.medium.teachable.lecture,
link: helpers.annotation_open_link(annotation, is_shared),
medium_title: annotation.medium.caption,
medium_date: annotation.medium.lesson&.date_localized
}
end
end
3 changes: 3 additions & 0 deletions app/controllers/cypress/cypress_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ module Cypress
# The main purpose of this class is to send back errors as JSON object
# to parse them in the Cypress test UI. This way, we can display the error
# message and the stacktrace in the Cypress test.
#
# The convention with the frontend is to return the status `created`
# for successful requests and `bad_request` (or anything else) for failed requests.
class CypressController < ApplicationController
respond_to :json
rescue_from Exception, with: :show_errors
Expand Down
26 changes: 26 additions & 0 deletions app/controllers/cypress/i18n_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
module Cypress
# Allows to access i18n keys in Cypress tests.
class I18nController < CypressController
def create
unless params[:i18n_key].is_a?(String)
msg = "Argument `i18n_key` must be a string indicating the i18n key."
msg += " But we got: '#{params[:i18n_key]}'"
raise(ArgumentError, msg)
end

substitutions = {}
if params[:substitutions].present?
unless params[:substitutions].is_a?(Hash)
msg = "Argument `substitution` must be a hash indicating the substitutions."
msg += " But we got: '#{params[:substitutions]}'"
raise(ArgumentError, msg)
end
substitutions = params[:substitutions].to_unsafe_hash.symbolize_keys
end

i18n_key = params[:i18n_key]

render json: I18n.t(i18n_key, **substitutions), status: :created
end
end
end
1 change: 1 addition & 0 deletions app/controllers/media_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,7 @@ def check_annotation_visibility
# which has nothing to do with the thyme player(s).
def feedback
I18n.locale = @medium.locale_with_inheritance
@time = params[:time]
render layout: "feedback"
end

Expand Down
22 changes: 22 additions & 0 deletions app/helpers/annotations_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
require "uri"

module AnnotationsHelper
def annotation_open_link(annotation, is_shared)
link = if is_shared
feedback_video_link_timed(annotation.medium_id, annotation.timestamp)
else
video_link_timed(annotation.medium_id, annotation.timestamp)
end
link = URI.parse(link)
link.query = link.query.present? ? "#{link.query}&ann=#{annotation.id}" : "ann=#{annotation.id}"
link.to_s
end

def annotation_index_border_color(annotation, is_student_annotation)
# The border color of annotation cards shared by students, will be set
# via JS according to the color of the annotation CATEGORY.
return "" if is_student_annotation

"border-color: #{annotation[:color]}"
end
end
8 changes: 8 additions & 0 deletions app/helpers/media_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,4 +149,12 @@ def external_link_description_not_empty(medium)
# link url itself.
medium.external_link_description.presence || medium.external_reference_link
end

def video_link_timed(medium, timestamp)
play_medium_path(medium, params: { time: timestamp.total_seconds })
end

def feedback_video_link_timed(medium, timestamp)
feedback_medium_path(medium, params: { time: timestamp.total_seconds })
end
end
18 changes: 18 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -801,6 +801,19 @@ def last_sign_in_ip=(_ip)
def current_sign_in_ip=(_ip)
end

##############################################################################
# Annotations
##############################################################################

def own_annotations
Annotation.where(user: self)
end

def students_annotations
Annotation.where(medium_id: medium_ids_of_lectures_or_edited_lectures,
visible_for_teacher: true)
end

private

def set_defaults
Expand Down Expand Up @@ -848,4 +861,9 @@ def archive_user(archive_name)
confirmed_at: Time.zone.now,
archived: true)
end

def medium_ids_of_lectures_or_edited_lectures
lectures = given_lectures + edited_lectures
lectures.flat_map(&:media_with_inheritance).pluck(:id)
end
end
2 changes: 1 addition & 1 deletion app/views/annotations/_annotation_area.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<div id="annotation-infobar">
<!--- ANNOTATION CATEGORY -->
</div>
<div id="annotation-comment">
<div id="annotation-comment" data-cy="annotation-comment">
<!--- ANNOTATION COMMENT -->
</div>
<!--- ANNOTATION AREA BUTTONS -->
Expand Down
Loading

0 comments on commit 8e44e6f

Please sign in to comment.