From 2a4c154b3d02223c4527359a4711c61794d42e2a Mon Sep 17 00:00:00 2001 From: Luke Arndt Date: Tue, 24 Sep 2024 16:39:16 +1200 Subject: [PATCH 01/10] Install leftovers gem for unused code analysis --- Gemfile | 1 + Gemfile.lock | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/Gemfile b/Gemfile index d80ceb7d..0acf3d62 100644 --- a/Gemfile +++ b/Gemfile @@ -50,6 +50,7 @@ group :development, :test do gem "factory_bot_rails", "~> 6.0" gem "faker" gem "i18n-tasks", "~> 0.9.6" + gem "leftovers", require: false gem "rspec-rails" end diff --git a/Gemfile.lock b/Gemfile.lock index 37b814c5..c5fdcb2b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -141,6 +141,7 @@ GEM faraday-net_http (>= 2.0, < 3.2) faraday-net_http (3.1.0) net-http + fast_ignore (0.17.4) ffi (1.16.3) fog-aws (3.21.0) fog-core (~> 2.1) @@ -202,6 +203,10 @@ GEM language_server-protocol (3.17.0.3) launchy (2.5.2) addressable (~> 2.8) + leftovers (0.12.2) + fast_ignore (>= 0.17.0) + parallel + parser letter_opener (1.8.1) launchy (>= 2.2, < 3) letter_opener_web (2.0.0) @@ -479,6 +484,7 @@ DEPENDENCIES jsonapi-serializer kaminari launchy + leftovers letter_opener_web listen net-imap From d58dd888e8307a0f04aa2d038262974f3e98dbe7 Mon Sep 17 00:00:00 2001 From: Luke Arndt Date: Tue, 24 Sep 2024 16:41:36 +1200 Subject: [PATCH 02/10] Add Analysis build step with leftovers job --- .github/workflows/analysis.yml | 27 +++++++++++++++++++++++++++ bin/leftovers | 16 ++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 .github/workflows/analysis.yml create mode 100755 bin/leftovers diff --git a/.github/workflows/analysis.yml b/.github/workflows/analysis.yml new file mode 100644 index 00000000..f33bcd1a --- /dev/null +++ b/.github/workflows/analysis.yml @@ -0,0 +1,27 @@ +name: Analysis + +on: + push: + branches: + - master + pull_request: + +jobs: + test: + strategy: + fail-fast: false + max-parallel: 20 + runs-on: ubuntu-latest + + env: + CI: true + RAILS_ENV: test + + name: leftovers + steps: + - uses: actions/checkout@v2 + - uses: ruby/setup-ruby@v1 + with: + bundler-cache: true + + - run: bin/leftovers diff --git a/bin/leftovers b/bin/leftovers new file mode 100755 index 00000000..e02990d7 --- /dev/null +++ b/bin/leftovers @@ -0,0 +1,16 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +# The application 'leftovers' is installed as part of a gem, and +# this file is here to facilitate running it. + +require "pathname" +ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", + Pathname.new(__FILE__).realpath) + +bundle_binstub = File.expand_path("../bundle", __FILE__) + +require "rubygems" +require "bundler/setup" + +load Gem.bin_path("leftovers", "leftovers") From 9f8d21bbb4e85353f595b389432cbb481e3159c9 Mon Sep 17 00:00:00 2001 From: Luke Arndt Date: Tue, 24 Sep 2024 16:43:28 +1200 Subject: [PATCH 03/10] Add leftovers:keep comments to code I'm confident in --- app/channels/application_cable/channel.rb | 2 +- app/channels/application_cable/connection.rb | 2 +- app/controllers/application_controller.rb | 2 +- app/controllers/impact_omniauth_callbacks_controller.rb | 4 ++-- app/serializers/fast_versioned_serializer.rb | 4 ++-- config/application.rb | 2 +- db/seeds.rb | 2 +- scheduler.rb | 2 +- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/channels/application_cable/channel.rb b/app/channels/application_cable/channel.rb index 9aec2305..71e17714 100644 --- a/app/channels/application_cable/channel.rb +++ b/app/channels/application_cable/channel.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -module ApplicationCable +module ApplicationCable # leftovers:keep class Channel < ActionCable::Channel::Base end end diff --git a/app/channels/application_cable/connection.rb b/app/channels/application_cable/connection.rb index 8d6c2a1b..a756c637 100644 --- a/app/channels/application_cable/connection.rb +++ b/app/channels/application_cable/connection.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -module ApplicationCable +module ApplicationCable # leftovers:keep class Connection < ActionCable::Connection::Base end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index ae62d647..74224d4b 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -19,7 +19,7 @@ class ApplicationController < ActionController::Base before_action :set_paper_trail_whodunnit # Allow pundit to authorize a non-logged in user - def pundit_user + def pundit_user # leftovers:keep current_user || User.new end diff --git a/app/controllers/impact_omniauth_callbacks_controller.rb b/app/controllers/impact_omniauth_callbacks_controller.rb index b91e62f5..277a720b 100644 --- a/app/controllers/impact_omniauth_callbacks_controller.rb +++ b/app/controllers/impact_omniauth_callbacks_controller.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true -class ImpactOmniauthCallbacksController < DeviseTokenAuth::OmniauthCallbacksController +class ImpactOmniauthCallbacksController < DeviseTokenAuth::OmniauthCallbacksController # leftovers:keep def auth_hash @_auth_hash ||= request.env["omniauth.auth"] ||= session.delete("dta.omniauth.auth") end - def get_resource_from_auth_hash + def get_resource_from_auth_hash # leftovers:keep super if @resource diff --git a/app/serializers/fast_versioned_serializer.rb b/app/serializers/fast_versioned_serializer.rb index 2c328dcc..e6e5591c 100644 --- a/app/serializers/fast_versioned_serializer.rb +++ b/app/serializers/fast_versioned_serializer.rb @@ -4,8 +4,8 @@ module FastVersionedSerializer def self.included(base) base.include FastApplicationSerializer - base.attribute :created_by_id - base.attribute :updated_by_id + base.attribute :created_by_id # leftovers:keep + base.attribute :updated_by_id # leftovers:keep end def current_user_has_permission? diff --git a/config/application.rb b/config/application.rb index 49c8b1a5..a9b0f79c 100644 --- a/config/application.rb +++ b/config/application.rb @@ -8,7 +8,7 @@ # you've limited to :test, :development, or :production. Bundler.require(*Rails.groups) -module HumanRightsNationalReporting +module HumanRightsNationalReporting # leftovers:keep class Application < Rails::Application # Settings in config/environments/* take precedence over those specified here. # Application configuration should go into files in config/initializers diff --git a/db/seeds.rb b/db/seeds.rb index f808ebae..55d6836d 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -1349,7 +1349,7 @@ def base_seeds! ) end - def development_seeds! + def development_seeds! # leftovers:keep nil unless User.count.zero? end diff --git a/scheduler.rb b/scheduler.rb index 627ae6b6..4d177c89 100644 --- a/scheduler.rb +++ b/scheduler.rb @@ -2,7 +2,7 @@ require File.expand_path("../config/environment", __FILE__) require "clockwork" -module Clockwork +module Clockwork # leftovers:keep every(1.day, "Send Due Emails", at: "10:30", tz: Rails.application.config.time_zone) do SendDueEmailsJob.perform_now end From 152c7c11bc01804764f7bd59ccb141d993046608 Mon Sep 17 00:00:00 2001 From: Luke Arndt Date: Tue, 24 Sep 2024 16:45:18 +1200 Subject: [PATCH 04/10] Add leftovers rules that follow general patterns --- .leftovers_todo.yml | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 .leftovers_todo.yml diff --git a/.leftovers_todo.yml b/.leftovers_todo.yml new file mode 100644 index 00000000..8118129e --- /dev/null +++ b/.leftovers_todo.yml @@ -0,0 +1,42 @@ +# for instructions on how to address these +# see https://github.com/robotdana/leftovers/tree/v0.12.2/README.md#how-to-resolve + +keep: + # Policies are used by Pundit to determine what a user can and cannot do. + # Ideally we'd keep track of which policies are actually used, but we + # haven't figured out a way to do that yet. + - path: 'app/policies/*_policy.rb' + privacy: public + type: Method + names: + - create? + - destroy? + - edit? + - index? + - permitted_attributes + - resolve + - show? + - update? + - path: 'app/policies/*_policy.rb' + type: Constant + name: + - has_suffix: Policy + + # Mailer previews are used to display emails in the browser. + # They're not used in production, but they're useful for development. + - path: 'spec/mailers/previews/*_preview.rb' + privacy: public + type: Constant + name: + - has_suffix: Preview + +dynamic: + # Nested associations are usually assigned in our controllers, + # but the process is often too indirect for leftovers to spot. + - path: 'app/models/*.rb' + name: accepts_nested_attributes_for + calls: + argument: '*' + transforms: + - add_suffix: '_attributes' + - add_suffix: '_attributes=' From 093c2093cbcfb5bfb70ece5819b146f262714346 Mon Sep 17 00:00:00 2001 From: Luke Arndt Date: Tue, 24 Sep 2024 16:47:35 +1200 Subject: [PATCH 05/10] Add keep annotations for unused has_many calls --- app/models/category.rb | 2 +- app/models/framework.rb | 2 +- app/models/role.rb | 2 +- app/models/taxonomy.rb | 2 +- app/models/user.rb | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/models/category.rb b/app/models/category.rb index b6739cee..c55609fc 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -8,7 +8,7 @@ class Category < VersionedRecord has_many :measure_categories, inverse_of: :category, dependent: :destroy has_many :sdgtarget_categories, inverse_of: :category, dependent: :destroy has_many :recommendations, through: :recommendation_categories - has_many :users, through: :user_categories + has_many :users, through: :user_categories # leftovers:keep has_many :measures, through: :measure_categories has_many :indicators, through: :recommendations has_many :indicators_via_measures, through: :recommendations diff --git a/app/models/framework.rb b/app/models/framework.rb index 69a2bdcd..1d234ab0 100644 --- a/app/models/framework.rb +++ b/app/models/framework.rb @@ -3,7 +3,7 @@ class Framework < ApplicationRecord has_many :frameworks, through: :framework_frameworks, source: :other_framework has_many :framework_taxonomies, inverse_of: :framework, dependent: :destroy - has_many :taxonomies, through: :framework_taxonomies + has_many :taxonomies, through: :framework_taxonomies # leftovers:keep has_many :recommendations diff --git a/app/models/role.rb b/app/models/role.rb index 75b5bf00..5b3b8267 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -1,4 +1,4 @@ class Role < ApplicationRecord has_many :user_roles - has_many :users, through: :user_roles + has_many :users, through: :user_roles # leftovers:keep end diff --git a/app/models/taxonomy.rb b/app/models/taxonomy.rb index 4c7634dd..6b9db3a4 100644 --- a/app/models/taxonomy.rb +++ b/app/models/taxonomy.rb @@ -1,7 +1,7 @@ class Taxonomy < VersionedRecord has_many :categories belongs_to :taxonomy, class_name: "Taxonomy", foreign_key: :parent_id, optional: true - has_many :taxonomies, class_name: "Taxonomy", foreign_key: :parent_id + has_many :taxonomies, class_name: "Taxonomy", foreign_key: :parent_id # leftovers:keep has_many :framework_taxonomies, inverse_of: :taxonomy, dependent: :destroy has_many :frameworks, through: :framework_taxonomies diff --git a/app/models/user.rb b/app/models/user.rb index 5ea92040..6e8adce7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -11,8 +11,8 @@ class User < VersionedRecord has_many :user_roles, dependent: :destroy has_many :roles, through: :user_roles - has_many :managed_categories, foreign_key: :manager_id, class_name: "Category" - has_many :managed_indicators, foreign_key: :manager_id, class_name: "Indicator" + has_many :managed_categories, foreign_key: :manager_id, class_name: "Category" # leftovers:keep + has_many :managed_indicators, foreign_key: :manager_id, class_name: "Indicator" # leftovers:keep has_many :user_categories has_many :categories, through: :user_categories has_many :bookmarks From 77fda0b07e202b9100ac327a738705cc1bd4f1d5 Mon Sep 17 00:00:00 2001 From: Luke Arndt Date: Tue, 24 Sep 2024 16:48:42 +1200 Subject: [PATCH 06/10] Delete code that I'm confident is not called --- app/models/taxonomy.rb | 4 ---- spec/support/controller_macros.rb | 5 ----- spec/support/feature_spec_helpers.rb | 5 ----- 3 files changed, 14 deletions(-) delete mode 100644 spec/support/feature_spec_helpers.rb diff --git a/app/models/taxonomy.rb b/app/models/taxonomy.rb index 6b9db3a4..7627442c 100644 --- a/app/models/taxonomy.rb +++ b/app/models/taxonomy.rb @@ -24,10 +24,6 @@ def sub_relation end end - def self.current_reporting_cycle - find(current_reporting_cycle_id) - end - def self.current_reporting_cycle_id Rails.application.config.x.reporting_cycle_taxonomy_id end diff --git a/spec/support/controller_macros.rb b/spec/support/controller_macros.rb index 34d7525f..2f0e5d9c 100644 --- a/spec/support/controller_macros.rb +++ b/spec/support/controller_macros.rb @@ -11,9 +11,4 @@ def sign_in(user = double("user")) end user end - - def sign_in_admin - user = sign_in - allow(user).to receive(:role?).with("admin").and_return(true) - end end diff --git a/spec/support/feature_spec_helpers.rb b/spec/support/feature_spec_helpers.rb deleted file mode 100644 index b7cddea6..00000000 --- a/spec/support/feature_spec_helpers.rb +++ /dev/null @@ -1,5 +0,0 @@ -module FeatureSpecHelpers - def flash?(content) - page.has_css?(".callout", text: content) - end -end From b742999e5668ddf79611433d2b095d05dee69159 Mon Sep 17 00:00:00 2001 From: Luke Arndt Date: Tue, 24 Sep 2024 16:49:19 +1200 Subject: [PATCH 07/10] Add keep annotations + comments where I'm unsure --- app/jobs/send_category_due_emails_job.rb | 4 +++- app/serializers/fast_versioned_serializer.rb | 3 ++- scheduler.rb | 2 ++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/jobs/send_category_due_emails_job.rb b/app/jobs/send_category_due_emails_job.rb index c01b1e07..7c3544e0 100644 --- a/app/jobs/send_category_due_emails_job.rb +++ b/app/jobs/send_category_due_emails_job.rb @@ -1,4 +1,6 @@ -class SendCategoryDueEmailsJob < ApplicationJob +# This is currently uncalled because it lacks an entry in Scheduler.rb +# Maybe we can delete it? +class SendCategoryDueEmailsJob < ApplicationJob # leftovers:keep queue_as :default def perform diff --git a/app/serializers/fast_versioned_serializer.rb b/app/serializers/fast_versioned_serializer.rb index e6e5591c..fa7aae0f 100644 --- a/app/serializers/fast_versioned_serializer.rb +++ b/app/serializers/fast_versioned_serializer.rb @@ -8,7 +8,8 @@ def self.included(base) base.attribute :updated_by_id # leftovers:keep end - def current_user_has_permission? + # This appears to be uncalled but I'm not 100% sure. Maybe we can delete it? + def current_user_has_permission? # leftovers:keep current_user&.role?("admin") || current_user&.role?("manager") end end diff --git a/scheduler.rb b/scheduler.rb index 4d177c89..49f68191 100644 --- a/scheduler.rb +++ b/scheduler.rb @@ -11,6 +11,8 @@ module Clockwork # leftovers:keep SendOverdueEmailsJob.perform_now end + # Should this also have a call to send Category Due Emails? + every(1.day, "Send Category Overdue Emails", at: "9:15", tz: Rails.application.config.time_zone) do SendCategoryOverdueEmailsJob.perform_now end From 6c89f01433743b885cef2888136b37a8ee711cfd Mon Sep 17 00:00:00 2001 From: Luke Arndt Date: Tue, 24 Sep 2024 17:18:57 +1200 Subject: [PATCH 08/10] Rename to .leftovers.yml and exclude vendor --- .leftovers_todo.yml => .leftovers.yml | 3 +++ 1 file changed, 3 insertions(+) rename .leftovers_todo.yml => .leftovers.yml (97%) diff --git a/.leftovers_todo.yml b/.leftovers.yml similarity index 97% rename from .leftovers_todo.yml rename to .leftovers.yml index 8118129e..81ba168f 100644 --- a/.leftovers_todo.yml +++ b/.leftovers.yml @@ -1,6 +1,9 @@ # for instructions on how to address these # see https://github.com/robotdana/leftovers/tree/v0.12.2/README.md#how-to-resolve +exclude_paths: + - 'vendor/**/*' + keep: # Policies are used by Pundit to determine what a user can and cannot do. # Ideally we'd keep track of which policies are actually used, but we From a258c19da528b5bbc44a65b8f2a2007e8445cbcd Mon Sep 17 00:00:00 2001 From: Luke Arndt Date: Tue, 24 Sep 2024 17:20:35 +1200 Subject: [PATCH 09/10] Remove leftovers:keep annotations to test CI --- app/controllers/application_controller.rb | 2 +- app/models/user.rb | 2 +- scheduler.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 74224d4b..ae62d647 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -19,7 +19,7 @@ class ApplicationController < ActionController::Base before_action :set_paper_trail_whodunnit # Allow pundit to authorize a non-logged in user - def pundit_user # leftovers:keep + def pundit_user current_user || User.new end diff --git a/app/models/user.rb b/app/models/user.rb index 6e8adce7..dd9b6882 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -12,7 +12,7 @@ class User < VersionedRecord has_many :user_roles, dependent: :destroy has_many :roles, through: :user_roles has_many :managed_categories, foreign_key: :manager_id, class_name: "Category" # leftovers:keep - has_many :managed_indicators, foreign_key: :manager_id, class_name: "Indicator" # leftovers:keep + has_many :managed_indicators, foreign_key: :manager_id, class_name: "Indicator" has_many :user_categories has_many :categories, through: :user_categories has_many :bookmarks diff --git a/scheduler.rb b/scheduler.rb index 49f68191..9b9e9cef 100644 --- a/scheduler.rb +++ b/scheduler.rb @@ -2,7 +2,7 @@ require File.expand_path("../config/environment", __FILE__) require "clockwork" -module Clockwork # leftovers:keep +module Clockwork every(1.day, "Send Due Emails", at: "10:30", tz: Rails.application.config.time_zone) do SendDueEmailsJob.perform_now end From 74ec030b9b7d5e7b8f10f6bbf39128de89a01761 Mon Sep 17 00:00:00 2001 From: Luke Arndt Date: Tue, 24 Sep 2024 17:21:24 +1200 Subject: [PATCH 10/10] Revert "Remove leftovers:keep annotations to test CI" This reverts commit a258c19da528b5bbc44a65b8f2a2007e8445cbcd. --- app/controllers/application_controller.rb | 2 +- app/models/user.rb | 2 +- scheduler.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index ae62d647..74224d4b 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -19,7 +19,7 @@ class ApplicationController < ActionController::Base before_action :set_paper_trail_whodunnit # Allow pundit to authorize a non-logged in user - def pundit_user + def pundit_user # leftovers:keep current_user || User.new end diff --git a/app/models/user.rb b/app/models/user.rb index dd9b6882..6e8adce7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -12,7 +12,7 @@ class User < VersionedRecord has_many :user_roles, dependent: :destroy has_many :roles, through: :user_roles has_many :managed_categories, foreign_key: :manager_id, class_name: "Category" # leftovers:keep - has_many :managed_indicators, foreign_key: :manager_id, class_name: "Indicator" + has_many :managed_indicators, foreign_key: :manager_id, class_name: "Indicator" # leftovers:keep has_many :user_categories has_many :categories, through: :user_categories has_many :bookmarks diff --git a/scheduler.rb b/scheduler.rb index 9b9e9cef..49f68191 100644 --- a/scheduler.rb +++ b/scheduler.rb @@ -2,7 +2,7 @@ require File.expand_path("../config/environment", __FILE__) require "clockwork" -module Clockwork +module Clockwork # leftovers:keep every(1.day, "Send Due Emails", at: "10:30", tz: Rails.application.config.time_zone) do SendDueEmailsJob.perform_now end