From 76ea7089d999733edcc782368347ccda238e724d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Turbelin?= Date: Mon, 9 May 2022 21:55:02 +0200 Subject: [PATCH] Implement suggestions --- app/controllers/application_controller.rb | 2 +- app/models/spree/user.rb | 24 ++--------- app/services/permitted_attributes/user.rb | 2 +- app/views/spree/admin/users/_form.html.haml | 11 ++--- config/locales/en.yml | 6 +-- ...25230907_add_disabled_at_to_spree_users.rb | 2 +- spec/models/spree/user_spec.rb | 42 ++++++++++--------- 7 files changed, 34 insertions(+), 55 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index fc3658cdc3..9c22277244 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -162,7 +162,7 @@ class ApplicationController < ActionController::Base end def check_disabled_user - return unless current_spree_user.disabled? + return unless current_spree_user.disabled flash[:success] = nil flash.now[:error] = I18n.t("devise.failure.disabled") diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index 74e3614dd2..3ff88d8fa9 100644 --- a/app/models/spree/user.rb +++ b/app/models/spree/user.rb @@ -144,30 +144,12 @@ module Spree "#{self.class.name};#{id}" end - def disabled? + def disabled disabled_at.present? end - def disable! - self.disabled_at = Time.zone.now - save! - end - - def enable! - self.disabled_at = nil - save! - end - - def toggle_disable - disabled? - end - - def toggle_disable=(value) - if value == '1' - disable! - else - enable! - end + def disabled=(value) + self.disabled_at = value == '1' ? Time.zone.now : nil end protected diff --git a/app/services/permitted_attributes/user.rb b/app/services/permitted_attributes/user.rb index dd361440ef..0b511bab21 100644 --- a/app/services/permitted_attributes/user.rb +++ b/app/services/permitted_attributes/user.rb @@ -15,7 +15,7 @@ module PermittedAttributes private def permitted_attributes - [:email, :password, :password_confirmation, :toggle_disable] + [:email, :password, :password_confirmation, :disabled] end end end diff --git a/app/views/spree/admin/users/_form.html.haml b/app/views/spree/admin/users/_form.html.haml index 449ff284d0..5262ca05df 100644 --- a/app/views/spree/admin/users/_form.html.haml +++ b/app/views/spree/admin/users/_form.html.haml @@ -24,10 +24,7 @@ = f.label :password_confirmation, t(".confirm_password") = f.password_field :password_confirmation, class: "fullwidth" = f.error_message_on :password_confirmation - = f.field_container :disable do - = f.label :disable, t(".disable") - =# check_box_tag "user[toggle_disable]", "1", @user.disabled?, id: "user_toggle_disable" - = f.check_box :toggle_disable - = f.error_message_on :disable - - if @user.disabled? - = label_tag @user.disabled_at \ No newline at end of file + = f.field_container :disabled do + = f.label :disabled, t(".disabled") + = f.check_box :disabled + = f.error_message_on :disabled diff --git a/config/locales/en.yml b/config/locales/en.yml index a6eb54c63f..8aae021b39 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -207,9 +207,7 @@ en: signed_up_but_unconfirmed: "A message with a confirmation link has been sent to your email address. Please open the link to activate your account." unknown_error: "Something went wrong while creating your account. Check your email address and try again." failure: - disabled: | - Your account has been disabled. - Please contact an adminsitrator to solve this issue. + disabled: "Your account has been disabled. Please contact an administrator to solve this issue." invalid: | Invalid email or password. Were you a guest last time? Perhaps you need to create an account or reset your password. @@ -3968,7 +3966,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using back_to_users_list: "Back To Users List" general_settings: "General Settings" form: - disable: "Disable?" + disabled: "Disabled?" email: "Email" roles: "Roles" enterprise_limit: "Enterprise Limit" diff --git a/db/migrate/20220425230907_add_disabled_at_to_spree_users.rb b/db/migrate/20220425230907_add_disabled_at_to_spree_users.rb index ab7de2c22a..a9b4b55d62 100644 --- a/db/migrate/20220425230907_add_disabled_at_to_spree_users.rb +++ b/db/migrate/20220425230907_add_disabled_at_to_spree_users.rb @@ -1,5 +1,5 @@ class AddDisabledAtToSpreeUsers < ActiveRecord::Migration[6.1] - def up + def change add_column :spree_users, :disabled_at, :datetime end end diff --git a/spec/models/spree/user_spec.rb b/spec/models/spree/user_spec.rb index 25e87f5bd4..76da72d81c 100644 --- a/spec/models/spree/user_spec.rb +++ b/spec/models/spree/user_spec.rb @@ -202,35 +202,37 @@ describe Spree::User do end end - describe "#disable!" do - it "sets disabled datetime" do - user = create(:user) - expect(user.disabled_at).to be_nil - user.disable! - expect(user.disabled_at).not_to be_nil + describe "#disabled=" do + context 'when the value is truthy' do + it "sets disabled datetime" do + user = create(:user) + expect(user.disabled_at).to be_nil + user.disabled = '1' + expect(user.disabled_at).not_to be_nil + end + end + + context 'when the value is falsey' do + it "clears disabled datetime" do + user = create(:user, disabled_at: Time.zone.now) + expect(user.disabled_at).not_to be_nil + user.disabled = '0' + expect(user.disabled_at).to be_nil + end end end - describe "#enable!" do - it "clears disabled datetime" do - user = create(:user, disabled_at: Time.zone.now) - expect(user.disabled_at).not_to be_nil - user.enable! - expect(user.disabled_at).to be_nil - end - end - - describe "#disabled?" do + describe "#disabled" do it "returns true with a disabled datetime" do user = create(:user) - user.disable! - expect(user.disabled?).to be_truthy + user.disabled = '1' + expect(user.disabled).to be_truthy end it "returns false without a disabled datetime" do user = create(:user) - user.enable! - expect(user.disabled?).to be_falsey + user.disabled = '0' + expect(user.disabled).to be_falsey end end end