Skip to content

Commit

Permalink
Merge pull request #6093 from chrisnicollo/cc/guard_clause
Browse files Browse the repository at this point in the history
Fix Rubocop Style/GuardClause Violations
  • Loading branch information
ragesoss authored Jan 8, 2025
2 parents 531960e + 11a64bd commit d003c31
Show file tree
Hide file tree
Showing 13 changed files with 50 additions and 73 deletions.
2 changes: 0 additions & 2 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,6 @@ Rails/FilePath:
Enabled: false
Performance/UnfreezeString:
Enabled: false
Style/GuardClause:
Enabled: false
Rails/HasManyOrHasOneDependent:
Enabled: false
Rails/InverseOf:
Expand Down
13 changes: 6 additions & 7 deletions app/controllers/courses_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -408,13 +408,12 @@ def update_course_format
def update_last_reviewed
username = params.dig(:course, 'last_reviewed', 'username')
timestamp = params.dig(:course, 'last_reviewed', 'timestamp')
if username && timestamp
@course.flags['last_reviewed'] = {
'username' => username,
'timestamp' => timestamp
}
@course.save
end
return unless username && timestamp
@course.flags['last_reviewed'] = {
'username' => username,
'timestamp' => timestamp
}
@course.save
end

def handle_post_course_creation_updates
Expand Down
9 changes: 4 additions & 5 deletions app/controllers/requested_accounts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,9 @@ def passcode_valid?

def handle_existing_request
existing_request = RequestedAccount.find_by(course: @course, username: params[:username])
if existing_request
existing_request.update(email: params[:email])
render json: { message: existing_request.updated_email_message }
yield
end
return unless existing_request
existing_request.update(email: params[:email])
render json: { message: existing_request.updated_email_message }
yield
end
end
23 changes: 7 additions & 16 deletions app/helpers/article_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,25 +35,16 @@ def rating_priority(rating)
def rating_display(rating)
rating = default_class(rating)
return nil if rating.nil?
if %w[fa ga fl].include? rating
return rating
else
return rating[0] # use the first letter of the rating as the abbreviated version
end
return rating if %w[fa ga fl].include? rating
return rating[0] # use the first letter of the rating as the abbreviated version
end

def default_class(rating)
# Handles the different article classes and returns a known article class
if %w[fa fl a ga b c start stub list].include? rating
return rating
elsif rating.eql? 'bplus'
return 'b'
elsif rating.eql? 'a/ga'
return 'a'
elsif %w[al bl cl sl].include? rating
return 'list'
else
return nil
end
return rating if %w[fa fl a ga b c start stub list].include? rating
return 'b' if rating.eql? 'bplus'
return 'a' if rating.eql? 'a/ga'
return 'list' if %w[al bl cl sl].include? rating
return nil
end
end
7 changes: 3 additions & 4 deletions app/models/campaign.rb
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,9 @@ def valid_start_and_end_dates?
def set_slug
campaign_slug = slug.presence || title
self.slug = campaign_slug.downcase
unless /^[\p{L}0-9_]+$/.match?(campaign_slug.downcase)
# Strip everything but unicode letters and digits, and convert spaces to underscores.
self.slug = campaign_slug.downcase.gsub(/[^\p{L}0-9 ]/, '').tr(' ', '_')
end
return if /^[\p{L}0-9_]+$/.match?(campaign_slug.downcase)
# Strip everything but unicode letters and digits, and convert spaces to underscores.
self.slug = campaign_slug.downcase.gsub(/[^\p{L}0-9 ]/, '').tr(' ', '_')
end

def set_default_times
Expand Down
9 changes: 2 additions & 7 deletions app/services/add_sandbox_template.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,8 @@ def initialize(home_wiki:, sandbox:, sandbox_template:, current_user:)

def add_template
# Never double-post the sandbox template
if sandbox_template_present?
return
elsif default_template_present?
replace_default_with_sandbox_template
else
add_sandbox_template
end
return if sandbox_template_present?
default_template_present? ? replace_default_with_sandbox_template : add_sandbox_template
end

def sandbox_template_present?
Expand Down
9 changes: 6 additions & 3 deletions app/services/sandbox_url_updator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,11 @@ def validate_new_url
raise InvalidUrlError, I18n.t('assignments.invalid_url', url: @new_url) unless new_url_match
# Handle mismatched wiki
new_language, new_project = new_url_match.captures
unless existing_language == new_language && existing_project == new_project
raise MismatchedWikiError, I18n.t('assignments.mismatched_wiki', url: @new_url)
end
wiki_matches = (existing_language == new_language && existing_project == new_project)
handle_mismatched_wiki unless wiki_matches
end

def handle_mismatched_wiki
raise MismatchedWikiError, I18n.t('assignments.mismatched_wiki', url: @new_url)
end
end
8 changes: 5 additions & 3 deletions lib/assignment_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,12 @@ def set_article_from_database

def check_wiki_edu_discouraged_article
category = Category.find_by(name: ENV['blocked_assignment_category'])
article_discouraged = (category.present? && category.article_titles.include?(@clean_title))
handle_discouraged_article if article_discouraged
end

if category.present? && category.article_titles.include?(@clean_title)
raise DiscouragedArticleError, I18n.t('assignments.blocked_assignment', title: @clean_title)
end
def handle_discouraged_article
raise DiscouragedArticleError, I18n.t('assignments.blocked_assignment', title: @clean_title)
end

def import_article_from_wiki
Expand Down
11 changes: 5 additions & 6 deletions lib/list_course_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,11 @@ def add_instructor_real_names

def add_classroom_program_manager_if_exists
cpm = SpecialUsers.classroom_program_manager
if cpm && @course.type == 'ClassroomProgramCourse'
CoursesUsers.create(user: cpm,
course: @course,
role: CoursesUsers::Roles::WIKI_ED_STAFF_ROLE,
real_name: cpm.real_name)
end
return unless cpm && @course.type == 'ClassroomProgramCourse'
CoursesUsers.create(user: cpm,
course: @course,
role: CoursesUsers::Roles::WIKI_ED_STAFF_ROLE,
real_name: cpm.real_name)
end

def send_approval_notification_emails
Expand Down
15 changes: 6 additions & 9 deletions lib/reference_counter_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,12 @@ def get_number_of_references_from_revision_id(rev_id)
tries ||= 5
response = toolforge_server.get(references_query_url(rev_id))
parsed_response = Oj.load(response.body)
if response.status == 200
return { 'num_ref' => parsed_response['num_ref'] }
else
# Log the error and return empty hash
# Sentry.capture_message 'Non-200 response hitting references counter API', level: 'warning',
# extra: { project_code: @project_code, language_code: @language_code, rev_id:,
# status_code: response.status, content: parsed_response }
return { 'num_ref' => nil }
end
return { 'num_ref' => parsed_response['num_ref'] } if response.status == 200
# Log the error and return empty hash
# Sentry.capture_message 'Non-200 response hitting references counter API', level: 'warning',
# extra: { project_code: @project_code, language_code: @language_code, rev_id:,
# status_code: response.status, content: parsed_response }
return { 'num_ref' => nil }
rescue StandardError => e
tries -= 1
retry unless tries.zero?
Expand Down
5 changes: 2 additions & 3 deletions lib/revision_feedback_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@ def feedback
def citation_feedback
ref_tags = @features['feature.wikitext.revision.ref_tags']
# cite_templates = @features['feature.enwiki.revision.cite_templates']
if ref_tags < MINIMUM_REFERENCES
@feedback << 'Cite your sources! This article needs more references.'
end
return unless ref_tags < MINIMUM_REFERENCES
@feedback << 'Cite your sources! This article needs more references.'
end

# The largest reasonable average section size, calculated from content characters
Expand Down
5 changes: 2 additions & 3 deletions lib/revision_score_api_handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@ def initialize(language: 'en', project: 'wikipedia', wiki: nil, update_service:
# Initialize LiftWingApi if the wiki is valid for it
@lift_wing_api = LiftWingApi.new(@wiki, @update_service) if LiftWingApi.valid_wiki?(@wiki)
# Initialize ReferenceCounterApi if the wiki is valid for it
if ReferenceCounterApi.valid_wiki?(@wiki)
@reference_counter_api = ReferenceCounterApi.new(@wiki, @update_service)
end
return unless ReferenceCounterApi.valid_wiki?(@wiki)
@reference_counter_api = ReferenceCounterApi.new(@wiki, @update_service)
end

# Returns data from LiftWing API and/or reference-counter API.
Expand Down
7 changes: 2 additions & 5 deletions lib/training_module_due_date_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,8 @@ def module_progress

def flags(course_id)
flags_hash = @tmu&.flags
if @training_module.exercise? && flags_hash.present?
return flags_hash[course_id] || flags_hash
else
return flags_hash
end
return flags_hash[course_id] || flags_hash if @training_module.exercise? && flags_hash.present?
return flags_hash
end

def sandbox_url
Expand Down

0 comments on commit d003c31

Please sign in to comment.