From dfdad5d0ca7968e93f857755f2490adace07ed56 Mon Sep 17 00:00:00 2001 From: Splines <37160523+Splines@users.noreply.github.com> Date: Wed, 17 Apr 2024 22:53:56 +0200 Subject: [PATCH] Update Rubocop version and enforce all cops (#617) * Update Rubocop (and plugins) to latest versions * Autocorrect safe cops One rule was disabled since we don't want to alter already performed migrations. * 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. * 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. --- Gemfile.lock | 11 ++- .../commontator/comments_controller.rb | 2 +- app/controllers/readers_controller.rb | 5 +- app/helpers/media_helper.rb | 2 +- app/models/manuscript.rb | 75 ++++++++----------- app/models/medium_publisher.rb | 11 ++- app/models/notion.rb | 2 +- app/models/quiz.rb | 2 +- app/models/referral.rb | 2 +- app/models/submission.rb | 2 +- app/models/tag.rb | 8 +- app/models/term.rb | 2 +- ...02094500_change_submission_foreign_keys.rb | 2 + 13 files changed, 62 insertions(+), 64 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index f1a5bd214..ecdd3a132 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -484,17 +484,25 @@ GEM rspec-mocks (~> 3.11) rspec-support (~> 3.11) rspec-support (3.12.0) - rubocop (1.63.0) + rubocop (1.63.1) json (~> 2.3) language_server-protocol (>= 3.17.0) parallel (~> 1.10) parser (>= 3.3.0.2) + 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.31.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.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) rubocop-ast (1.31.2) parser (>= 3.3.0.4) rubocop-performance (1.21.0) @@ -505,6 +513,7 @@ GEM rack (>= 1.1) rubocop (>= 1.33.0, < 2.0) rubocop-ast (>= 1.31.1, < 2.0) + rubocop-ast (>= 1.31.1, < 2.0) ruby-graphviz (1.2.5) rexml ruby-progressbar (1.13.0) 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/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/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/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/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/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/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/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| 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 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