Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable Rails related Rubocop rules #9234

Merged
merged 13 commits into from
May 11, 2022
27 changes: 2 additions & 25 deletions .rubocop-disabled.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
Rails/DurationArithmetic:
Enabled: false

Naming/VariableNumber:
Enabled: false

Expand All @@ -16,38 +13,18 @@ Style/OpenStructUse:
Security/IoMethods:
Enabled: false

Rails/CompactBlank:
Enabled: false

Security/Open:
Enabled: false

Rails/EagerEvaluationLogMessage:
Enabled: false

Rails/RedundantPresenceValidationOnBelongsTo:
Enabled: false

Rails/HasManyOrHasOneDependent:
Enabled: false

Gemspec/RequireMFA:
Enabled: false

Rails/DuplicateScope:
Enabled: false

Rails/ExpandedDateRange:
Enabled: false

Rails/I18nLocaleTexts:
Enabled: false
#Rails/I18nLocaleTexts:
# Enabled: false

Naming/MemoizedInstanceVariableName:
Enabled: false

RSpec/VerifiedDoubleReference:
Enabled: false

Rails/TransactionExitStatement:
Enabled: false
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@
name: { "ca" => "Scope2", "en" => "Scope3" }),
component: current_component,
category: create(:category, participatory_space: participatory_space),
created_at: Time.current - 2.days),
created_at: 2.days.ago),
create(:result, scope: create(:scope, organization: component.organization,
name: { "ca" => "Scope3", "en" => "Scope1" }),
component: current_component,
category: create(:category, participatory_space: participatory_space),
created_at: Time.current - 1.day),
created_at: 1.day.ago),
create(:result, scope: create(:scope, organization: component.organization,
name: { "ca" => "Scope1", "en" => "Scope2" }),
component: current_component,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def spaces
if type.ids.include?("all")
object_class.where(organization: @organization)
else
object_class.where(id: type.ids.reject(&:blank?))
object_class.where(id: type.ids.compact_blank)
end
end.flatten.compact
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
let(:resource_controller) { Decidim::Assemblies::Admin::ParticipatorySpacePrivateUsersController }

before do
invited_user_1.update!(invitation_sent_at: Time.current - 1.day, invitation_accepted_at: Time.current)
invited_user_1.update!(invitation_sent_at: 1.day.ago, invitation_accepted_at: Time.current)

switch_to_host(organization.host)
login_as user, scope: :user
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
let!(:invited_user_2) { create(:assembly_valuator, email: email, assembly: assembly) }

before do
invited_user_2.update!(invitation_sent_at: Time.current - 1.day, invitation_accepted_at: Time.current, last_sign_in_at: Time.current)
invited_user_2.update!(invitation_sent_at: 1.day.ago, invitation_accepted_at: Time.current, last_sign_in_at: Time.current)

switch_to_host(organization.host)
login_as admin, scope: :user
Expand Down
2 changes: 1 addition & 1 deletion decidim-blogs/spec/system/explore_posts_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
include_context "with a component"
let(:manifest_name) { "blogs" }

let!(:old_post) { create(:post, component: component, created_at: Time.current - 2.days) }
let!(:old_post) { create(:post, component: component, created_at: 2.days.ago) }
let!(:new_post) { create(:post, component: component, created_at: Time.current) }

let!(:image) { create(:attachment, attached_to: old_post) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@ def initialize(current_order, project, current_user)
# Returns nothing.
def call
transaction do
return broadcast(:invalid) if voting_not_enabled? || order.checked_out? || exceeds_budget?
raise ActiveRecord::RecordInvalid if voting_not_enabled? || order.checked_out? || exceeds_budget?

add_line_item
broadcast(:ok, order)
rescue ActiveRecord::RecordInvalid
return broadcast(:invalid)
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def voting_ends_soon?

end_time = current_component.participatory_space.active_step[:end_date].in_time_zone(time_zone).end_of_day

Time.current + 6.hours >= end_time
6.hours.from_now >= end_time
end

def minimum_interval_between_reminders
Expand Down
1 change: 0 additions & 1 deletion decidim-budgets/app/models/decidim/budgets/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ class Order < Budgets::ApplicationRecord
has_many :projects, through: :line_items, class_name: "Decidim::Budgets::Project", foreign_key: "decidim_project_id"

validates :user, uniqueness: { scope: :budget }
validates :budget, presence: true
validate :user_belongs_to_organization

# Rules active for the budget threshold and minimum budgets rules.
Expand Down
4 changes: 2 additions & 2 deletions decidim-budgets/spec/system/admin_orders_projects_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@
scope: create(:scope, organization: component.organization, name: { "ca" => "Scope2", "en" => "Scope3" }),
budget: budget,
category: create(:category, participatory_space: participatory_space),
created_at: Time.current - 2.days,
created_at: 2.days.ago,
budget_amount: 10_000),
create(:project,
scope: create(:scope, organization: component.organization, name: { "ca" => "Scope3", "en" => "Scope1" }),
budget: budget,
category: create(:category, participatory_space: participatory_space),
created_at: Time.current - 1.day,
created_at: 1.day.ago,
budget_amount: 75_000),
create(:project,
scope: create(:scope, organization: component.organization, name: { "ca" => "Scope1", "en" => "Scope2" }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def initialize(component, day, user = nil)
def query
Decidim::Query.merge(
ConferenceProgramMeetings.new(@component, @user)
).query.where(start_time: @day.beginning_of_day..@day.end_of_day).order(start_time: :asc)
).query.where(start_time: @day.all_day).order(start_time: :asc)
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@
factory :conference_invite, class: "Decidim::Conferences::ConferenceInvite" do
conference
user
sent_at { Time.current - 1.day }
sent_at { 1.day.ago }
accepted_at { nil }
rejected_at { nil }
registration_type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
let!(:invited_user_2) { create(:conference_valuator, email: email, conference: conference) }

before do
invited_user_2.update!(invitation_sent_at: Time.current - 1.day, invitation_accepted_at: Time.current, last_sign_in_at: Time.current)
invited_user_2.update!(invitation_sent_at: 1.day.ago, invitation_accepted_at: Time.current, last_sign_in_at: Time.current)

switch_to_host(organization.host)
login_as admin, scope: :user
Expand Down
2 changes: 1 addition & 1 deletion decidim-core/app/cells/decidim/upload_modal_cell.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def help_messages
def attachments
@attachments = begin
attachments = options[:attachments] || form.object.send(attribute)
attachments = Array(attachments).reject(&:blank?)
attachments = Array(attachments).compact_blank
attachments.map { |attachment| attachment.is_a?(String) ? ActiveStorage::Blob.find_signed(attachment) : attachment }
end
end
Expand Down
2 changes: 1 addition & 1 deletion decidim-core/app/commands/decidim/gallery_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module GalleryMethods

def build_gallery(attached_to = nil)
@gallery = []
@form.add_photos.reject(&:blank?).each do |photo|
@form.add_photos.compact_blank.each do |photo|
if photo.is_a?(Hash) && photo.has_key?(:id)
update_attachment_title_for(photo)
next
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module MultipleAttachmentsMethods

def build_attachments
@documents = []
@form.add_documents.reject(&:blank?).each do |attachment|
@form.add_documents.compact_blank.each do |attachment|
if attachment.is_a?(Hash) && attachment.has_key?(:id)
update_attachment_title_for(attachment)
next
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def unsubscribe
user = User.find_by(decidim_organization_id: current_organization.id, id: decrypted_string.split("-").first)
sent_at_time = Time.zone.at(decrypted_string.split("-").second.to_i)

if sent_at_time > (Time.current - 15.days)
if sent_at_time > (15.days.ago)
UnsubscribeSettings.call(user) do
on(:ok) do
flash.now[:notice] = t("newsletters.unsubscribe.success", scope: "decidim")
Expand Down
2 changes: 1 addition & 1 deletion decidim-core/app/mailers/decidim/application_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def set_smtp
port: @organization.smtp_settings["port"],
user_name: @organization.smtp_settings["user_name"],
password: Decidim::AttributeEncryptor.decrypt(@organization.smtp_settings["encrypted_password"])
) { |_k, o, v| v.presence || o }.reject! { |_k, v| v.blank? }
) { |_k, o, v| v.presence || o }.compact_blank!
end
end
end
2 changes: 1 addition & 1 deletion decidim-core/app/models/decidim/action_log.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class ActionLog < ApplicationRecord
)
}

validates :organization, :user, :action, presence: true
validates :action, presence: true
validates :resource, presence: true, if: ->(log) { log.action != "delete" }
validates :visibility, presence: true, inclusion: { in: %w(private-only admin-only public-only all) }

Expand Down
1 change: 0 additions & 1 deletion decidim-core/app/models/decidim/amendment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ class Amendment < ApplicationRecord
belongs_to :amender, foreign_key: "decidim_user_id", class_name: "Decidim::User"
belongs_to :emendation, foreign_key: "decidim_emendation_id", foreign_type: "decidim_emendation_type", polymorphic: true

validates :amendable, :amender, :emendation, presence: true
validates :state, presence: true, inclusion: { in: STATES }

def draft?
Expand Down
3 changes: 1 addition & 2 deletions decidim-core/app/models/decidim/area.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ class Area < ApplicationRecord
inverse_of: :areas,
optional: true

validates :name, :organization, presence: true
validates :name, uniqueness: { scope: [:organization, :area_type] }
validates :name, presence: true, uniqueness: { scope: [:organization, :area_type] }

before_destroy :abort_if_dependencies

Expand Down
2 changes: 0 additions & 2 deletions decidim-core/app/models/decidim/coauthorship.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ class Coauthorship < ApplicationRecord

belongs_to :coauthorable, polymorphic: true, counter_cache: true

validates :coauthorable, presence: true

after_commit :author_is_follower, on: [:create]

def identity
Expand Down
1 change: 0 additions & 1 deletion decidim-core/app/models/decidim/contextual_help_section.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ class ContextualHelpSection < ApplicationRecord
translatable_fields :content

belongs_to :organization, class_name: "Decidim::Organization"
validates :organization, presence: true
validates :content, presence: true

# Public: Finds content given an id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ class BadgeScore < ApplicationRecord
self.table_name = "decidim_gamification_badge_scores"

belongs_to :user, class_name: "Decidim::UserBaseEntity"
validates :user, presence: true
validates :value, numericality: { greater_than_or_equal_to: 0 }
end
end
Expand Down
3 changes: 1 addition & 2 deletions decidim-core/app/models/decidim/messaging/message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ class Message < ApplicationRecord
foreign_key: :decidim_message_id,
inverse_of: :message

validates :sender, :body, presence: true
validates :body, length: { maximum: ->(_message) { Decidim.config.maximum_conversation_message_length } }
validates :body, presence: true, length: { maximum: ->(_message) { Decidim.config.maximum_conversation_message_length } }

default_scope { order(created_at: :asc) }

Expand Down
2 changes: 0 additions & 2 deletions decidim-core/app/models/decidim/messaging/participation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ class Participation < ApplicationRecord
foreign_key: :decidim_participant_id,
class_name: "Decidim::UserBaseEntity"

validates :conversation, :participant, presence: true

validates :decidim_conversation_id, uniqueness: { scope: :decidim_participant_id }
end
end
Expand Down
2 changes: 0 additions & 2 deletions decidim-core/app/models/decidim/messaging/receipt.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ class Receipt < ApplicationRecord
belongs_to :recipient, foreign_key: "decidim_recipient_id", class_name: "Decidim::User"
belongs_to :message, foreign_key: "decidim_message_id", class_name: "Decidim::Messaging::Message"

validates :recipient, :message, presence: true

scope :recipient, ->(recipient) { where(recipient: recipient) }
scope :unread_by, ->(user) { recipient(user).unread }
scope :unread, -> { where(read_at: nil) }
Expand Down
12 changes: 6 additions & 6 deletions decidim-core/app/models/decidim/organization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ class Organization < ApplicationRecord

has_many :static_pages, foreign_key: "decidim_organization_id", class_name: "Decidim::StaticPage", inverse_of: :organization, dependent: :destroy
has_many :static_page_topics, class_name: "Decidim::StaticPageTopic", inverse_of: :organization, dependent: :destroy
has_many :scopes, -> { order(name: :asc) }, foreign_key: "decidim_organization_id", class_name: "Decidim::Scope", inverse_of: :organization
has_many :scope_types, -> { order(name: :asc) }, foreign_key: "decidim_organization_id", class_name: "Decidim::ScopeType", inverse_of: :organization
has_many :areas, -> { order(name: :asc) }, foreign_key: "decidim_organization_id", class_name: "Decidim::Area", inverse_of: :organization
has_many :area_types, -> { order(name: :asc) }, foreign_key: "decidim_organization_id", class_name: "Decidim::AreaType", inverse_of: :organization
has_many :admins, -> { where(admin: true) }, foreign_key: "decidim_organization_id", class_name: "Decidim::User"
has_many :users_with_any_role, -> { where.not(roles: []) }, foreign_key: "decidim_organization_id", class_name: "Decidim::User"
has_many :scopes, -> { order(name: :asc) }, foreign_key: "decidim_organization_id", class_name: "Decidim::Scope", inverse_of: :organization, dependent: :destroy
alecslupu marked this conversation as resolved.
Show resolved Hide resolved
has_many :scope_types, -> { order(name: :asc) }, foreign_key: "decidim_organization_id", class_name: "Decidim::ScopeType", inverse_of: :organization, dependent: :destroy
has_many :areas, -> { order(name: :asc) }, foreign_key: "decidim_organization_id", class_name: "Decidim::Area", inverse_of: :organization, dependent: :destroy
has_many :area_types, -> { order(name: :asc) }, foreign_key: "decidim_organization_id", class_name: "Decidim::AreaType", inverse_of: :organization, dependent: :destroy
has_many :admins, -> { where(admin: true) }, foreign_key: "decidim_organization_id", class_name: "Decidim::User", dependent: :destroy
has_many :users_with_any_role, -> { where.not(roles: []) }, foreign_key: "decidim_organization_id", class_name: "Decidim::User", dependent: :destroy
has_many :users, foreign_key: "decidim_organization_id", class_name: "Decidim::User", dependent: :destroy
has_many :user_entities, foreign_key: "decidim_organization_id", class_name: "Decidim::UserBaseEntity", dependent: :destroy
has_many :oauth_applications, foreign_key: "decidim_organization_id", class_name: "Decidim::OAuthApplication", inverse_of: :organization, dependent: :destroy
Expand Down
2 changes: 1 addition & 1 deletion decidim-core/app/models/decidim/scope.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class Scope < ApplicationRecord

before_validation :update_part_of, on: :update

validates :name, :code, :organization, presence: true
validates :name, :code, presence: true
validates :code, uniqueness: { scope: :organization }
validate :forbid_cycles

Expand Down
2 changes: 0 additions & 2 deletions decidim-core/app/models/decidim/share_token.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

module Decidim
class ShareToken < ApplicationRecord
validates :organization, presence: true
validates :user, presence: true
validates :token, presence: true, uniqueness: { scope: [:decidim_organization_id, :token_for_type, :token_for_id] }

belongs_to :organization, foreign_key: "decidim_organization_id", class_name: "Decidim::Organization"
Expand Down
8 changes: 4 additions & 4 deletions decidim-core/db/migrate/20190412131728_fix_user_names.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ def change

weird_characters.each do |character|
Decidim::UserBaseEntity.where(deleted_at: nil).where("name like '%#{character}%' escape '\' OR nickname like '%#{character}%' escape '\'").find_each do |entity|
Rails.logger.debug "detected character: #{character}"
Rails.logger.debug "UserBaseEntity ID: #{entity.id}"
Rails.logger.debug "#{entity.name} => #{entity.name.delete(characters_to_remove).strip}"
Rails.logger.debug "#{entity.nickname} => #{entity.nickname.delete(characters_to_remove).strip}"
Rails.logger.debug { "detected character: #{character}" }
Rails.logger.debug { "UserBaseEntity ID: #{entity.id}" }
Rails.logger.debug { "#{entity.name} => #{entity.name.delete(characters_to_remove).strip}" }
Rails.logger.debug { "#{entity.nickname} => #{entity.nickname.delete(characters_to_remove).strip}" }

entity.name = entity.name.delete(characters_to_remove).strip
sanitized_nickname = entity.nickname.delete(characters_to_remove).strip
Expand Down
3 changes: 1 addition & 2 deletions decidim-core/lib/decidim/authorable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ module Authorable
}

scope :with_any_origin, lambda { |*origin_keys|
search_values = origin_keys.compact.reject(&:blank?)
search_values = origin_keys.compact.compact_blank

conditions = [:official, :participants, :user_group].map do |key|
search_values.member?(key.to_s) ? try("with_#{key}_origin") : nil
Expand All @@ -42,7 +42,6 @@ module Authorable
scoped_query
}

validates :author, presence: true
validate :verified_user_group, :user_group_membership
validate :author_belongs_to_organization

Expand Down
2 changes: 1 addition & 1 deletion decidim-core/lib/decidim/coauthorable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ module Coauthorable
}

scope :with_any_origin, lambda { |*origin_keys|
search_values = origin_keys.compact.reject(&:blank?)
search_values = origin_keys.compact.compact_blank

conditions = [:official, :participants, :user_group, :meeting].map do |key|
search_values.member?(key.to_s) ? try("with_#{key}_origin") : nil
Expand Down
2 changes: 1 addition & 1 deletion decidim-core/lib/decidim/filterable_resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def ransacker_text_multi(field_name, attrs)

def scope_search_multi(scope_key, possible_scopes)
scope scope_key, lambda { |*value_keys|
search_values = value_keys.compact.reject(&:blank?)
search_values = value_keys.compact.compact_blank

conditions = possible_scopes.map do |scope|
search_values.member?(scope.to_s) ? try(scope) : nil
Expand Down