From 703a3e0b7826a44b0362586b45ae67c157b4ef58 Mon Sep 17 00:00:00 2001 From: Rasmus Kjellberg <2277443+kjellberg@users.noreply.github.com> Date: Sat, 1 Jun 2024 09:14:37 +0200 Subject: [PATCH] refactor: separated services for users and teams. closes #25, closes #26, closes #17 --- app/controllers/application_controller.rb | 8 ++++ app/views/kiqr/onboarding/new.html.erb | 2 +- app/views/kiqr/settings/_form.html.erb | 4 +- gems/kiqr/Gemfile.lock | 13 +++++ .../controllers/kiqr/accounts_controller.rb | 4 +- .../controllers/kiqr/onboarding_controller.rb | 29 +++++++---- .../controllers/kiqr/settings_controller.rb | 24 ++++++---- gems/kiqr/config/locales/kiqr/en.yml | 4 ++ gems/kiqr/lib/kiqr.rb | 10 ++-- .../kiqr/lib/kiqr/services/accounts/create.rb | 42 ---------------- gems/kiqr/lib/kiqr/services/teams/create.rb | 24 ++++++++++ .../services/{accounts => teams}/update.rb | 6 ++- gems/kiqr/lib/kiqr/services/users/update.rb | 21 ++++++++ gems/kiqr/test/dummy/db/schema.rb | 18 ++++++- .../test/services/accounts/create_test.rb | 48 ------------------- .../test/services/accounts/update_test.rb | 41 ---------------- gems/kiqr/test/services/teams/create_test.rb | 31 ++++++++++++ gems/kiqr/test/services/teams/update_test.rb | 41 ++++++++++++++++ .../kiqr/onboarding_controller_test.rb | 4 +- test/system/accounts/accounts_test.rb | 10 ++-- test/system/users/onboarding_test.rb | 4 +- test/test_helper.rb | 8 ---- 22 files changed, 220 insertions(+), 176 deletions(-) delete mode 100644 gems/kiqr/lib/kiqr/services/accounts/create.rb create mode 100644 gems/kiqr/lib/kiqr/services/teams/create.rb rename gems/kiqr/lib/kiqr/services/{accounts => teams}/update.rb (81%) create mode 100644 gems/kiqr/lib/kiqr/services/users/update.rb delete mode 100644 gems/kiqr/test/services/accounts/create_test.rb delete mode 100644 gems/kiqr/test/services/accounts/update_test.rb create mode 100644 gems/kiqr/test/services/teams/create_test.rb create mode 100644 gems/kiqr/test/services/teams/update_test.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 79f7715..99a6422 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -20,6 +20,14 @@ def ensure_onboarded redirect_to onboarding_path if user_signed_in? && !current_user.onboarded? end + # Get the options for the locale form select field + def options_for_locale + I18n.available_locales.map do |locale| + [I18n.t("languages.#{locale}"), locale] + end + end + helper_method :options_for_locale + # Override the method to change the sign-in redirect path def after_sign_in_path_for(resource) if session[:after_sign_in_path].present? diff --git a/app/views/kiqr/onboarding/new.html.erb b/app/views/kiqr/onboarding/new.html.erb index 915b1d7..6d5126a 100644 --- a/app/views/kiqr/onboarding/new.html.erb +++ b/app/views/kiqr/onboarding/new.html.erb @@ -6,7 +6,7 @@ description: t(".description") ) do %>
- <%= render "kiqr/accounts/form", account: @account %> + <%= render "kiqr/settings/form", user: @user %>
<% end %> <% end %> diff --git a/app/views/kiqr/settings/_form.html.erb b/app/views/kiqr/settings/_form.html.erb index 931ece9..d1d61e4 100644 --- a/app/views/kiqr/settings/_form.html.erb +++ b/app/views/kiqr/settings/_form.html.erb @@ -1,7 +1,7 @@ -<%= simple_form_for(user, url: settings_path, method: :patch) do |f| %> +<%= simple_form_for(user, url: form_submit_path, method: form_method) do |f| %> <%= f.simple_fields_for :personal_account, user.personal_account do |pa| %> - <%= pa.input :name %> + <%= pa.input :name, label: t(".personal_account.name.label"), placeholder: t(".personal_account.name.placeholder") %> <% end %> <%= f.input :locale, label: t(".locale.label"), placeholder: t(".locale.placeholder"), required: true, autofocus: true, prompt: t(".locale.prompt"), as: :select, collection: options_for_locale %> diff --git a/gems/kiqr/Gemfile.lock b/gems/kiqr/Gemfile.lock index 4f63dbe..e73919a 100644 --- a/gems/kiqr/Gemfile.lock +++ b/gems/kiqr/Gemfile.lock @@ -4,6 +4,8 @@ PATH kiqr (0.1.0) devise (~> 4.9, >= 4.9.3) devise-two-factor (~> 5.0.0) + omniauth (~> 2.1.1) + omniauth-rails_csrf_protection (~> 1.0.1) public_uid (~> 2.2) rails (>= 7.1.3.2) @@ -112,6 +114,7 @@ GEM i18n (>= 1.8.11, < 2) globalid (1.2.1) activesupport (>= 6.1) + hashie (5.0.0) i18n (1.14.4) concurrent-ruby (~> 1.0) io-console (0.7.2) @@ -142,6 +145,13 @@ GEM nio4r (2.7.1) nokogiri (1.16.3-arm64-darwin) racc (~> 1.4) + omniauth (2.1.2) + hashie (>= 3.4.6) + rack (>= 2.2.3) + rack-protection + omniauth-rails_csrf_protection (1.0.1) + actionpack (>= 4.2) + omniauth (~> 2.0) orm_adapter (0.5.0) psych (5.1.2) stringio @@ -151,6 +161,9 @@ GEM nio4r (~> 2.0) racc (1.7.3) rack (3.0.10) + rack-protection (4.0.0) + base64 (>= 0.1.0) + rack (>= 3.0.0, < 4) rack-session (2.0.0) rack (>= 3.0.0) rack-test (2.1.0) diff --git a/gems/kiqr/app/controllers/kiqr/accounts_controller.rb b/gems/kiqr/app/controllers/kiqr/accounts_controller.rb index 6f48687..f6bc7a2 100644 --- a/gems/kiqr/app/controllers/kiqr/accounts_controller.rb +++ b/gems/kiqr/app/controllers/kiqr/accounts_controller.rb @@ -10,7 +10,7 @@ def create @account = Account.new(account_params) if @account.valid? - Kiqr::Services::Accounts::Create.call!(account: @account, user: current_user) + Kiqr::Services::Teams::Create.call!(account: @account, user: current_user) kiqr_flash_message(:notice, :account_created) redirect_to dashboard_path(account_id: @account) else @@ -22,7 +22,7 @@ def update @account.assign_attributes(account_params) if @account.valid? - Kiqr::Services::Accounts::Update.call!(account: @account, user: current_user) + Kiqr::Services::Teams::Update.call!(account: @account, user: current_user) kiqr_flash_message(:notice, :account_updated) redirect_to edit_account_path else diff --git a/gems/kiqr/app/controllers/kiqr/onboarding_controller.rb b/gems/kiqr/app/controllers/kiqr/onboarding_controller.rb index e3b0030..3b931c9 100644 --- a/gems/kiqr/app/controllers/kiqr/onboarding_controller.rb +++ b/gems/kiqr/app/controllers/kiqr/onboarding_controller.rb @@ -18,25 +18,38 @@ class OnboardingController < KiqrController end def new - @account = current_user.build_personal_account(personal: true) + @user = current_user + @user.build_personal_account end def create - @account = Account.new(account_params) + @user = current_user + @user.assign_attributes(user_params) + @user.personal_account&.personal = true - if @account.valid? - Kiqr::Services::Accounts::Create.call!(account: @account, user: current_user, personal: true) - kiqr_flash_message(:notice, :account_created) - redirect_to after_onboarding_path + if @user.valid? + Kiqr::Services::Users::Update.call!(user: @user) + kiqr_flash_message(:notice, :settings_updated) + redirect_to after_sign_in_path_for(@user) else render :new, status: :unprocessable_entity end + # @account = Account.new(account_params) + + # if @account.valid? + # Kiqr::Services::Accounts::Create.call!(account: @account, user: current_user, personal: true) + # kiqr_flash_message(:notice, :account_created) + # redirect_to after_onboarding_path + # else + # render :new, status: :unprocessable_entity + # end end private - def account_params - params.require(:account).permit(Kiqr.config.account_attributes) + def user_params + personal_account_attributes = Kiqr.config.account_attributes.prepend(:id) + params.require(:user).permit(:time_zone, :locale, personal_account_attributes: personal_account_attributes) end # This is the path to redirect to after the onboarding process diff --git a/gems/kiqr/app/controllers/kiqr/settings_controller.rb b/gems/kiqr/app/controllers/kiqr/settings_controller.rb index 0afc72a..31e39e9 100644 --- a/gems/kiqr/app/controllers/kiqr/settings_controller.rb +++ b/gems/kiqr/app/controllers/kiqr/settings_controller.rb @@ -3,7 +3,10 @@ class SettingsController < KiqrController before_action :setup_user, only: %i[edit update] def update - if @user.update(preferences_params) + @user.assign_attributes(user_params) + + if @user.valid? + Kiqr::Services::Users::Update.call!(user: @user) kiqr_flash_message(:notice, :settings_updated) redirect_to edit_settings_path else @@ -11,20 +14,23 @@ def update end end + def form_submit_path + settings_path + end + helper_method :form_submit_path + + def form_method + :patch + end + helper_method :form_method + private def setup_user @user = current_user end - def options_for_locale - I18n.available_locales.map do |locale| - [I18n.t("languages.#{locale}"), locale] - end - end - helper_method :options_for_locale - - def preferences_params + def user_params personal_account_attributes = Kiqr.config.account_attributes.prepend(:id) params.require(:user).permit(:time_zone, :locale, personal_account_attributes: personal_account_attributes) end diff --git a/gems/kiqr/config/locales/kiqr/en.yml b/gems/kiqr/config/locales/kiqr/en.yml index d223ffd..2eabaea 100644 --- a/gems/kiqr/config/locales/kiqr/en.yml +++ b/gems/kiqr/config/locales/kiqr/en.yml @@ -98,6 +98,10 @@ en: title: "Settings" description: "Personal profile & settings" form: + personal_account: + name: + label: "Full name" + placeholder: "Enter your full name" time_zone: label: "Time zone" placeholder: "Select a time zone" diff --git a/gems/kiqr/lib/kiqr.rb b/gems/kiqr/lib/kiqr.rb index d457068..1402207 100644 --- a/gems/kiqr/lib/kiqr.rb +++ b/gems/kiqr/lib/kiqr.rb @@ -13,9 +13,9 @@ module Kiqr autoload :Config, "kiqr/config" module Services - module Accounts - autoload :Create, "kiqr/services/accounts/create" - autoload :Update, "kiqr/services/accounts/update" + module Teams + autoload :Create, "kiqr/services/teams/create" + autoload :Update, "kiqr/services/teams/update" end module Invitations @@ -24,6 +24,10 @@ module Invitations autoload :Destroy, "kiqr/services/invitations/destroy" autoload :Reject, "kiqr/services/invitations/reject" end + + module Users + autoload :Update, "kiqr/services/users/update" + end end module Errors diff --git a/gems/kiqr/lib/kiqr/services/accounts/create.rb b/gems/kiqr/lib/kiqr/services/accounts/create.rb deleted file mode 100644 index 9b1c19b..0000000 --- a/gems/kiqr/lib/kiqr/services/accounts/create.rb +++ /dev/null @@ -1,42 +0,0 @@ -module Kiqr - module Services - module Accounts - # Create account service - # User can create a personal account or a team account. - # User can have only one personal account. - # User can be part of multiple team accounts. - class Create < Kiqr::ApplicationService - def call(account:, user:, personal: false) - @account, @user = account, user - - personal ? create_personal_account : create_team_account - - success account - end - - private - - def create_personal_account - raise StandardError, "User already has a personal account" if user.personal_account&.persisted? - - user.transaction do - account.personal = true - user.personal_account = account - user.save! - end - end - - def create_team_account - account.transaction do - account.account_users.new(user: user, owner: true) - account.save! - end - end - - private - - attr_reader :account, :user - end - end - end -end diff --git a/gems/kiqr/lib/kiqr/services/teams/create.rb b/gems/kiqr/lib/kiqr/services/teams/create.rb new file mode 100644 index 0000000..2d385d1 --- /dev/null +++ b/gems/kiqr/lib/kiqr/services/teams/create.rb @@ -0,0 +1,24 @@ +module Kiqr + module Services + module Teams + # Create account service + # User can create a personal account or a team account. + # User can have only one personal account. + # User can be part of multiple team accounts. + class Create < Kiqr::ApplicationService + def call(account:, user:) + @account, @user = account, user + + account.account_users.new(user: user, owner: true) + account.save! + + success account + end + + private + + attr_reader :account, :user + end + end + end +end diff --git a/gems/kiqr/lib/kiqr/services/accounts/update.rb b/gems/kiqr/lib/kiqr/services/teams/update.rb similarity index 81% rename from gems/kiqr/lib/kiqr/services/accounts/update.rb rename to gems/kiqr/lib/kiqr/services/teams/update.rb index 162623f..ff54279 100644 --- a/gems/kiqr/lib/kiqr/services/accounts/update.rb +++ b/gems/kiqr/lib/kiqr/services/teams/update.rb @@ -1,10 +1,12 @@ module Kiqr module Services - module Accounts + module Teams # Update account service # User can update their personal account or if they are part of the team. class Update < Kiqr::ApplicationService def call(account:, user:) + raise StandardError, "Account is not a team" unless account.team? + @account, @user = account, user permission_check @@ -18,7 +20,7 @@ def call(account:, user:) # Check if user has permission to edit the account # User can edit their personal account or if they are part of the team. def permission_check - return if user.personal_account == account || account.account_users.find_by(user: user) + return if account.account_users.find_by(user: user) raise StandardError, "User does not have permission to edit this account" end diff --git a/gems/kiqr/lib/kiqr/services/users/update.rb b/gems/kiqr/lib/kiqr/services/users/update.rb new file mode 100644 index 0000000..6ec8e7f --- /dev/null +++ b/gems/kiqr/lib/kiqr/services/users/update.rb @@ -0,0 +1,21 @@ +module Kiqr + module Services + module Users + # Update account service + # User can update their personal account or if they are part of the team. + class Update < Kiqr::ApplicationService + def call(user:) + @user = user + + user.save! + + success user + end + + private + + attr_reader :user + end + end + end +end diff --git a/gems/kiqr/test/dummy/db/schema.rb b/gems/kiqr/test/dummy/db/schema.rb index 917447c..1e14bd0 100644 --- a/gems/kiqr/test/dummy/db/schema.rb +++ b/gems/kiqr/test/dummy/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_04_08_210931) do +ActiveRecord::Schema[7.1].define(version: 2024_04_24_103325) do create_table "account_invitations", force: :cascade do |t| t.string "public_uid" t.integer "account_id", null: false @@ -47,6 +47,21 @@ t.index ["public_uid"], name: "index_accounts_on_public_uid", unique: true end + create_table "omniauth_identities", force: :cascade do |t| + t.string "public_uid" + t.integer "user_id", null: false + t.string "provider", null: false + t.string "provider_uid", null: false + t.text "credentials" + t.text "info" + t.text "extra" + t.datetime "expires_at" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["public_uid"], name: "index_omniauth_identities_on_public_uid", unique: true + t.index ["user_id"], name: "index_omniauth_identities_on_user_id" + end + create_table "users", force: :cascade do |t| t.string "email", default: "", null: false t.string "encrypted_password", default: "", null: false @@ -84,5 +99,6 @@ add_foreign_key "account_invitations", "accounts" add_foreign_key "account_users", "accounts" add_foreign_key "account_users", "users" + add_foreign_key "omniauth_identities", "users" add_foreign_key "users", "accounts", column: "personal_account_id" end diff --git a/gems/kiqr/test/services/accounts/create_test.rb b/gems/kiqr/test/services/accounts/create_test.rb deleted file mode 100644 index 04987d0..0000000 --- a/gems/kiqr/test/services/accounts/create_test.rb +++ /dev/null @@ -1,48 +0,0 @@ -require "test_helper" - -module Kiqr - module Services - module Accounts - class CreateTest < ActionDispatch::IntegrationTest - def setup - @service = Kiqr::Services::Accounts::Create.new - end - - test "creates team account" do - user = create(:user) - account = build(:account) - @service.call(account:, user:, personal: false) - - refute_empty account.account_users, "Expected account_users not to be empty" - assert account.account_users.find_by(user: user).owner, "Expected account_users to have owner set to true" - refute account.personal, "Expected account.personal to be false" - assert account.persisted?, "Expected account to be saved" - end - - test "creates personal account" do - user = create(:user, personal_account: nil) - account = build(:account) - @service.call(account:, user:, personal: true) - - assert_empty account.account_users, "Expected account_users to be empty" - assert account.personal, "Expected account.personal to be true" - assert_equal account, user.personal_account, "Expected user to have personal account" - end - - test "can only have one personal account" do - user = create(:user) - assert_raises(StandardError) do - @service.call(account: Account.new(name: "Jane Doe"), user:, personal: true) - end - end - - test "account with invalid attributes raises error" do - user = create(:user) - assert_raises(StandardError) do - @service.call(account: build(:account, name: "no"), user:, personal: false) - end - end - end - end - end -end diff --git a/gems/kiqr/test/services/accounts/update_test.rb b/gems/kiqr/test/services/accounts/update_test.rb deleted file mode 100644 index f2d93de..0000000 --- a/gems/kiqr/test/services/accounts/update_test.rb +++ /dev/null @@ -1,41 +0,0 @@ -require "test_helper" - -module Kiqr - module Services - module Accounts - class UpdateTest < ActionDispatch::IntegrationTest - def setup - @service = Kiqr::Services::Accounts::Update.new - end - - test "updates personal account" do - user = create(:user) - account = user.personal_account - account.assign_attributes(name: "New Name") - - @service.call(account: account, user: user) - assert_equal "New Name", account.reload.name, "Expected account name to be updated" - end - - test "updates team account" do - account = create(:account) - user = create(:user, with_account: account) - account.assign_attributes(name: "New Team Name") - - @service.call(account: account, user: user) - assert_equal "New Team Name", account.reload.name, "Expected account name to be updated" - end - - test "account with invalid attributes raises error" do - user = create(:user) - alien_account = create(:account) - alien_account.assign_attributes(name: "no") - - assert_raises(StandardError) do - @service.call(account: alien_account, user: user) - end - end - end - end - end -end diff --git a/gems/kiqr/test/services/teams/create_test.rb b/gems/kiqr/test/services/teams/create_test.rb new file mode 100644 index 0000000..78126ee --- /dev/null +++ b/gems/kiqr/test/services/teams/create_test.rb @@ -0,0 +1,31 @@ +require "test_helper" + +module Kiqr + module Services + module Teams + class CreateTest < ActionDispatch::IntegrationTest + def setup + @service = Kiqr::Services::Teams::Create.new + end + + test "creates team account with ownership" do + user = create(:user) + team = build(:account) + @service.call(account: team, user:) + + refute_empty team.account_users, "Expected account_users not to be empty" + assert team.account_users.find_by(user: user).owner, "Expected account_users to have owner set to true" + refute team.personal, "Expected personal to be false" + assert team.persisted?, "Expected account to be saved" + end + + test "account with invalid attributes raises error" do + user = create(:user) + assert_raises(StandardError) do + @service.call(account: build(:account, name: "no"), user:) + end + end + end + end + end +end diff --git a/gems/kiqr/test/services/teams/update_test.rb b/gems/kiqr/test/services/teams/update_test.rb new file mode 100644 index 0000000..8ce1ebf --- /dev/null +++ b/gems/kiqr/test/services/teams/update_test.rb @@ -0,0 +1,41 @@ +require "test_helper" + +module Kiqr + module Services + module Teams + class UpdateTest < ActionDispatch::IntegrationTest + def setup + @service = Kiqr::Services::Teams::Update.new + end + + test "can't update personal account" do + personal_account = create(:account, personal: true) + user = create(:user, personal_account: personal_account) + + assert_raises(StandardError) do + @service.call(account: personal_account, user: user) + end + end + + test "updates team account" do + team = create(:account) + user = create(:user, with_account: team) + team.assign_attributes(name: "New Team Name") + + @service.call(account: team, user: user) + assert_equal "New Team Name", team.reload.name, "Expected account name to be updated" + end + + test "account with invalid attributes raises error" do + user = create(:user) + alien_team = create(:account) + alien_team.assign_attributes(name: "no") + + assert_raises(StandardError) do + @service.call(account: alien_team, user: user) + end + end + end + end + end +end diff --git a/test/controllers/kiqr/onboarding_controller_test.rb b/test/controllers/kiqr/onboarding_controller_test.rb index dad2457..c44033d 100644 --- a/test/controllers/kiqr/onboarding_controller_test.rb +++ b/test/controllers/kiqr/onboarding_controller_test.rb @@ -16,14 +16,14 @@ class Kiqr::OnboardingControllerTest < ActionDispatch::IntegrationTest test "can onboard user" do user = create(:user, personal_account: nil) sign_in user - post onboarding_path, params: {account: {name: "Personal Account"}} + post onboarding_path, params: {user: {personal_account_attributes: {name: "Foobar zoo"}}} assert_redirected_to dashboard_path assert user.reload.personal_account.personal? end test "validates user onboarding" do sign_in create(:user, personal_account: nil) - post onboarding_path, params: {account: {name: "no"}} + post onboarding_path, params: {user: {personal_account_attributes: {name: "no"}}} assert_response :unprocessable_entity assert_template :new end diff --git a/test/system/accounts/accounts_test.rb b/test/system/accounts/accounts_test.rb index 4613b72..5073980 100644 --- a/test/system/accounts/accounts_test.rb +++ b/test/system/accounts/accounts_test.rb @@ -10,13 +10,13 @@ class EditAccountsTest < ApplicationSystemTestCase visit edit_account_path(account_id: team_account) # Fill the team account form - fill_in_account_fields + fill_in "account[name]", with: "Foozoo free thinkers" click_on "commit" assert_text I18n.t("kiqr.flash_messages.account_updated") team_account.reload - assert_equal "New name", team_account.name + assert_equal "Foozoo free thinkers", team_account.name end test "can create team account" do @@ -26,14 +26,14 @@ class EditAccountsTest < ApplicationSystemTestCase visit new_account_path assert_selector("input[name='account[name]']") - # Fill the personal account setup form - fill_in_account_fields + # Fill the team account form + fill_in "account[name]", with: "Foobar code warriors" click_on "commit" assert_text I18n.t("kiqr.flash_messages.account_created") account = user.reload.accounts.last - assert_equal "New name", account.name + assert_equal "Foobar code warriors", account.name assert account.account_users.find_by(user: user).owner? assert_current_path dashboard_path(account_id: account) end diff --git a/test/system/users/onboarding_test.rb b/test/system/users/onboarding_test.rb index 0ff8faa..5a5ff9b 100644 --- a/test/system/users/onboarding_test.rb +++ b/test/system/users/onboarding_test.rb @@ -27,12 +27,12 @@ class OnboardingTest < ApplicationSystemTestCase assert_current_path onboarding_path # Fill the personal account setup form - fill_in_account_fields + fill_in "user[personal_account_attributes][name]", with: "Sven Bertilsson" click_on "commit" # Should be redirected to dashboard after successfully signing up. assert_current_path dashboard_path - assert_equal "New name", user.reload.personal_account.name + assert_equal "Sven Bertilsson", user.reload.personal_account.name end end diff --git a/test/test_helper.rb b/test/test_helper.rb index fa69ea5..4d17ea2 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -16,14 +16,6 @@ # Load migrations ActiveRecord::Schema.verbose = false -# Shared helper for filling in account fields. -# This helper is used in onboarding and accounts tests. -# You can add more fields as needed. -def fill_in_account_fields - fill_in "account[name]", with: "New name" - # fill_in "account[custom_field]", with: "Custom data" -end - module ActiveSupport class TestCase include Devise::Test::IntegrationHelpers