From a0cc2affeb3bcc607cd756106de935f4a7d69cb8 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Wed, 26 Jun 2024 10:04:19 +0200 Subject: [PATCH 1/5] Allow edditing course id --- app/controllers/saved_annotations_controller.rb | 2 ++ app/javascript/packs/saved_annotation.js | 1 + app/policies/saved_annotation_policy.rb | 2 +- app/views/saved_annotations/edit.html.erb | 8 ++++++++ config/locales/views/saved_annotations/en.yml | 2 ++ config/locales/views/saved_annotations/nl.yml | 2 ++ 6 files changed, 16 insertions(+), 1 deletion(-) diff --git a/app/controllers/saved_annotations_controller.rb b/app/controllers/saved_annotations_controller.rb index 4051969233..ee01769c5c 100644 --- a/app/controllers/saved_annotations_controller.rb +++ b/app/controllers/saved_annotations_controller.rb @@ -55,6 +55,7 @@ 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 end def create @@ -83,6 +84,7 @@ 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 render :edit end end diff --git a/app/javascript/packs/saved_annotation.js b/app/javascript/packs/saved_annotation.js index b7a58c03f1..861fa105df 100644 --- a/app/javascript/packs/saved_annotation.js +++ b/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"; diff --git a/app/policies/saved_annotation_policy.rb b/app/policies/saved_annotation_policy.rb index 9c1ed88e2f..8992f763c9 100644 --- a/app/policies/saved_annotation_policy.rb +++ b/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 diff --git a/app/views/saved_annotations/edit.html.erb b/app/views/saved_annotations/edit.html.erb index 5d5688c77a..2557e46917 100644 --- a/app/views/saved_annotations/edit.html.erb +++ b/app/views/saved_annotations/edit.html.erb @@ -33,6 +33,14 @@ <%= f.text_area :annotation_text, class: "form-control" %> <%= t ".markdown_html" %> +
+ <%= f.label :course, class: "form-label" %> + " + > +
<% end %>
diff --git a/config/locales/views/saved_annotations/en.yml b/config/locales/views/saved_annotations/en.yml index 19f801d602..5bdb6c7f93 100644 --- a/config/locales/views/saved_annotations/en.yml +++ b/config/locales/views/saved_annotations/en.yml @@ -12,5 +12,7 @@ en: 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 diff --git a/config/locales/views/saved_annotations/nl.yml b/config/locales/views/saved_annotations/nl.yml index df8185d8a1..30da0ef45d 100644 --- a/config/locales/views/saved_annotations/nl.yml +++ b/config/locales/views/saved_annotations/nl.yml @@ -12,5 +12,7 @@ nl: 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 From d76103a247a90489cc315fae07857f73141da86b Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Wed, 26 Jun 2024 11:29:43 +0200 Subject: [PATCH 2/5] Allow updating exercise --- app/controllers/saved_annotations_controller.rb | 2 ++ app/views/saved_annotations/edit.html.erb | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/app/controllers/saved_annotations_controller.rb b/app/controllers/saved_annotations_controller.rb index ee01769c5c..06a6551648 100644 --- a/app/controllers/saved_annotations_controller.rb +++ b/app/controllers/saved_annotations_controller.rb @@ -56,6 +56,7 @@ 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 @@ -85,6 +86,7 @@ def update 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 diff --git a/app/views/saved_annotations/edit.html.erb b/app/views/saved_annotations/edit.html.erb index 2557e46917..5e2eed9639 100644 --- a/app/views/saved_annotations/edit.html.erb +++ b/app/views/saved_annotations/edit.html.erb @@ -41,6 +41,14 @@ placeholder="<%= t ".course_placeholder" %>" >
+
+ <%= f.label :exercise, class: "form-label" %> + " + > +
<% end %>
From 31334cee32b2f8c153e486256ca61c64d03ec521 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Wed, 26 Jun 2024 11:32:28 +0200 Subject: [PATCH 3/5] Allow null for saved annotations --- app/models/activity_read_state.rb | 1 + app/models/saved_annotation.rb | 8 ++++---- app/models/submission.rb | 1 + db/schema.rb | 8 +++++--- test/factories/saved_annotations.rb | 4 ++-- test/factories/submissions.rb | 1 + test/models/activity_read_state_test.rb | 1 + test/models/saved_annotation_test.rb | 4 ++-- test/models/submission_test.rb | 1 + 9 files changed, 18 insertions(+), 11 deletions(-) diff --git a/app/models/activity_read_state.rb b/app/models/activity_read_state.rb index caa2ae8279..2788f9b48b 100644 --- a/app/models/activity_read_state.rb +++ b/app/models/activity_read_state.rb @@ -8,6 +8,7 @@ # user_id :integer not null # created_at :datetime not null # updated_at :datetime not null +# series_id :integer # class ActivityReadState < ApplicationRecord belongs_to :activity diff --git a/app/models/saved_annotation.rb b/app/models/saved_annotation.rb index ef7cb470f9..d2310a7c61 100644 --- a/app/models/saved_annotation.rb +++ b/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,8 +17,8 @@ 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 diff --git a/app/models/submission.rb b/app/models/submission.rb index f41c91f50a..b8523c80a4 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -14,6 +14,7 @@ # fs_key :string(24) # number :integer # annotated :boolean default(FALSE), not null +# series_id :integer # class Submission < ApplicationRecord diff --git a/db/schema.rb b/db/schema.rb index 56ec0e3916..3eacf5b77b 100644 --- a/db/schema.rb +++ b/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_03_07_124219) 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 @@ -82,6 +82,7 @@ t.integer "user_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.integer "series_id" t.index ["activity_id", "course_id", "user_id"], name: "activity_read_states_unique", unique: true t.index ["course_id"], name: "fk_rails_f674cacc14" t.index ["user_id"], name: "fk_rails_96d00253e9" @@ -420,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 @@ -499,6 +500,7 @@ t.string "fs_key", limit: 24 t.integer "number" t.boolean "annotated", default: false, null: false + t.integer "series_id" t.index ["accepted"], name: "index_submissions_on_accepted" t.index ["course_id"], name: "index_submissions_on_course_id" t.index ["exercise_id", "status", "course_id"], name: "ex_st_co_idx" diff --git a/test/factories/saved_annotations.rb b/test/factories/saved_annotations.rb index e637b575ae..f4808e6603 100644 --- a/test/factories/saved_annotations.rb +++ b/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) diff --git a/test/factories/submissions.rb b/test/factories/submissions.rb index f3a9dc1b71..3fc8ff92f1 100644 --- a/test/factories/submissions.rb +++ b/test/factories/submissions.rb @@ -14,6 +14,7 @@ # fs_key :string(24) # number :integer # annotated :boolean default(FALSE), not null +# series_id :integer # FactoryBot.define do diff --git a/test/models/activity_read_state_test.rb b/test/models/activity_read_state_test.rb index c81a8d69cc..aa5a2e47ae 100644 --- a/test/models/activity_read_state_test.rb +++ b/test/models/activity_read_state_test.rb @@ -8,6 +8,7 @@ # user_id :integer not null # created_at :datetime not null # updated_at :datetime not null +# series_id :integer # require 'test_helper' diff --git a/test/models/saved_annotation_test.rb b/test/models/saved_annotation_test.rb index 48b2ab050a..e4bcb5591b 100644 --- a/test/models/saved_annotation_test.rb +++ b/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) diff --git a/test/models/submission_test.rb b/test/models/submission_test.rb index bfa9008e8d..266f26c409 100644 --- a/test/models/submission_test.rb +++ b/test/models/submission_test.rb @@ -14,6 +14,7 @@ # fs_key :string(24) # number :integer # annotated :boolean default(FALSE), not null +# series_id :integer # require 'test_helper' From b00c53ed3c2a14395ce564fc5d65f02314777bcf Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Wed, 26 Jun 2024 11:48:07 +0200 Subject: [PATCH 4/5] Fix code working with nil values --- .../saved_annotation_list.ts | 8 ++++++-- app/models/saved_annotation.rb | 4 ++-- .../saved_annotations/index.json.jbuilder | 20 +++++++++++-------- app/views/saved_annotations/show.html.erb | 19 ++++++++++++------ config/locales/views/saved_annotations/en.yml | 4 +++- config/locales/views/saved_annotations/nl.yml | 4 +++- ...or_saved_annotation_exercise_and_course.rb | 6 ++++++ 7 files changed, 45 insertions(+), 20 deletions(-) create mode 100644 db/migrate/20240626093130_allow_null_for_saved_annotation_exercise_and_course.rb diff --git a/app/assets/javascripts/components/saved_annotations/saved_annotation_list.ts b/app/assets/javascripts/components/saved_annotations/saved_annotation_list.ts index b080421d53..4e7501fdf3 100644 --- a/app/assets/javascripts/components/saved_annotations/saved_annotation_list.ts +++ b/app/assets/javascripts/components/saved_annotations/saved_annotation_list.ts @@ -73,8 +73,12 @@ export class SavedAnnotationList extends DodonaElement { ${sa.title} ${sa.annotation_text} - ${sa.course.name} - ${sa.exercise.name} + + ${sa.course ? html`${sa.course.name}` : ""} + + + ${sa.exercise ? html`${sa.exercise.name}` : ""} + `)} diff --git a/app/models/saved_annotation.rb b/app/models/saved_annotation.rb index d2310a7c61..169cf5d915 100644 --- a/app/models/saved_annotation.rb +++ b/app/models/saved_annotation.rb @@ -24,8 +24,8 @@ class SavedAnnotation < ApplicationRecord 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) } diff --git a/app/views/saved_annotations/index.json.jbuilder b/app/views/saved_annotations/index.json.jbuilder index 46a295c250..64be67132c 100644 --- a/app/views/saved_annotations/index.json.jbuilder +++ b/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 diff --git a/app/views/saved_annotations/show.html.erb b/app/views/saved_annotations/show.html.erb index 56b056bcc9..7f44e5dd64 100644 --- a/app/views/saved_annotations/show.html.erb +++ b/app/views/saved_annotations/show.html.erb @@ -12,12 +12,19 @@

<%= @saved_annotation.title %>

<%= markdown @saved_annotation.annotation_text %>
- <%= 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 - %>
+ <%= 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 %>
<%= t ".count_info_html", count: @saved_annotation.annotations_count %>
diff --git a/config/locales/views/saved_annotations/en.yml b/config/locales/views/saved_annotations/en.yml index 5bdb6c7f93..3f2f1d729b 100644 --- a/config/locales/views/saved_annotations/en.yml +++ b/config/locales/views/saved_annotations/en.yml @@ -4,7 +4,9 @@ en: title: Saved comments show: linked_submissions: Linked submissions - usage_info_html: This comment can be reused on all submissions for the exercise %{exercise_name} in the course %{course_name}. + usage_info: This comment can be reused on all submissions + exercise_info_html: for the exercise %{exercise_name} + course_info_html: in the course %{course_name} count_info_html: This comment is used %{count} times. edit: title: Edit saved comment diff --git a/config/locales/views/saved_annotations/nl.yml b/config/locales/views/saved_annotations/nl.yml index 30da0ef45d..64f747cb75 100644 --- a/config/locales/views/saved_annotations/nl.yml +++ b/config/locales/views/saved_annotations/nl.yml @@ -4,7 +4,9 @@ nl: title: Opgeslagen opmerkingen show: linked_submissions: Gekoppelde oplossingen - usage_info_html: Deze opmerking kan worden hergebruikt op alle ingediende oplossingen voor de oefening %{exercise_name} in de cursus %{course_name}. + usage_info: Deze opmerking kan worden hergebruikt op alle ingediende oplossingen + exercise_info_html: voor de oefening %{exercise_name} + course_info_html: in de cursus %{course_name} count_info_html: Deze opmerking wordt %{count} keer gebruikt. edit: title: Bewerk opgeslagen opmerking diff --git a/db/migrate/20240626093130_allow_null_for_saved_annotation_exercise_and_course.rb b/db/migrate/20240626093130_allow_null_for_saved_annotation_exercise_and_course.rb new file mode 100644 index 0000000000..783d8b4f4e --- /dev/null +++ b/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 From c9450c45b5d144a3e744e9dacedf9c1e377c5caa Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Thu, 27 Jun 2024 09:24:31 +0200 Subject: [PATCH 5/5] add tests --- test/models/saved_annotation_test.rb | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/models/saved_annotation_test.rb b/test/models/saved_annotation_test.rb index e4bcb5591b..c413816661 100644 --- a/test/models/saved_annotation_test.rb +++ b/test/models/saved_annotation_test.rb @@ -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