From e476bececd244baa1f70df2f82332dfbeb52d499 Mon Sep 17 00:00:00 2001 From: Splines Date: Mon, 15 Apr 2024 22:59:40 +0200 Subject: [PATCH 1/4] Update Rubocop (and plugins) to latest versions --- Gemfile | 6 +++--- Gemfile.lock | 33 +++++++++++++++++---------------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/Gemfile b/Gemfile index aa44445ae..ff97daa92 100644 --- a/Gemfile +++ b/Gemfile @@ -111,9 +111,9 @@ group :development, :docker_development do # Spring speeds up development by keeping your application running in the background. Read more: https://github.com/rails/spring gem "marcel" gem "pgreset" - gem "rubocop", "~> 1.57", require: false - gem "rubocop-performance", "~> 1.16", require: false - gem "rubocop-rails", "~> 2.22", ">= 2.22.1", require: false + gem "rubocop", "~> 1.63", require: false + gem "rubocop-performance", "~> 1.21", require: false + gem "rubocop-rails", "~> 2.24", require: false gem "spring" gem "spring-watcher-listen", "~> 2.0.0" # gem 'bullet' diff --git a/Gemfile.lock b/Gemfile.lock index 946c11b31..59d98cf0f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -284,7 +284,7 @@ GEM js-routes (1.4.9) railties (>= 4) sprockets-rails - json (2.6.3) + json (2.7.2) jsonapi-renderer (0.2.2) kaminari (1.2.2) activesupport (>= 4.1.0) @@ -359,8 +359,8 @@ GEM options (2.3.2) orm_adapter (0.5.0) pairing_heap (3.0.0) - parallel (1.23.0) - parser (3.2.2.4) + parallel (1.24.0) + parser (3.3.0.5) ast (~> 2.4.1) racc pdf-reader (2.11.0) @@ -443,7 +443,7 @@ GEM ffi (~> 1.0) redis-client (0.14.1) connection_pool - regexp_parser (2.8.2) + regexp_parser (2.9.0) request_store (1.5.1) rack (>= 1.4) responders (3.1.0) @@ -484,26 +484,27 @@ GEM rspec-mocks (~> 3.11) rspec-support (~> 3.11) rspec-support (3.12.0) - rubocop (1.57.2) + rubocop (1.63.1) json (~> 2.3) language_server-protocol (>= 3.17.0) parallel (~> 1.10) - parser (>= 3.2.2.4) + parser (>= 3.3.0.2) rainbow (>= 2.2.2, < 4.0) regexp_parser (>= 1.8, < 3.0) rexml (>= 3.2.5, < 4.0) - rubocop-ast (>= 1.28.1, < 2.0) + rubocop-ast (>= 1.31.1, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 2.4.0, < 3.0) - rubocop-ast (1.30.0) - parser (>= 3.2.1.0) - rubocop-performance (1.19.1) - rubocop (>= 1.7.0, < 2.0) - rubocop-ast (>= 0.4.0) - rubocop-rails (2.22.1) + rubocop-ast (1.31.2) + parser (>= 3.3.0.4) + rubocop-performance (1.21.0) + rubocop (>= 1.48.1, < 2.0) + rubocop-ast (>= 1.31.1, < 2.0) + rubocop-rails (2.24.1) activesupport (>= 4.2.0) rack (>= 1.1) rubocop (>= 1.33.0, < 2.0) + rubocop-ast (>= 1.31.1, < 2.0) ruby-graphviz (1.2.5) rexml ruby-progressbar (1.13.0) @@ -701,9 +702,9 @@ DEPENDENCIES rgl rqrcode rspec-rails - rubocop (~> 1.57) - rubocop-performance (~> 1.16) - rubocop-rails (~> 2.22, >= 2.22.1) + rubocop (~> 1.63) + rubocop-performance (~> 1.21) + rubocop-rails (~> 2.24) rubyzip (~> 2.3.0) sass-rails (>= 6) selenium-webdriver From c9c28fade3614b3ebd41e5d3a3b28a4e22eded22 Mon Sep 17 00:00:00 2001 From: Splines Date: Mon, 15 Apr 2024 23:02:51 +0200 Subject: [PATCH 2/4] Autocorrect safe cops One rule was disabled since we don't want to alter already performed migrations. --- app/controllers/commontator/comments_controller.rb | 2 +- app/helpers/media_helper.rb | 2 +- app/models/referral.rb | 2 +- db/migrate/20201002094500_change_submission_foreign_keys.rb | 2 ++ 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/controllers/commontator/comments_controller.rb b/app/controllers/commontator/comments_controller.rb index 331919345..43d668ecb 100644 --- a/app/controllers/commontator/comments_controller.rb +++ b/app/controllers/commontator/comments_controller.rb @@ -156,7 +156,7 @@ def upvote # PUT /comments/1/downvote def downvote - security_transgression_unless(@comment.can_be_voted_on_by?(@commontator_user) && \ + security_transgression_unless(@comment.can_be_voted_on_by?(@commontator_user) && @comment.thread.config.comment_voting.to_sym == :ld) @comment.downvote_from(@commontator_user) diff --git a/app/helpers/media_helper.rb b/app/helpers/media_helper.rb index 6cc9c5140..fe432a51f 100644 --- a/app/helpers/media_helper.rb +++ b/app/helpers/media_helper.rb @@ -147,6 +147,6 @@ def edit_or_show_medium_path(medium) def external_link_description_not_empty(medium) # Uses link display name if not empty, otherwise falls back to the # link url itself. - (medium.external_link_description.presence || medium.external_reference_link) + medium.external_link_description.presence || medium.external_reference_link end end diff --git a/app/models/referral.rb b/app/models/referral.rb index 769d0a116..59f70ad6f 100644 --- a/app/models/referral.rb +++ b/app/models/referral.rb @@ -33,7 +33,7 @@ def vtt_time_span # provide metadata for vtt file def vtt_properties - link = (item.link.presence || item.medium_link) + link = item.link.presence || item.medium_link # at the moment, relations between items can be only of the form # script <-> video, which means that between them there will be at most # one script, one manuscript and one video diff --git a/db/migrate/20201002094500_change_submission_foreign_keys.rb b/db/migrate/20201002094500_change_submission_foreign_keys.rb index f8c2c3d59..c82a3a37f 100644 --- a/db/migrate/20201002094500_change_submission_foreign_keys.rb +++ b/db/migrate/20201002094500_change_submission_foreign_keys.rb @@ -1,4 +1,5 @@ # rubocop:disable Rails/ +# rubocop:disable Lint/SymbolConversion class ChangeSubmissionForeignKeys < ActiveRecord::Migration[6.0] def up remove_index :user_submission_joins, @@ -33,3 +34,4 @@ def id_to_uuid(table_name, relation_name, relation_class) end end # rubocop:enable Rails/ +# rubocop:enable Lint/SymbolConversion From afccc70b01f7866c974b72ef8d605c2d6b35a68c Mon Sep 17 00:00:00 2001 From: Splines Date: Mon, 15 Apr 2024 23:11:59 +0200 Subject: [PATCH 3/4] Autocorrect unsafe cops Special care is needed for `tag.rb` around line 190. The autofix did not produce a valid result, I had to put extra curly braces around that line as we want to push an object to the array. --- app/controllers/readers_controller.rb | 5 +- app/models/manuscript.rb | 75 ++++++++++++--------------- app/models/medium_publisher.rb | 11 ++-- app/models/quiz.rb | 2 +- app/models/submission.rb | 2 +- app/models/tag.rb | 8 ++- 6 files changed, 45 insertions(+), 58 deletions(-) diff --git a/app/controllers/readers_controller.rb b/app/controllers/readers_controller.rb index 931b93a45..ed0a41de6 100644 --- a/app/controllers/readers_controller.rb +++ b/app/controllers/readers_controller.rb @@ -21,9 +21,8 @@ def update_all .map(&:commontator_thread) existing_readers = Reader.where(user: current_user, thread: threads) missing_thread_ids = threads.map(&:id) - existing_readers.pluck(:thread_id) - new_readers = [] - missing_thread_ids.each do |t| - new_readers << Reader.new(thread_id: t, user: current_user) + new_readers = missing_thread_ids.map do |t| + Reader.new(thread_id: t, user: current_user) end Reader.import new_readers Reader.where(user: current_user, thread: threads).touch_all diff --git a/app/models/manuscript.rb b/app/models/manuscript.rb index 3316d5fb7..13317cb1d 100644 --- a/app/models/manuscript.rb +++ b/app/models/manuscript.rb @@ -148,19 +148,16 @@ def create_or_update_chapter_items! attrs = [:medium_id, :pdf_destination, :section_id, :sort, :page, :description, :ref_number, :position, :quarantine] item_details = items.pluck(*attrs).map { |i| attrs.zip(i).to_h } - contents = [] - @chapters.each do |c| - contents.push( - { medium_id: @medium.id, - pdf_destination: c["destination"], - section_id: nil, - sort: "chapter", - page: c["page"].to_i, - description: c["description"], - ref_number: c["label"], - position: nil, - quarantine: nil } - ) + contents = @chapters.map do |c| + { medium_id: @medium.id, + pdf_destination: c["destination"], + section_id: nil, + sort: "chapter", + page: c["page"].to_i, + description: c["description"], + ref_number: c["label"], + position: nil, + quarantine: nil } end create_or_update_items!(contents, item_details, item_destinations, item_id_map) @@ -176,21 +173,18 @@ def create_or_update_section_items! attrs = [:medium_id, :pdf_destination, :section_id, :sort, :page, :description, :ref_number, :position, :quarantine] item_details = items.pluck(*attrs).map { |i| attrs.zip(i).to_h } - contents = [] # NOTE: that sections get a position -1 in order to place them ahead # of all content items within themseleves in #script_items_by_position - @sections.each do |s| - contents.push( - { medium_id: @medium.id, - pdf_destination: s["destination"], - section_id: s["mampf_section"].id, - sort: "section", - page: s["page"].to_i, - description: s["description"], - ref_number: s["label"], - position: -1, - quarantine: nil } - ) + contents = @sections.map do |s| + { medium_id: @medium.id, + pdf_destination: s["destination"], + section_id: s["mampf_section"].id, + sort: "section", + page: s["page"].to_i, + description: s["description"], + ref_number: s["label"], + position: -1, + quarantine: nil } end create_or_update_items!(contents, item_details, item_destinations, item_id_map) @@ -208,22 +202,19 @@ def create_or_update_content_items!(filter_boxes) attrs = [:medium_id, :pdf_destination, :section_id, :sort, :page, :description, :ref_number, :position, :hidden, :quarantine] item_details = items.pluck(*attrs).map { |i| attrs.zip(i).to_h } - contents = [] - @content.each do |c| - contents.push( - { medium_id: @medium.id, - pdf_destination: c["destination"], - section_id: @sections.find do |s| - c["section"] == s["section"] - end ["mampf_section"]&.id, - sort: Item.internal_sort(c["sort"]), - page: c["page"].to_i, - description: c["description"], - ref_number: c["label"], - position: c["counter"], - hidden: filter_boxes[c["counter"]].third == false, - quarantine: nil } - ) + contents = @content.map do |c| + { medium_id: @medium.id, + pdf_destination: c["destination"], + section_id: @sections.find do |s| + c["section"] == s["section"] + end ["mampf_section"]&.id, + sort: Item.internal_sort(c["sort"]), + page: c["page"].to_i, + description: c["description"], + ref_number: c["label"], + position: c["counter"], + hidden: filter_boxes[c["counter"]].third == false, + quarantine: nil } end create_or_update_items!(contents, item_details, item_destinations, item_id_map) diff --git a/app/models/medium_publisher.rb b/app/models/medium_publisher.rb index 3607359e3..ea13cb70f 100644 --- a/app/models/medium_publisher.rb +++ b/app/models/medium_publisher.rb @@ -128,13 +128,12 @@ def realize_optional_stuff! # to the medium's teachable's media_scope def create_notifications! @medium.teachable&.media_scope&.touch - notifications = [] @medium.teachable.media_scope.users.touch_all - @medium.teachable.media_scope.users.each do |u| - notifications << Notification.new(recipient: u, - notifiable_id: @medium.id, - notifiable_type: "Medium", - action: "create") + notifications = @medium.teachable.media_scope.users.map do |u| + Notification.new(recipient: u, + notifiable_id: @medium.id, + notifiable_type: "Medium", + action: "create") end Notification.import notifications end diff --git a/app/models/quiz.rb b/app/models/quiz.rb index 4d03afe8c..f4935b447 100644 --- a/app/models/quiz.rb +++ b/app/models/quiz.rb @@ -115,7 +115,7 @@ def preselected_hide_solution(vertex_id, crosses) def questions ids = quiz_graph&.vertices&.values&.select { |v| v[:type] == "Question" } - &.map { |v| v[:id] } + &.pluck(:id) Question.where(id: ids) end diff --git a/app/models/submission.rb b/app/models/submission.rb index 61c640286..ecdfa44b5 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -63,7 +63,7 @@ def correction_size end def preceding_tutorial(user) - assignment.previous&.map { |a| a.tutorial(user) }&.compact&.first + assignment.previous&.filter_map { |a| a.tutorial(user) }&.first end def invited_users diff --git a/app/models/tag.rb b/app/models/tag.rb index c92002ec0..d506e1a4f 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -184,13 +184,11 @@ def self.select_by_title_except(excluded_tags) # converts the subgraph of all tags of distance <= 2 to the given marked tag # into a cytoscape array representing this subgraph def self.to_cytoscape(tags, marked_tag, highlight_related_tags: true) - result = [] # add vertices - tags.each do |t| - result.push(data: t.cytoscape_vertex(marked_tag, - highlight_related_tags: - highlight_related_tags)) + result = tags.map do |t| + { data: t.cytoscape_vertex(marked_tag, highlight_related_tags: highlight_related_tags) } end + # add edges edges = [] tags.each do |t| From 456239f8aba8d7ef3c87d20c9d2e50f59085a022 Mon Sep 17 00:00:00 2001 From: Splines Date: Wed, 17 Apr 2024 15:10:35 +0200 Subject: [PATCH 4/4] Remove unnecessary Unique validation RuboCop TODO They probably got unnecessary due to the update of RuboCop, though I haven't looked into their changelog to confirm. --- app/models/notion.rb | 2 +- app/models/term.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/notion.rb b/app/models/notion.rb index 15d339ef9..fbc1cd4e7 100644 --- a/app/models/notion.rb +++ b/app/models/notion.rb @@ -2,7 +2,7 @@ class Notion < ApplicationRecord belongs_to :tag, optional: true, touch: true belongs_to :aliased_tag, class_name: "Tag", optional: true, touch: true - validates :title, uniqueness: { scope: :locale } # rubocop:todo Rails/UniqueValidationWithoutIndex + validates :title, uniqueness: { scope: :locale } validates :title, presence: true validate :presence_of_tag, if: :persisted? diff --git a/app/models/term.rb b/app/models/term.rb index 28999bcea..1fe8d72bb 100644 --- a/app/models/term.rb +++ b/app/models/term.rb @@ -4,7 +4,7 @@ class Term < ApplicationRecord has_many :lectures # season can only be SS/WS, and there can be only one of this type each year - validates :season, presence: true, # rubocop:todo Rails/UniqueValidationWithoutIndex + validates :season, presence: true, inclusion: { in: ["SS", "WS"] }, uniqueness: { scope: :year } # a year >=2000 needs to be present