From ff8e53f8861503200811bf5518ea02859450d265 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Fri, 31 Jul 2026 12:24:11 +0200 Subject: [PATCH 1/3] style(rubocop): resolve Rails/LexicallyScopedActionFilter by inlining SponsorConcerns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cop requires that `before_action` `only:` actions be explicitly defined on the class where the filter is declared. `Admin::SponsorConcerns` was only included by `Admin::WorkshopsController`, yet the `sponsor`, `destroy_sponsor`, `host`, and `destroy_host` actions were defined inside a nested `InstanceMethods` module — invisible to the cop. Inlined the concern into the controller (YAGNI — no other controller used it). Also replaced two `update_attribute` calls with `update` since the moved code was no longer covered by the SkipsModelValidations exclusion. --- .rubocop_todo.yml | 5 -- app/controllers/admin/workshops_controller.rb | 43 +++++++++++++- .../concerns/admin/sponsor_concerns.rb | 56 ------------------- 3 files changed, 41 insertions(+), 63 deletions(-) delete mode 100644 app/controllers/concerns/admin/sponsor_concerns.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 9de0e18d9..47f3a87ba 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -162,11 +162,6 @@ Rails/InverseOf: - 'app/models/member.rb' - 'app/models/workshop_invitation.rb' -# Offense count: 2 -Rails/LexicallyScopedActionFilter: - Exclude: - - 'app/controllers/concerns/admin/sponsor_concerns.rb' - # Offense count: 2 Rails/OutputSafety: Exclude: diff --git a/app/controllers/admin/workshops_controller.rb b/app/controllers/admin/workshops_controller.rb index d92f353b0..c50b523ed 100644 --- a/app/controllers/admin/workshops_controller.rb +++ b/app/controllers/admin/workshops_controller.rb @@ -1,9 +1,10 @@ class Admin::WorkshopsController < Admin::ApplicationController - include Admin::SponsorConcerns - include Admin::WorkshopConcerns + include Admin::WorkshopConcerns before_action :set_workshop_by_id, only: %i[show edit destroy update] before_action :set_and_decorate_workshop, only: %i[attendees_checklist attendees_emails send_invites changes] + before_action :set_workshop, only: %i[sponsor destroy_sponsor host destroy_host] + before_action :set_sponsor, only: %i[sponsor host] WORKSHOP_DELETION_TIME_FRAME_SINCE_CREATION = 4.hours @@ -114,6 +115,36 @@ def changes @student_invitations = invitations.to_students end + def sponsor + flash[:notice] = if workshop_sponsors.save + 'Sponsor added successfully' + else + workshop_sponsors.errors.full_messages.to_s + end + redirect_back fallback_location: root_path + end + + def destroy_sponsor + @sponsor = Sponsor.find(params[:sponsor_id]) + @workshop.workshop_sponsors.find_by(sponsor: @sponsor).destroy + redirect_back fallback_location: root_path + end + + def host + set_sponsor + @workshop_sponsor = WorkshopSponsor.find_or_create_by(workshop: @workshop, sponsor: @sponsor) + @workshop_sponsor.update(host: true) + flash[:notice] = 'Host set successfully' + + redirect_back fallback_location: root_path + end + + def destroy_host + @workshop.workshop_sponsors.find_by(host: true).update(host: false) + + redirect_back fallback_location: root_path + end + private def workshop_params @@ -202,4 +233,12 @@ def update_workshop_details assign_organisers(organiser_ids) assign_host(host_id) end + + def set_sponsor + @sponsor = Sponsor.find(params[:workshop][:sponsor_ids]) + end + + def workshop_sponsor(host = false) + @workshop_sponsor ||= WorkshopSponsor.new(workshop: @workshop, sponsor: @sponsor, host: host) + end end diff --git a/app/controllers/concerns/admin/sponsor_concerns.rb b/app/controllers/concerns/admin/sponsor_concerns.rb deleted file mode 100644 index 7105c1be0..000000000 --- a/app/controllers/concerns/admin/sponsor_concerns.rb +++ /dev/null @@ -1,56 +0,0 @@ -module Admin::SponsorConcerns - extend ActiveSupport::Concern - - included do - before_action :set_workshop, only: %i[sponsor destroy_sponsor host destroy_host] - before_action :set_sponsor, only: %i[sponsor host] - - include InstanceMethods - end - - module InstanceMethods - def sponsor - flash[:notice] = if workshop_sponsors.save - 'Sponsor added successfully' - else - workshop_sponsors.errors.full_messages.to_s - end - redirect_back fallback_location: root_path - end - - def destroy_sponsor - @sponsor = Sponsor.find(params[:sponsor_id]) - @workshop.workshop_sponsors.find_by(sponsor: @sponsor).destroy - redirect_back fallback_location: root_path - end - - def host - set_sponsor - @workshop_sponsor = WorkshopSponsor.find_or_create_by(workshop: @workshop, sponsor: @sponsor) - @workshop_sponsor.update_attribute(:host, true) - flash[:notice] = 'Host set successfully' - - redirect_back fallback_location: root_path - end - - def destroy_host - @workshop.workshop_sponsors.find_by(host: true).update_attribute(:host, false) - - redirect_back fallback_location: root_path - end - - private - - def set_workshop - @workshop = Workshop.find(params[:workshop_id]) - end - - def set_sponsor - @sponsor = Sponsor.find(params[:workshop][:sponsor_ids]) - end - - def workshop_sponsor(host = false) - @workshop_sponsor ||= WorkshopSponsor.new(workshop: @workshop, sponsor: @sponsor, host: host) - end - end -end From 1e428fda585001b2bfbe18e81f390a41984bc3d3 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Fri, 31 Jul 2026 12:24:29 +0200 Subject: [PATCH 2/3] style(rubocop): resolve Rails/InverseOf on Member.feedbacks and WorkshopInvitation.waiting_list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added missing `inverse_of` options so Rails can avoid extra queries when traversing associations in both directions. - `Member.has_many :feedbacks` → `inverse_of: :coach` (matches `Feedback.belongs_to :coach, class_name: 'Member'`) - `WorkshopInvitation.has_one :waiting_list` → `inverse_of: :invitation` (matches `WaitingList.belongs_to :invitation, class_name: 'WorkshopInvitation'`) --- .rubocop_todo.yml | 7 ------- app/models/member.rb | 2 +- app/models/workshop_invitation.rb | 2 +- 3 files changed, 2 insertions(+), 9 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 47f3a87ba..85ad6e2ef 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -155,13 +155,6 @@ Rails/HelperInstanceVariable: Exclude: - 'app/helpers/email_helper.rb' -# Offense count: 2 -# Configuration parameters: IgnoreScopes. -Rails/InverseOf: - Exclude: - - 'app/models/member.rb' - - 'app/models/workshop_invitation.rb' - # Offense count: 2 Rails/OutputSafety: Exclude: diff --git a/app/models/member.rb b/app/models/member.rb index 860a39bc7..f68b6427c 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -17,7 +17,7 @@ class Member < ApplicationRecord has_many :workshop_invitations has_many :invitations has_many :auth_services - has_many :feedbacks, foreign_key: :coach_id + has_many :feedbacks, foreign_key: :coach_id, inverse_of: :coach has_many :subscriptions has_many :groups, through: :subscriptions has_many :member_notes diff --git a/app/models/workshop_invitation.rb b/app/models/workshop_invitation.rb index ee5a7dfff..73343eae3 100644 --- a/app/models/workshop_invitation.rb +++ b/app/models/workshop_invitation.rb @@ -4,7 +4,7 @@ class WorkshopInvitation < ApplicationRecord belongs_to :workshop belongs_to :member belongs_to :overrider, foreign_key: :last_overridden_by_id, class_name: 'Member', inverse_of: false, optional: true - has_one :waiting_list, foreign_key: :invitation_id + has_one :waiting_list, foreign_key: :invitation_id, inverse_of: :invitation validates :workshop, :member, presence: true validates :member_id, uniqueness: { scope: %i[workshop_id role] } From 3d5e0a04a074e60f79c09643909551969c8a378c Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Fri, 31 Jul 2026 12:25:15 +0200 Subject: [PATCH 3/3] style(rubocop): resolve Rails/OutputSafety with documented disable comments Both call sites intentionally mark strings as `html_safe` because the content is already safe at generation time. Added `rubocop:disable/enable` blocks with inline justifications rather than leaving them in the todo file. - `ApplicationHelper#dot_markdown`: Commonmarker is a trusted markdown parser; its output is already safe HTML. - `AddressPresenter#to_html`: Each address line is individually `ERB::Util.html_escape'd before joining with `
`. --- .rubocop_todo.yml | 6 ------ app/helpers/application_helper.rb | 3 +++ app/presenters/address_presenter.rb | 3 +++ 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 85ad6e2ef..1e32c980b 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -155,12 +155,6 @@ Rails/HelperInstanceVariable: Exclude: - 'app/helpers/email_helper.rb' -# Offense count: 2 -Rails/OutputSafety: - Exclude: - - 'app/helpers/application_helper.rb' - - 'app/presenters/address_presenter.rb' - # Offense count: 9 # Configuration parameters: ForbiddenMethods, AllowedMethods. # ForbiddenMethods: decrement!, decrement_counter, increment!, increment_counter, insert, insert!, insert_all, insert_all!, toggle!, touch, touch_all, update_all, update_attribute, update_column, update_columns, update_counters, upsert, upsert_all diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 663149dad..72c890099 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -20,7 +20,10 @@ def retrieve_title end def dot_markdown(text) + # Commonmarker sanitises raw HTML; `.html_safe` prevents Rails double-escaping the result + # rubocop:disable Rails/OutputSafety Commonmarker.to_html(text).html_safe + # rubocop:enable Rails/OutputSafety end def belongs_to_group?(group) diff --git a/app/presenters/address_presenter.rb b/app/presenters/address_presenter.rb index 8915e6475..b029a1a0a 100644 --- a/app/presenters/address_presenter.rb +++ b/app/presenters/address_presenter.rb @@ -5,10 +5,13 @@ def to_html city_and_postal_code = [model.city, model.postal_code].delete_if(&:empty?) .join(', ') + # Every element is html_escape'd; `.html_safe` prevents Rails double-escaping the joined string + # rubocop:disable Rails/OutputSafety [model.flat, model.street, city_and_postal_code, lat, lng] .delete_if(&:empty?) .map { |line| ERB::Util.html_escape(line) } .join('
').html_safe + # rubocop:enable Rails/OutputSafety end def for_map