From 179a82d2dd6757efa9d8dc3c31b29be0e63b7c25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 2 Feb 2024 15:50:25 +0200 Subject: [PATCH] Use keyword arguments for ApplicationService calls Not all services are using keywords, which breaks those calls in Ruby 3 --- app/controllers/admin/users_controller.rb | 2 +- app/controllers/settings_controller.rb | 12 ++++++------ app/controllers/signup_controller.rb | 4 ++-- app/models/app_catalog/web_app.rb | 2 +- app/models/user.rb | 4 ++-- .../app_catalog_manager/update_metadata.rb | 2 +- app/services/application_service.rb | 4 ++-- app/services/create_account.rb | 14 +++++++------- app/services/ldap_manager/fetch_avatar.rb | 5 ++--- app/services/ldap_manager/update_avatar.rb | 2 +- app/services/ldap_manager/update_display_name.rb | 2 +- app/services/ldap_manager/update_email.rb | 2 +- .../ldap_manager/update_email_maildrop.rb | 2 +- .../ldap_manager/update_email_password.rb | 2 +- app/services/nostr_manager/validate_id.rb | 2 +- app/services/nostr_manager/verify_signature.rb | 2 +- spec/features/settings/email_spec.rb | 2 +- spec/features/settings/profile_spec.rb | 2 +- spec/features/signup_spec.rb | 4 ++-- spec/models/user_spec.rb | 2 +- spec/services/create_account_spec.rb | 16 ++++++++-------- 21 files changed, 44 insertions(+), 45 deletions(-) diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 3cb5735..db5da30 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -21,7 +21,7 @@ class Admin::UsersController < Admin::BaseController @services_enabled = @user.services_enabled - @avatar = LdapManager::FetchAvatar.call(cn: @user.cn, ou: @user.ou) + @avatar = LdapManager::FetchAvatar.call(cn: @user.cn) end private diff --git a/app/controllers/settings_controller.rb b/app/controllers/settings_controller.rb index f1fa357..b054740 100644 --- a/app/controllers/settings_controller.rb +++ b/app/controllers/settings_controller.rb @@ -24,11 +24,11 @@ class SettingsController < ApplicationController if @user.save if @user.display_name && (@user.display_name != @user.ldap_entry[:display_name]) - LdapManager::UpdateDisplayName.call(@user.dn, @user.display_name) + LdapManager::UpdateDisplayName.call(dn: @user.dn, display_name: @user.display_name) end if @user.avatar_new.present? - LdapManager::UpdateAvatar.call(@user.dn, @user.avatar_new) + LdapManager::UpdateAvatar.call(dn: @user.dn, file: @user.avatar_new) end redirect_to setting_path(@settings_section), flash: { @@ -64,10 +64,10 @@ class SettingsController < ApplicationController @user.current_password = nil session[:new_email_password] = generate_email_password hashed_password = hash_email_password(session[:new_email_password]) - LdapManager::UpdateEmailPassword.call(@user.dn, hashed_password) + LdapManager::UpdateEmailPassword.call(dn: @user.dn, password_hash: hashed_password) if @user.ldap_entry[:email_maildrop] != @user.address - LdapManager::UpdateEmailMaildrop.call(@user.dn, @user.address) + LdapManager::UpdateEmailMaildrop.call(dn: @user.dn, address: @user.address) end redirect_to new_password_services_email_path @@ -88,8 +88,8 @@ class SettingsController < ApplicationController def set_nostr_pubkey signed_event = nostr_event_params[:signed_event].to_h.symbolize_keys - is_valid_id = NostrManager::ValidateId.call(signed_event) - is_valid_sig = NostrManager::VerifySignature.call(signed_event) + is_valid_id = NostrManager::ValidateId.call(event: signed_event) + is_valid_sig = NostrManager::VerifySignature.call(event: signed_event) is_correct_content = signed_event[:content] == "Connect my public key to #{current_user.address} (confirmation #{session[:shared_secret]})" unless is_valid_id && is_valid_sig && is_correct_content diff --git a/app/controllers/signup_controller.rb b/app/controllers/signup_controller.rb index c21820b..236db54 100644 --- a/app/controllers/signup_controller.rb +++ b/app/controllers/signup_controller.rb @@ -96,13 +96,13 @@ class SignupController < ApplicationController session[:new_user] = nil session[:validation_error] = nil - CreateAccount.call( + CreateAccount.call(account: { username: @user.cn, domain: Setting.primary_domain, email: @user.email, password: @user.password, invitation: @invitation - ) + }) end def set_context diff --git a/app/models/app_catalog/web_app.rb b/app/models/app_catalog/web_app.rb index b42bea7..0b88333 100644 --- a/app/models/app_catalog/web_app.rb +++ b/app/models/app_catalog/web_app.rb @@ -11,6 +11,6 @@ class AppCatalog::WebApp < ApplicationRecord if: Proc.new { |a| a.url.present? } def update_metadata - AppCatalogManager::UpdateMetadata.call(self) + AppCatalogManager::UpdateMetadata.call(app: self) end end diff --git a/app/models/user.rb b/app/models/user.rb index 7bc6091..9620bfe 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -92,7 +92,7 @@ class User < ApplicationRecord def devise_after_confirmation if ldap_entry[:mail] != self.email # E-Mail update confirmed - LdapManager::UpdateEmail.call(self.dn, self.email) + LdapManager::UpdateEmail.call(dn: self.dn, address: self.email) else # E-Mail from signup confirmed (i.e. account activation) @@ -164,7 +164,7 @@ class User < ApplicationRecord end def avatar - @avatar_base64 ||= LdapManager::FetchAvatar.call(cn: cn, ou: ou) + @avatar_base64 ||= LdapManager::FetchAvatar.call(cn: cn) end def services_enabled diff --git a/app/services/app_catalog_manager/update_metadata.rb b/app/services/app_catalog_manager/update_metadata.rb index d78c5d8..c11b864 100644 --- a/app/services/app_catalog_manager/update_metadata.rb +++ b/app/services/app_catalog_manager/update_metadata.rb @@ -3,7 +3,7 @@ require "down" module AppCatalogManager class UpdateMetadata < AppCatalogManagerService - def initialize(app) + def initialize(app:) @app = app end diff --git a/app/services/application_service.rb b/app/services/application_service.rb index 401fe02..2d0394b 100644 --- a/app/services/application_service.rb +++ b/app/services/application_service.rb @@ -1,7 +1,7 @@ class ApplicationService # This enables executing a service's `#call` method directly via # `MyService.call(args)`, without creating a class instance it first. - def self.call(*args, &block) - new(*args, &block).call + def self.call(**args, &block) + new(**args, &block).call end end diff --git a/app/services/create_account.rb b/app/services/create_account.rb index 8c732cf..4e31200 100644 --- a/app/services/create_account.rb +++ b/app/services/create_account.rb @@ -1,11 +1,11 @@ class CreateAccount < ApplicationService - def initialize(args) - @username = args[:username] - @domain = args[:ou] || Setting.primary_domain - @email = args[:email] - @password = args[:password] - @invitation = args[:invitation] - @confirmed = args[:confirmed] + def initialize(account:) + @username = account[:username] + @domain = account[:ou] || Setting.primary_domain + @email = account[:email] + @password = account[:password] + @invitation = account[:invitation] + @confirmed = account[:confirmed] end def call diff --git a/app/services/ldap_manager/fetch_avatar.rb b/app/services/ldap_manager/fetch_avatar.rb index 9d8f2b6..c2643f7 100644 --- a/app/services/ldap_manager/fetch_avatar.rb +++ b/app/services/ldap_manager/fetch_avatar.rb @@ -1,12 +1,11 @@ module LdapManager class FetchAvatar < LdapManagerService - def initialize(cn:, ou: nil) + def initialize(cn:) @cn = cn - @ou = ou end def call - treebase = @ou ? "ou=#{@ou},cn=users,#{suffix}" : ldap_config["base"] + treebase = ldap_config["base"] attributes = %w{ jpegPhoto } filter = Net::LDAP::Filter.eq("cn", @cn) diff --git a/app/services/ldap_manager/update_avatar.rb b/app/services/ldap_manager/update_avatar.rb index f26b92c..f3c8975 100644 --- a/app/services/ldap_manager/update_avatar.rb +++ b/app/services/ldap_manager/update_avatar.rb @@ -2,7 +2,7 @@ require "image_processing/vips" module LdapManager class UpdateAvatar < LdapManagerService - def initialize(dn, file) + def initialize(dn:, file:) @dn = dn @img_data = process(file) end diff --git a/app/services/ldap_manager/update_display_name.rb b/app/services/ldap_manager/update_display_name.rb index 2d3e90f..85418d3 100644 --- a/app/services/ldap_manager/update_display_name.rb +++ b/app/services/ldap_manager/update_display_name.rb @@ -1,6 +1,6 @@ module LdapManager class UpdateDisplayName < LdapManagerService - def initialize(dn, display_name) + def initialize(dn:, display_name:) @dn = dn @display_name = display_name end diff --git a/app/services/ldap_manager/update_email.rb b/app/services/ldap_manager/update_email.rb index 80f40e4..9df3f17 100644 --- a/app/services/ldap_manager/update_email.rb +++ b/app/services/ldap_manager/update_email.rb @@ -1,6 +1,6 @@ module LdapManager class UpdateEmail < LdapManagerService - def initialize(dn, address) + def initialize(dn:, address:) @dn = dn @address = address end diff --git a/app/services/ldap_manager/update_email_maildrop.rb b/app/services/ldap_manager/update_email_maildrop.rb index d26cf16..1945308 100644 --- a/app/services/ldap_manager/update_email_maildrop.rb +++ b/app/services/ldap_manager/update_email_maildrop.rb @@ -1,6 +1,6 @@ module LdapManager class UpdateEmailMaildrop < LdapManagerService - def initialize(dn, address) + def initialize(dn:, address:) @dn = dn @address = address end diff --git a/app/services/ldap_manager/update_email_password.rb b/app/services/ldap_manager/update_email_password.rb index e9cde6a..7ef327a 100644 --- a/app/services/ldap_manager/update_email_password.rb +++ b/app/services/ldap_manager/update_email_password.rb @@ -1,6 +1,6 @@ module LdapManager class UpdateEmailPassword < LdapManagerService - def initialize(dn, password_hash) + def initialize(dn:, password_hash:) @dn = dn @password_hash = password_hash end diff --git a/app/services/nostr_manager/validate_id.rb b/app/services/nostr_manager/validate_id.rb index d184b75..e838441 100644 --- a/app/services/nostr_manager/validate_id.rb +++ b/app/services/nostr_manager/validate_id.rb @@ -1,6 +1,6 @@ module NostrManager class ValidateId < NostrManagerService - def initialize(event) + def initialize(event:) @event = Nostr::Event.new(**event) end diff --git a/app/services/nostr_manager/verify_signature.rb b/app/services/nostr_manager/verify_signature.rb index 63b0489..9c62234 100644 --- a/app/services/nostr_manager/verify_signature.rb +++ b/app/services/nostr_manager/verify_signature.rb @@ -1,6 +1,6 @@ module NostrManager class VerifySignature < NostrManagerService - def initialize(event) + def initialize(event:) @event = Nostr::Event.new(**event) end diff --git a/spec/features/settings/email_spec.rb b/spec/features/settings/email_spec.rb index 348d0be..b8bac1d 100644 --- a/spec/features/settings/email_spec.rb +++ b/spec/features/settings/email_spec.rb @@ -50,7 +50,7 @@ RSpec.describe 'E-Mail settings', type: :feature do expect(LdapManager::UpdateEmailPassword).to receive(:call).and_return(true) expect(LdapManager::UpdateEmailMaildrop).to receive(:call) - .with(user.dn, user.address).and_return(true) + .with(dn: user.dn, address: user.address).and_return(true) visit setting_path(:email) fill_in 'Current account password', with: "valid password" diff --git a/spec/features/settings/profile_spec.rb b/spec/features/settings/profile_spec.rb index d6eb8ff..764da2d 100644 --- a/spec/features/settings/profile_spec.rb +++ b/spec/features/settings/profile_spec.rb @@ -29,7 +29,7 @@ RSpec.describe 'Profile settings', type: :feature do scenario 'works with valid input' do expect(LdapManager::UpdateDisplayName).to receive(:call) - .with(user.dn, "Marky Mark").and_return(true) + .with(dn: user.dn, display_name: "Marky Mark").and_return(true) visit setting_path(:profile) fill_in 'Display name', with: "Marky Mark" diff --git a/spec/features/signup_spec.rb b/spec/features/signup_spec.rb index 30511a7..e668e9d 100644 --- a/spec/features/signup_spec.rb +++ b/spec/features/signup_spec.rb @@ -53,7 +53,7 @@ RSpec.describe "Signup", type: :feature do expect(page).to have_content("Choose a password") expect(CreateAccount).to receive(:call) - .with({ + .with(account: { username: "tony", domain: "kosmos.org", email: "tony@example.com", password: "a-valid-password", invitation: Invitation.last @@ -97,7 +97,7 @@ RSpec.describe "Signup", type: :feature do expect(page).to have_content("Password is too short") expect(CreateAccount).to receive(:call) - .with({ + .with(account: { username: "tony", domain: "kosmos.org", email: "tony@example.com", password: "a-valid-password", invitation: Invitation.last diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 5253c62..7011793 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -190,7 +190,7 @@ RSpec.describe User, type: :model do it "updates the LDAP 'mail' attribute" do expect(LdapManager::UpdateEmail).to receive(:call) - .with("cn=willherschel,ou=kosmos.org,cn=users,dc=kosmos,dc=org", "will@hrsch.el") + .with(dn: "cn=willherschel,ou=kosmos.org,cn=users,dc=kosmos,dc=org", address: "will@hrsch.el") user.send :devise_after_confirmation end diff --git a/spec/services/create_account_spec.rb b/spec/services/create_account_spec.rb index 064d1ab..5155f19 100644 --- a/spec/services/create_account_spec.rb +++ b/spec/services/create_account_spec.rb @@ -2,11 +2,11 @@ require 'rails_helper' RSpec.describe CreateAccount, type: :model do describe "#create_user_in_database" do - let(:service) { CreateAccount.new( + let(:service) { CreateAccount.new(account: { username: 'isaacnewton', email: 'isaacnewton@example.com', password: 'bright-ideas-in-autumn' - )} + })} it "creates a new user record in the akkounts database" do expect(User.count).to eq(0) @@ -19,12 +19,12 @@ RSpec.describe CreateAccount, type: :model do describe "#update_invitation" do let(:invitation) { create :invitation } - let(:service) { CreateAccount.new( + let(:service) { CreateAccount.new(account: { username: 'isaacnewton', email: 'isaacnewton@example.com', password: 'bright-ideas-in-autumn', invitation: invitation - )} + })} before(:each) do service.send(:update_invitation, 23) @@ -42,11 +42,11 @@ RSpec.describe CreateAccount, type: :model do describe "#add_ldap_document" do include ActiveJob::TestHelper - let(:service) { CreateAccount.new( + let(:service) { CreateAccount.new(account: { username: 'halfinney', email: 'halfinney@example.com', password: 'remember-remember-the-5th-of-november' - )} + })} it "enqueues a job to create the LDAP user document" do service.send(:add_ldap_document) @@ -68,10 +68,10 @@ RSpec.describe CreateAccount, type: :model do describe "#create_lndhub_account" do include ActiveJob::TestHelper - let(:service) { CreateAccount.new( + let(:service) { CreateAccount.new(account: { username: 'halfinney', email: 'halfinney@example.com', password: 'bright-ideas-in-winter' - )} + })} let(:new_user) { create :user, cn: "halfinney", ou: "kosmos.org" } it "enqueues a job to create an LndHub account" do