From 88d33f361fcd5de267caa23bd4ad40b5b14dbd45 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 2 May 2023 06:57:11 -0400 Subject: [PATCH] Fix Lint/DuplicateBranch cop (#24766) --- .rubocop_todo.yml | 9 ------ app/lib/permalink_redirector.rb | 46 ++++++++++++++++++++++----- app/models/account_statuses_filter.rb | 6 ++-- app/validators/email_mx_validator.rb | 4 +-- app/validators/vote_validator.rb | 18 ++++++++--- lib/mastodon/maintenance_cli.rb | 4 +-- 6 files changed, 56 insertions(+), 31 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 446e38b0a733246f405438e27ccc4010c962c735..a3b80f141bfcb6d015a9f968b16d106d72c69751 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -119,15 +119,6 @@ Lint/ConstantDefinitionInBlock: - 'spec/lib/connection_pool/shared_timed_stack_spec.rb' - 'spec/models/concerns/remotable_spec.rb' -# Configuration parameters: IgnoreLiteralBranches, IgnoreConstantBranches. -Lint/DuplicateBranch: - Exclude: - - 'app/lib/permalink_redirector.rb' - - 'app/models/account_statuses_filter.rb' - - 'app/validators/email_mx_validator.rb' - - 'app/validators/vote_validator.rb' - - 'lib/mastodon/maintenance_cli.rb' - # Configuration parameters: AllowComments, AllowEmptyLambdas. Lint/EmptyBlock: Exclude: diff --git a/app/lib/permalink_redirector.rb b/app/lib/permalink_redirector.rb index cf1a37625105e3abfe0347b0fb910b4f1533fa21..063a2188b5f2af594646dff17f2f145ee6283e5f 100644 --- a/app/lib/permalink_redirector.rb +++ b/app/lib/permalink_redirector.rb @@ -8,19 +8,49 @@ class PermalinkRedirector end def redirect_path - if path_segments[0].present? && path_segments[0].start_with?('@') && path_segments[1] =~ /\d/ - find_status_url_by_id(path_segments[1]) - elsif path_segments[0].present? && path_segments[0].start_with?('@') - find_account_url_by_name(path_segments[0]) - elsif path_segments[0] == 'statuses' && path_segments[1] =~ /\d/ - find_status_url_by_id(path_segments[1]) - elsif path_segments[0] == 'accounts' && path_segments[1] =~ /\d/ - find_account_url_by_id(path_segments[1]) + if at_username_status_request? || statuses_status_request? + find_status_url_by_id(second_segment) + elsif at_username_request? + find_account_url_by_name(first_segment) + elsif accounts_request? && record_integer_id_request? + find_account_url_by_id(second_segment) end end private + def at_username_status_request? + at_username_request? && record_integer_id_request? + end + + def statuses_status_request? + statuses_request? && record_integer_id_request? + end + + def at_username_request? + first_segment.present? && first_segment.start_with?('@') + end + + def statuses_request? + first_segment == 'statuses' + end + + def accounts_request? + first_segment == 'accounts' + end + + def record_integer_id_request? + second_segment =~ /\d/ + end + + def first_segment + path_segments.first + end + + def second_segment + path_segments.second + end + def path_segments @path_segments ||= @path.gsub(/\A\//, '').split('/') end diff --git a/app/models/account_statuses_filter.rb b/app/models/account_statuses_filter.rb index 211f414787fe22d5c57917285916b3b7d2c330a6..c6dc1385f50482cb62d6ff2237b7405d49a4e781 100644 --- a/app/models/account_statuses_filter.rb +++ b/app/models/account_statuses_filter.rb @@ -32,9 +32,9 @@ class AccountStatusesFilter private def initial_scope - if suspended? - Status.none - elsif anonymous? + return Status.none if suspended? + + if anonymous? account.statuses.where(visibility: %i(public unlisted)) elsif author? account.statuses.all # NOTE: #merge! does not work without the #all diff --git a/app/validators/email_mx_validator.rb b/app/validators/email_mx_validator.rb index 19c57bdf66561c174c85462d1c681baa8b14f07a..a30a0c820d084ac8fd0fd178aa42f6f9abdef4e2 100644 --- a/app/validators/email_mx_validator.rb +++ b/app/validators/email_mx_validator.rb @@ -8,9 +8,7 @@ class EmailMxValidator < ActiveModel::Validator domain = get_domain(user.email) - if domain.blank? - user.errors.add(:email, :invalid) - elsif domain.include?('..') + if domain.blank? || domain.include?('..') user.errors.add(:email, :invalid) elsif !on_allowlist?(domain) resolved_ips, resolved_domains = resolve_mx(domain) diff --git a/app/validators/vote_validator.rb b/app/validators/vote_validator.rb index 9c55f9ab6daa4f6b61935619de89d27a0f8f09f7..9bd17fbe80828ed890003c9e97038d73382bfcad 100644 --- a/app/validators/vote_validator.rb +++ b/app/validators/vote_validator.rb @@ -6,15 +6,23 @@ class VoteValidator < ActiveModel::Validator vote.errors.add(:base, I18n.t('polls.errors.invalid_choice')) if invalid_choice?(vote) - if vote.poll_multiple? && already_voted_for_same_choice_on_multiple_poll?(vote) - vote.errors.add(:base, I18n.t('polls.errors.already_voted')) - elsif !vote.poll_multiple? && already_voted_on_non_multiple_poll?(vote) - vote.errors.add(:base, I18n.t('polls.errors.already_voted')) - end + vote.errors.add(:base, I18n.t('polls.errors.already_voted')) if additional_voting_not_allowed?(vote) end private + def additional_voting_not_allowed?(vote) + poll_multiple_and_already_voted?(vote) || poll_non_multiple_and_already_voted?(vote) + end + + def poll_multiple_and_already_voted?(vote) + vote.poll_multiple? && already_voted_for_same_choice_on_multiple_poll?(vote) + end + + def poll_non_multiple_and_already_voted?(vote) + !vote.poll_multiple? && already_voted_on_non_multiple_poll?(vote) + end + def invalid_choice?(vote) vote.choice.negative? || vote.choice >= vote.poll.options.size end diff --git a/lib/mastodon/maintenance_cli.rb b/lib/mastodon/maintenance_cli.rb index ff8f6ddda947c8f2f2b2058916f666a160725cce..88bd191ea940c48aab34d7e43bf0fe743b1b8d34 100644 --- a/lib/mastodon/maintenance_cli.rb +++ b/lib/mastodon/maintenance_cli.rb @@ -664,9 +664,7 @@ module Mastodon def remove_index_if_exists!(table, name) ActiveRecord::Base.connection.remove_index(table, name: name) - rescue ArgumentError - nil - rescue ActiveRecord::StatementInvalid + rescue ArgumentError, ActiveRecord::StatementInvalid nil end end