Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow more flexible reuse of annotations across different courses/exercises #5648

Merged
merged 6 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,12 @@ export class SavedAnnotationList extends DodonaElement {
<a href="${sa.url}">${sa.title}</a>
</td>
<td class="ellipsis-overflow" title="${sa.annotation_text}">${sa.annotation_text}</td>
<td><d-filter-button param="course_id" value="${sa.course.id}">${sa.course.name}</td></td>
<td><d-filter-button param="exercise_id" value="${sa.exercise.id}">${sa.exercise.name}</td></td>
<td>
${sa.course ? html`<d-filter-button param="course_id" value="${sa.course.id}">${sa.course.name}</d-filter-button>` : ""}
</td>
<td>
${sa.exercise ? html`<d-filter-button param="exercise_id" value="${sa.exercise.id}">${sa.exercise.name}</d-filter-button>` : ""}
</td>
</tr>
`)}
</tbody>
Expand Down
4 changes: 4 additions & 0 deletions app/controllers/saved_annotations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@
def edit
@title = I18n.t('saved_annotations.edit.title')
@crumbs = [[I18n.t('saved_annotations.index.title'), saved_annotations_path], [@saved_annotation.title, saved_annotation_path(@saved_annotation)], [I18n.t('saved_annotations.edit.title'), '#']]
@courses = current_user.administrating_courses
@exercises = @courses.map(&:activities).flatten
end

def create
Expand Down Expand Up @@ -83,6 +85,8 @@
format.json { render json: @saved_annotation.errors.full_messages, status: :unprocessable_entity }
format.html do
@crumbs = [[I18n.t('saved_annotations.index.title'), saved_annotations_path], [@saved_annotation.title, saved_annotation_path(@saved_annotation)], [I18n.t('saved_annotations.edit.title'), '#']]
@courses = current_user.administrating_courses
@exercises = @courses.map(&:activities).flatten

Check warning on line 89 in app/controllers/saved_annotations_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/saved_annotations_controller.rb#L88-L89

Added lines #L88 - L89 were not covered by tests
bmesuere marked this conversation as resolved.
Show resolved Hide resolved
render :edit
end
end
Expand Down
1 change: 1 addition & 0 deletions app/javascript/packs/saved_annotation.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import "components/saved_annotations/saved_annotation_title_input";
import "components/saved_annotations/new_saved_annotation";
import "components/search/sort_button";
import "components/saved_annotations/new_saved_annotation_link";
import "components/datalist_input";
import { initNewSavedAnnotationButtons } from "components/saved_annotations/new_saved_annotation";
import { search } from "search";

Expand Down
12 changes: 6 additions & 6 deletions app/models/saved_annotation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
# title :string(255) not null
# annotation_text :text(16777215)
# user_id :integer not null
# exercise_id :integer not null
# course_id :integer not null
# exercise_id :integer
# course_id :integer
# created_at :datetime not null
# updated_at :datetime not null
# annotations_count :integer default(0)
Expand All @@ -17,15 +17,15 @@ class SavedAnnotation < ApplicationRecord
validates :annotation_text, presence: true

belongs_to :user
belongs_to :exercise
belongs_to :course
belongs_to :exercise, optional: true
belongs_to :course, optional: true

has_many :annotations, dependent: :nullify
has_many :submissions, through: :annotations

scope :by_user, ->(user_id) { where user_id: user_id }
scope :by_course, ->(course_id) { where course_id: course_id }
scope :by_exercise, ->(exercise_id) { where exercise_id: exercise_id }
scope :by_course, ->(course_id) { where(course_id: course_id).or(where(course_id: nil)) }
scope :by_exercise, ->(exercise_id) { where(exercise_id: exercise_id).or(where(exercise_id: nil)) }
scope :by_filter, ->(filter) { where 'title LIKE ? or annotation_text LIKE ?', "%#{filter}%", "%#{filter}%" }

scope :order_by_annotations_count, ->(direction) { reorder(annotations_count: direction) }
Expand Down
2 changes: 1 addition & 1 deletion app/policies/saved_annotation_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,6 @@ def destroy?
end

def permitted_attributes
%i[title annotation_text]
%i[title annotation_text course_id exercise_id]
end
end
16 changes: 16 additions & 0 deletions app/views/saved_annotations/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,22 @@
<%= f.text_area :annotation_text, class: "form-control" %>
<span class="help-block"><%= t ".markdown_html" %></span>
</div>
<div class="field form-group">
<%= f.label :course, class: "form-label" %>
<d-datalist-input name="saved_annotation[course_id]"
value="<%= @saved_annotation.course_id %>"
options="<%= @courses.map { |c| { value: c.id.to_s, label: c.name } }.to_json %>"
placeholder="<%= t ".course_placeholder" %>"
></d-datalist-input>
</div>
<div class="field form-group">
<%= f.label :exercise, class: "form-label" %>
<d-datalist-input name="saved_annotation[exercise_id]"
value="<%= @saved_annotation.exercise_id %>"
options="<%= @exercises.map { |c| { value: c.id.to_s, label: c.name } }.to_json %>"
placeholder="<%= t ".exercise_placeholder" %>"
></d-datalist-input>
</div>
<% end %>
</div>
<div class="card-actions card-border">
Expand Down
20 changes: 12 additions & 8 deletions app/views/saved_annotations/index.json.jbuilder
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,18 @@ json.array! @saved_annotations do |saved_annotation|
json.url user_url(saved_annotation.user)
json.id saved_annotation.user.id
end
json.exercise do
json.name saved_annotation.exercise.name
json.url activity_url(saved_annotation.exercise)
json.id saved_annotation.exercise.id
if saved_annotation.exercise.present?
json.exercise do
json.name saved_annotation.exercise.name
json.url activity_url(saved_annotation.exercise)
json.id saved_annotation.exercise.id
end
end
json.course do
json.name saved_annotation.course.name
json.url course_url(saved_annotation.course)
json.id saved_annotation.course.id
if saved_annotation.course.present?
json.course do
json.name saved_annotation.course.name
json.url course_url(saved_annotation.course)
json.id saved_annotation.course.id
end
end
end
19 changes: 13 additions & 6 deletions app/views/saved_annotations/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,19 @@
<div class="card-supporting-text">
<p><strong><%= @saved_annotation.title %></strong></p>
<blockquote><%= markdown @saved_annotation.annotation_text %></blockquote>
<%= t ".usage_info_html",
exercise_path: course_activity_path(@saved_annotation.course ,@saved_annotation.exercise),
exercise_name: @saved_annotation.exercise.name,
course_path: course_path(@saved_annotation.course),
course_name: @saved_annotation.course.name
%><br/>
<%= t ".usage_info" %>
<% if @saved_annotation.exercise.present? %>
<%= t ".exercise_info_html",
exercise_path: activity_scoped_path(activity: @saved_annotation.exercise, course: @saved_annotation.course),
exercise_name: @saved_annotation.exercise.name
%>
<% end %>
<% if @saved_annotation.course.present? %>
<%= t ".course_info_html",
course_path: course_path(@saved_annotation.course),
course_name: @saved_annotation.course.name
%>
<% end %><br/>
<%= t ".count_info_html", count: @saved_annotation.annotations_count %>
</div>
<div class="card-supporting-text card-border">
Expand Down
6 changes: 5 additions & 1 deletion config/locales/views/saved_annotations/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,17 @@ en:
title: Saved comments
show:
linked_submissions: Linked submissions
usage_info_html: This comment can be reused on all submissions for the exercise <a href="%{exercise_path}">%{exercise_name}</a> in the course <a href="%{course_path}">%{course_name}</a>.
usage_info: This comment can be reused on all submissions
exercise_info_html: for the exercise <a href="%{exercise_path}">%{exercise_name}</a>
course_info_html: in the course <a href="%{course_path}">%{course_name}</a>
count_info_html: This comment is used %{count} times.
edit:
title: Edit saved comment
markdown_html: <a href="https://docs.dodona.be/en/references/exercise-description/#markdown" target="_blank">Markdown</a> is supported.
warning_no_annotations_changed: Existing comments will not be updated. These changes will only be applied to new comments.
destroy: Delete
save: Save
course_placeholder: This comment is reusable in all courses.
exercise_placeholder: This comment is reusable in all exercises.
new:
title: Save comment
6 changes: 5 additions & 1 deletion config/locales/views/saved_annotations/nl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,17 @@ nl:
title: Opgeslagen opmerkingen
show:
linked_submissions: Gekoppelde oplossingen
usage_info_html: Deze opmerking kan worden hergebruikt op alle ingediende oplossingen voor de oefening <a href="%{exercise_path}">%{exercise_name}</a> in de cursus <a href="%{course_path}">%{course_name}</a>.
usage_info: Deze opmerking kan worden hergebruikt op alle ingediende oplossingen
exercise_info_html: voor de oefening <a href="%{exercise_path}">%{exercise_name}</a>
course_info_html: in de cursus <a href="%{course_path}">%{course_name}</a>
count_info_html: Deze opmerking wordt <strong>%{count}</strong> keer gebruikt.
edit:
title: Bewerk opgeslagen opmerking
markdown_html: <a href="https://docs.dodona.be/nl/references/exercise-description/#markdown" target="_blank">Markdown</a> wordt ondersteund.
warning_no_annotations_changed: Bestaande opmerkingen zullen niet worden aangepast. Deze wijzigingen zullen enkel worden toegepast op nieuwe opmerkingen.
destroy: Verwijderen
save: Opslaan
course_placeholder: Deze opmerking is herbruikbaar in alle cursussen.
exercise_placeholder: Deze opmerking is herbruikbaar in alle oefeningen.
new:
title: Opmerking opslaan
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class AllowNullForSavedAnnotationExerciseAndCourse < ActiveRecord::Migration[7.1]
def change
change_column_null :saved_annotations, :exercise_id, true
change_column_null :saved_annotations, :course_id, true
end
end
6 changes: 3 additions & 3 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.1].define(version: 2024_06_17_141422) do
ActiveRecord::Schema[7.1].define(version: 2024_06_26_093130) do
create_table "active_storage_attachments", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t|
t.string "name", null: false
t.string "record_type", null: false
Expand Down Expand Up @@ -421,8 +421,8 @@
t.string "title", null: false
t.text "annotation_text", size: :medium
t.integer "user_id", null: false
t.integer "exercise_id", null: false
t.integer "course_id", null: false
t.integer "exercise_id"
t.integer "course_id"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.integer "annotations_count", default: 0
Expand Down
4 changes: 2 additions & 2 deletions test/factories/saved_annotations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
# title :string(255) not null
# annotation_text :text(16777215)
# user_id :integer not null
# exercise_id :integer not null
# course_id :integer not null
# exercise_id :integer
# course_id :integer
# created_at :datetime not null
# updated_at :datetime not null
# annotations_count :integer default(0)
Expand Down
28 changes: 26 additions & 2 deletions test/models/saved_annotation_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
# title :string(255) not null
# annotation_text :text(16777215)
# user_id :integer not null
# exercise_id :integer not null
# course_id :integer not null
# exercise_id :integer
# course_id :integer
# created_at :datetime not null
# updated_at :datetime not null
# annotations_count :integer default(0)
Expand All @@ -18,4 +18,28 @@ class SavedAnnotationTest < ActiveSupport::TestCase
# test "the truth" do
# assert true
# end

test 'filtering by course_id should contain nil values' do
user = create :user
course = create :course
s1 = create :saved_annotation, course: nil, user: user
s2 = create :saved_annotation, course: course, user: user
s3 = create :saved_annotation, course: create(:course), user: user

assert_includes SavedAnnotation.by_course(course), s1
assert_includes SavedAnnotation.by_course(course), s2
assert_not_includes SavedAnnotation.by_course(course), s3
end

test 'filtering by exercise_id should contain nil values' do
user = create :user
exercise = create :exercise
s1 = create :saved_annotation, exercise: nil, user: user
s2 = create :saved_annotation, exercise: exercise, user: user
s3 = create :saved_annotation, exercise: create(:exercise), user: user

assert_includes SavedAnnotation.by_exercise(exercise), s1
assert_includes SavedAnnotation.by_exercise(exercise), s2
assert_not_includes SavedAnnotation.by_exercise(exercise), s3
end
end