Skip to content

Commit

Permalink
Fix lint
Browse files Browse the repository at this point in the history
  • Loading branch information
JoonasAapro committed Oct 3, 2024
1 parent 94e8880 commit 65626ec
Show file tree
Hide file tree
Showing 20 changed files with 108 additions and 96 deletions.
6 changes: 6 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ group :development, :test do
gem "byebug", "~> 11.0", platform: :mri
gem "decidim-dev", DECIDIM_VERSION
gem "decidim-sms-telia", github: "mainio/decidim-sms-telia", branch: "main"

# rubocop & rubocop-rspec are set to the following versions because of a change where FactoryBot/CreateList
# must be a boolean instead of contextual. These version locks can be removed when this problem is handled
# through decidim-dev.
gem "rubocop", "~>1.28"
gem "rubocop-rspec", "2.20"
end

group :development do
Expand Down
12 changes: 4 additions & 8 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -666,8 +666,6 @@ GEM
parser (>= 3.3.1.0)
rubocop-capybara (2.21.0)
rubocop (~> 1.41)
rubocop-factory_bot (2.26.0)
rubocop (~> 1.41)
rubocop-faker (1.1.0)
faker (>= 2.12.0)
rubocop (>= 0.82.0)
Expand All @@ -676,13 +674,9 @@ GEM
rack (>= 1.1)
rubocop (>= 1.33.0, < 2.0)
rubocop-ast (>= 1.31.1, < 2.0)
rubocop-rspec (2.31.0)
rubocop (~> 1.40)
rubocop-rspec (2.20.0)
rubocop (~> 1.33)
rubocop-capybara (~> 2.17)
rubocop-factory_bot (~> 2.22)
rubocop-rspec_rails (~> 2.28)
rubocop-rspec_rails (2.29.0)
rubocop (~> 1.40)
ruby-progressbar (1.13.0)
ruby-vips (2.2.2)
ffi (~> 1.12)
Expand Down Expand Up @@ -799,7 +793,9 @@ DEPENDENCIES
letter_opener_web (~> 2.0)
listen (~> 3.1)
puma (>= 5.6.2)
rubocop (~> 1.28)
rubocop-faker
rubocop-rspec (= 2.20)
spring (~> 2.0)
spring-watcher-listen (~> 2.0)
web-console (~> 4.2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def gateway
code = generate_code
gateway = Decidim.config.sms_gateway_service.constantize
if gateway.instance_method(:initialize).parameters.length > 2
gateway.new(phone_number, code, organization: organization)
gateway.new(phone_number, code, organization:)
else
gateway.new(phone_number, code)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def call
invalid!
end
rescue StandardError => e
puts e
Rails.logger.debug e
invalid!(e.message)
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def prefix
def access_code_instance(code)
digest = "#{code}-#{Rails.application.secrets.secret_key_base}"
code_hash = Digest::MD5.hexdigest(digest)
::Decidim::HelsinkiSmsauth::SigninCode.find_by(code_hash: code_hash)
::Decidim::HelsinkiSmsauth::SigninCode.find_by(code_hash:)
end

def destroy_access_code!
Expand Down
10 changes: 5 additions & 5 deletions app/controllers/decidim/helsinki_smsauth/omniauth_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class OmniauthController < ::Decidim::Devise::OmniauthRegistrationsController

helper Decidim::HelsinkiSmsauth::Engine.routes.url_helpers
helper Decidim::HelsinkiSmsauth::RegistrationHelper
before_action :ensure_authorized, only: [:new, :registration, :user_registry]
before_action :ensure_authorized, only: [:new, :user_registry]

def new
@form = form(OmniauthForm).instance
Expand Down Expand Up @@ -101,7 +101,7 @@ def school_info

def user_registry
user = find_user!
@form = SchoolMetadataForm.from_params(user_params.merge(current_locale: current_locale, organization: current_organization, user: user))
@form = SchoolMetadataForm.from_params(user_params.merge(current_locale:, organization: current_organization, user:))

RegisterByPhone.call(@form) do
on(:ok) do |new_user|
Expand Down Expand Up @@ -157,7 +157,7 @@ def access_code
end

def access_code_validation
@form = AccessCodeForm.from_params(params.merge(current_locale: current_locale, organization: current_organization))
@form = AccessCodeForm.from_params(params.merge(current_locale:, organization: current_organization))
VerifyAccessCode.call(@form) do
on(:ok) do |user, access_hash|
update_sessions!(**access_hash)
Expand Down Expand Up @@ -202,7 +202,7 @@ def authorize_user!(user)

def generated_metadata(authorization, phone_number)
metadata = authorization.metadata || {}
metadata.merge!({ phone_number: phone_number })
metadata.merge!({ phone_number: })
metadata.merge!({ school: auth_session["school"] }) if metadata["school"].nil?
metadata.merge!({ grade: auth_session["grade"] }) if metadata["grade"].nil?
metadata
Expand Down Expand Up @@ -316,7 +316,7 @@ def update_authorization!(user, auth_session)

def find_authorization(user)
Decidim::Authorization.find_or_initialize_by(
user: user,
user:,
name: "helsinki_smsauth_id"
)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def expiration_time
end

def set_expiration_info
t("expiration_note", scope: "decidim.helsinki_smsauth.verifications.admin.signin_codes.view_generated_codes", expiration_time: expiration_time)
t("expiration_note", scope: "decidim.helsinki_smsauth.verifications.admin.signin_codes.view_generated_codes", expiration_time:)
end

def global_expiration_period
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,23 @@ def new
# We use the :update action here because this is also where the user
# is redirected to in case they previously started the authorization
# but did not finish it (i.e. the authorization is "pending").
enforce_permission_to :update, :authorization, authorization: authorization
enforce_permission_to(:update, :authorization, authorization:)

@form = form(AuthorizationForm).instance
end

def edit
enforce_permission_to(:update, :authorization, authorization:)

@form = ConfirmationForm.new
verification_code
end

def create
# We use the :update action here because this is also where the user
# is redirected to in case they previously started the authorization
# but did not finish it (i.e. the authorization is "pending").
enforce_permission_to :update, :authorization, authorization: authorization
enforce_permission_to(:update, :authorization, authorization:)

@form = AuthorizationForm.from_params(params.merge(user: current_user, school: nil, grade: nil, organization: current_organization))
Decidim::Verifications::PerformAuthorizationStep.call(authorization, @form) do
Expand All @@ -38,13 +45,6 @@ def create
end
end

def edit
enforce_permission_to :update, :authorization, authorization: authorization

@form = ConfirmationForm.new
verification_code
end

def resend_code
return unless eligible_to?

Expand All @@ -54,7 +54,7 @@ def resend_code
on(:ok) do
flash_message_for_resend(last_request_time)
authorization_method = Decidim::Verifications::Adapter.from_element(authorization.name)
redirect_to authorization_method.resume_authorization_path(redirect_url: redirect_url)
redirect_to authorization_method.resume_authorization_path(redirect_url:)
end
on(:invalid) do
flash.now[:alert] = I18n.t(".error", scope: "decidim.helsinki_smsauth.omniauth.sms.authenticate_user")
Expand All @@ -64,13 +64,13 @@ def resend_code
end

def school_info
enforce_permission_to :update, :authorization, authorization: authorization
enforce_permission_to(:update, :authorization, authorization:)

@form = form(::Decidim::HelsinkiSmsauth::SchoolMetadataForm).instance
end

def school_validation
enforce_permission_to :update, :authorization, authorization: authorization
enforce_permission_to(:update, :authorization, authorization:)

@form = form(::Decidim::HelsinkiSmsauth::SchoolMetadataForm).from_params(params)
ValidateSchoolInfo.call(@form, authorization) do
Expand All @@ -81,7 +81,7 @@ def school_validation
end

def update
enforce_permission_to :update, :authorization, authorization: authorization
enforce_permission_to(:update, :authorization, authorization:)

@form = ConfirmationForm.from_params(params)
ConfirmUserPhoneAuthorization.call(authorization, @form, session) do
Expand All @@ -100,7 +100,7 @@ def update
end

def destroy
enforce_permission_to :destroy, :authorization, authorization: authorization
enforce_permission_to(:destroy, :authorization, authorization:)

DestroyAuthorization.call(authorization) do
on(:ok) do
Expand All @@ -111,13 +111,13 @@ def destroy
end

def access_code
enforce_permission_to :update, :authorization, authorization: authorization
enforce_permission_to(:update, :authorization, authorization:)

@form = form(::Decidim::HelsinkiSmsauth::AccessCodeForm).instance
end

def access_code_validation
@form = form(::Decidim::HelsinkiSmsauth::AccessCodeForm).from_params(params.merge({ current_locale: current_locale, organization: current_organization }))
@form = form(::Decidim::HelsinkiSmsauth::AccessCodeForm).from_params(params.merge({ current_locale:, organization: current_organization }))
ValidateAccessCode.call(@form, authorization, current_user) do
on(:ok) do
handle_redirect
Expand Down Expand Up @@ -167,7 +167,7 @@ def ensure_sending_limit

def redirect_smsauth
authorization_method = Decidim::Verifications::Adapter.from_element(authorization.name)
authorization_method.resume_authorization_path(redirect_url: redirect_url)
authorization_method.resume_authorization_path(redirect_url:)
end

def handle_redirect
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,16 @@ def phone_number
# The verification metadata to validate in the next step.
def verification_metadata
{
verification_code: verification_code,
verification_code:,
code_sent_at: Time.current
}
end

def metadata
{
phone_number: phone_with_country_code,
school: school,
grade: grade
school:,
grade:
}
end

Expand All @@ -58,7 +58,7 @@ def gateway
mobile_number = phone_with_country_code
gateway = Decidim.config.sms_gateway_service.constantize
if gateway.instance_method(:initialize).parameters.length > 2
gateway.new(mobile_number, generated_code, organization: organization)
gateway.new(mobile_number, generated_code, organization:)
else
gateway.new(mobile_number, generated_code)
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/decidim/helsinki_smsauth/signin_code.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def generate!
code = generated_code
digest = "#{code}-#{Rails.application.secrets.secret_key_base}"
self.code_hash = Digest::MD5.hexdigest(digest)
return code if ::Decidim::HelsinkiSmsauth::SigninCode.find_by(code_hash: code_hash).blank?
return code if ::Decidim::HelsinkiSmsauth::SigninCode.find_by(code_hash:).blank?
end
end

Expand Down
4 changes: 2 additions & 2 deletions lib/decidim/helsinki_smsauth/test/factories.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def code_count(total, used)
end
FactoryBot.define do
factory :signin_code_set, class: "Decidim::HelsinkiSmsauth::SigninCodeSet" do
creator { create(:user, :confirmed, :admin, organization: organization) }
creator { create(:user, :confirmed, :admin, organization:) }
generated_code_amount { 1 }
used_code_amount { 0 }
metadata do
Expand All @@ -22,7 +22,7 @@ def code_count(total, used)
create_list(
:signin_code,
code_count(evaluator.generated_code_amount, evaluator.used_code_amount),
signin_code_set: signin_code_set
signin_code_set:
)
end

Expand Down
20 changes: 12 additions & 8 deletions spec/shared/authenticate_with_phone_process_examples.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,33 @@
expect(page).to have_content("Problems?")
expect(page).to have_link("Log in with a code given by your teacher", href: "/users/auth/sms/access_code")
fill_in "Phone number", with: "45887874"
click_button "Send code"
click_on "Send code"
within_flash_messages do
expect(page).to have_content(/The code has been sent to/)
end
expect(page).to have_content("Log in via text message")
click_link("Resend the code")
click_on "Resend the code"
within_flash_messages do
expect(page).to have_content("Please wait at least 1 minute to resend the code.")
end
allow(Time).to receive(:current).and_return(2.minutes.from_now)
click_link("Resend the code")
click_on "Resend the code"
expect(page).to have_content(/The code has been sent to/)
fill_in "Login code", with: "000000"
click_button "Log in"
within ".new_sms_verification" do
click_on "Log in"
end
within_flash_messages do
expect(page).to have_content("Failed to verify the phone number. Please try again and check that you have entered the login code correctly.")
end
code = page.find("#hint").text
code = page.find_by_id("hint").text
fill_in "Login code", with: code
click_button "Log in"
expect(page).not_to have_current_path decidim_helsinki_smsauth.users_auth_sms_edit_path
within ".new_sms_verification" do
click_on "Log in"
end
expect(page).to have_no_current_path decidim_helsinki_smsauth.users_auth_sms_edit_path
within_flash_messages do
expect(page).not_to have_content("Failed to verify the phone number. Please try again and check that you have entered the login code correctly.")
expect(page).to have_no_content("Failed to verify the phone number. Please try again and check that you have entered the login code correctly.")
end
end
end
Expand Down
10 changes: 5 additions & 5 deletions spec/shared/filterable_login_code_examples.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

shared_examples "filterable login code" do
let!(:creators) { create_list(:user, 2, :confirmed, :admin, organization: organization) }
let!(:creators) { create_list(:user, 2, :confirmed, :admin, organization:) }
let!(:unused_codes) do
10.times do
create_list(:signin_code_set, 1, generated_code_amount: code_amount, creator: creators[0])
Expand Down Expand Up @@ -45,9 +45,9 @@
end

it "filters based on used codes" do
click_link "Filter"
click_on "Filter"
find("a", text: "Includes used codes").hover
click_link "Yes"
click_on "Yes"
table = find("table.stack.table-list")
rows = table.find("tbody").all("tr")
expect(rows.count).to eq(10)
Expand All @@ -56,9 +56,9 @@
expect(used_codes.text).to eq("0")
end

click_link "Filter"
click_on "Filter"
find("a", text: "Includes used codes").hover
click_link "No"
click_on "No"
rows = table.find("tbody").all("tr")
expect(rows.count).to eq(5)
rows.each do |row|
Expand Down
2 changes: 1 addition & 1 deletion spec/shared/school_select_examples.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
expect(page).to have_current_path(decidim_helsinki_smsauth.users_auth_sms_school_info_path)
expect(page).to have_content("Login successful")

click_button "Save and continue"
click_on "Save and continue"
expect(page).to have_current_path(decidim_helsinki_smsauth.users_auth_sms_school_info_path)
within ".user-person" do
expect(page).to have_content("There is an error in this field.")
Expand Down
10 changes: 5 additions & 5 deletions spec/shared/shared_context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ def initialize(phone_number, code, organization: nil, queued: false)
end

shared_context "with single access code" do
let!(:creator) { create(:user, :confirmed, :admin, organization: organization) }
let!(:signin_code_set) { create(:signin_code_set, creator: creator) }
let!(:signin_code) { create(:signin_code, code: access_code, signin_code_set: signin_code_set) }
let!(:creator) { create(:user, :confirmed, :admin, organization:) }
let!(:signin_code_set) { create(:signin_code_set, creator:) }
let!(:signin_code) { create(:signin_code, code: access_code, signin_code_set:) }
end

shared_context "with helsinki_smsauth_id authorization" do
let!(:organization) { create(:organization, omniauth_settings: omniauth_settings, available_authorizations: available_authorizations) }
let!(:user) { create(:user, :confirmed, organization: organization) }
let!(:organization) { create(:organization, omniauth_settings:, available_authorizations:) }
let!(:user) { create(:user, :confirmed, organization:) }
let(:available_authorizations) { ["helsinki_smsauth_id"] }
let(:omniauth_settings) do
{
Expand Down
Loading

0 comments on commit 65626ec

Please sign in to comment.