Merge pull request #5648 from dodona-edu/feat/more-flexible-scoping

Allow more flexible reuse of annotations across different courses/exercises

app/assets/javascripts/components/saved_annotations/saved_annotation_list.ts

@@ -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>

app/controllers/saved_annotations_controller.rb

@@ -55,6 +55,8 @@ def new
   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
@@ -83,6 +85,8 @@ def update
         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
           render :edit
         end
       end

app/javascript/packs/saved_annotation.js

@@ -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";
 

app/models/saved_annotation.rb

@@ -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)
@@ -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) }

app/policies/saved_annotation_policy.rb

@@ -38,6 +38,6 @@ def destroy?
   end
 
   def permitted_attributes
-    %i[title annotation_text]
+    %i[title annotation_text course_id exercise_id]
   end
 end

app/views/saved_annotations/edit.html.erb

@@ -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">

app/views/saved_annotations/index.json.jbuilder

@@ -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

app/views/saved_annotations/show.html.erb

@@ -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">

config/locales/views/saved_annotations/en.yml

@@ -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

config/locales/views/saved_annotations/nl.yml

@@ -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

db/migrate/20240626093130_allow_null_for_saved_annotation_exercise_and_course.rb

@@ -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

db/schema.rb

@@ -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
@@ -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

test/factories/saved_annotations.rb

@@ -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)

test/models/saved_annotation_test.rb

@@ -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)
@@ -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