From 39a6b5d20ff118baa844a98455becf32e762bc5c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois=20Turbelin?= <pacodelaluna@gmail.com>
Date: Tue, 26 Apr 2022 00:02:34 +0200
Subject: [PATCH 1/5] Add disabled_at logic on spree users

---
 app/models/spree/user.rb                      | 26 +++++++++++++++
 app/services/permitted_attributes/user.rb     |  2 +-
 app/views/spree/admin/users/_form.html.haml   |  9 +++++-
 config/locales/en.yml                         |  1 +
 ...25230907_add_disabled_at_to_spree_users.rb |  5 +++
 db/schema.rb                                  |  3 +-
 spec/models/spree/user_spec.rb                | 32 +++++++++++++++++++
 7 files changed, 75 insertions(+), 3 deletions(-)
 create mode 100644 db/migrate/20220425230907_add_disabled_at_to_spree_users.rb

diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb
index 542e43a0b9..74e3614dd2 100644
--- a/app/models/spree/user.rb
+++ b/app/models/spree/user.rb
@@ -144,6 +144,32 @@ module Spree
       "#{self.class.name};#{id}"
     end
 
+    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
+    end
+
     protected
 
     def password_required?
diff --git a/app/services/permitted_attributes/user.rb b/app/services/permitted_attributes/user.rb
index 4efbf0e3b5..dd361440ef 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]
+      [:email, :password, :password_confirmation, :toggle_disable]
     end
   end
 end
diff --git a/app/views/spree/admin/users/_form.html.haml b/app/views/spree/admin/users/_form.html.haml
index 47bf167151..449ff284d0 100644
--- a/app/views/spree/admin/users/_form.html.haml
+++ b/app/views/spree/admin/users/_form.html.haml
@@ -23,4 +23,11 @@
     = f.field_container :password do
       = f.label :password_confirmation, t(".confirm_password")
       = f.password_field :password_confirmation, class: "fullwidth"
-      = f.error_message_on :password_confirmation
\ No newline at end of file
+      = 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
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 3820f0c536..be5ab9ac9c 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -3965,6 +3965,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?"
           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
new file mode 100644
index 0000000000..ab7de2c22a
--- /dev/null
+++ b/db/migrate/20220425230907_add_disabled_at_to_spree_users.rb
@@ -0,0 +1,5 @@
+class AddDisabledAtToSpreeUsers < ActiveRecord::Migration[6.1]
+  def up
+    add_column :spree_users, :disabled_at, :datetime
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index edfd135051..96cab7ce42 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2022_06_02_013938) do
+ActiveRecord::Schema.define(version: 2022_06_21_230907) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -1055,6 +1055,7 @@ ActiveRecord::Schema.define(version: 2022_06_02_013938) do
     t.datetime "confirmed_at"
     t.datetime "confirmation_sent_at"
     t.string "unconfirmed_email", limit: 255
+    t.datetime "disabled_at"
     t.index ["confirmation_token"], name: "index_spree_users_on_confirmation_token", unique: true
     t.index ["email"], name: "email_idx_unique", unique: true
     t.index ["persistence_token"], name: "index_users_on_persistence_token"
diff --git a/spec/models/spree/user_spec.rb b/spec/models/spree/user_spec.rb
index 0730a050c9..25e87f5bd4 100644
--- a/spec/models/spree/user_spec.rb
+++ b/spec/models/spree/user_spec.rb
@@ -201,4 +201,36 @@ describe Spree::User do
       expect(user.flipper_id).to eq "Spree::User;42"
     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
+    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
+    it "returns true with a disabled datetime" do
+      user = create(:user)
+      user.disable!
+      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
+    end
+  end
 end

From bc9dcc7cbcc1cfd5610cc37a40499e203a81cbaa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois=20Turbelin?= <pacodelaluna@gmail.com>
Date: Wed, 4 May 2022 10:42:24 +0200
Subject: [PATCH 2/5] Block disabled users from logging in

---
 app/controllers/application_controller.rb | 9 +++++++++
 config/locales/en.yml                     | 3 +++
 2 files changed, 12 insertions(+)

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 5020faf7b6..fc3658cdc3 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -39,6 +39,7 @@ class ApplicationController < ActionController::Base
   include Spree::Core::ControllerHelpers::Common
 
   before_action :set_cache_headers # prevent cart emptying via cache when using back button #1213
+  before_action :check_disabled_user, if: :spree_user_signed_in?
   before_action :set_after_login_url
 
   include RawParams
@@ -159,6 +160,14 @@ class ApplicationController < ActionController::Base
     response.headers["Pragma"] = "no-cache"
     response.headers["Expires"] = "Fri, 01 Jan 1990 00:00:00 GMT"
   end
+
+  def check_disabled_user
+    return unless current_spree_user.disabled?
+
+    flash[:success] = nil
+    flash.now[:error] = I18n.t("devise.failure.disabled")
+    sign_out current_spree_user
+  end
 end
 
 require 'spree/i18n/initializer'
diff --git a/config/locales/en.yml b/config/locales/en.yml
index be5ab9ac9c..a6eb54c63f 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -207,6 +207,9 @@ 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.
       invalid: |
         Invalid email or password.
         Were you a guest last time?  Perhaps you need to create an account or reset your password.

From 76ea7089d999733edcc782368347ccda238e724d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois=20Turbelin?= <pacodelaluna@gmail.com>
Date: Mon, 9 May 2022 21:55:02 +0200
Subject: [PATCH 3/5] 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

From 31f8864b4ab515718242bbb44329322f8b0157b9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois=20Turbelin?= <pacodelaluna@gmail.com>
Date: Mon, 9 May 2022 22:58:30 +0200
Subject: [PATCH 4/5] Add system spec for Admin user disable feature

---
 spec/system/admin/users_spec.rb | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/spec/system/admin/users_spec.rb b/spec/system/admin/users_spec.rb
index e95b081066..647112a268 100644
--- a/spec/system/admin/users_spec.rb
+++ b/spec/system/admin/users_spec.rb
@@ -90,6 +90,14 @@ describe "Managing users" do
           click_button "Clear key"
           expect(page).to have_content "NO KEY"
         end
+
+        it "should allow to disable the user" do
+          check "Disabled"
+          click_button "Update"
+
+          expect(page).to have_content("Account updated")
+          expect(page).to have_checked_field "Disabled"
+        end
       end
     end
 

From 606a56a64de9fd3f6145e45a3d1865e817fcb4de Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois=20Turbelin?= <pacodelaluna@gmail.com>
Date: Thu, 12 May 2022 12:51:26 +0200
Subject: [PATCH 5/5] Improve specs

---
 spec/models/spree/user_spec.rb  | 8 ++++----
 spec/system/admin/users_spec.rb | 8 +++++++-
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/spec/models/spree/user_spec.rb b/spec/models/spree/user_spec.rb
index 76da72d81c..d29cd3d038 100644
--- a/spec/models/spree/user_spec.rb
+++ b/spec/models/spree/user_spec.rb
@@ -205,7 +205,7 @@ describe Spree::User do
   describe "#disabled=" do
     context 'when the value is truthy' do
       it "sets disabled datetime" do
-        user = create(:user)
+        user = Spree::User.new
         expect(user.disabled_at).to be_nil
         user.disabled = '1'
         expect(user.disabled_at).not_to be_nil
@@ -214,7 +214,7 @@ describe Spree::User do
 
     context 'when the value is falsey' do
       it "clears disabled datetime" do
-        user = create(:user, disabled_at: Time.zone.now)
+        user = Spree::User.new(disabled_at: Time.zone.now)
         expect(user.disabled_at).not_to be_nil
         user.disabled = '0'
         expect(user.disabled_at).to be_nil
@@ -224,13 +224,13 @@ describe Spree::User do
 
   describe "#disabled" do
     it "returns true with a disabled datetime" do
-      user = create(:user)
+      user = Spree::User.new
       user.disabled = '1'
       expect(user.disabled).to be_truthy
     end
 
     it "returns false without a disabled datetime" do
-      user = create(:user)
+      user = Spree::User.new(disabled_at: Time.zone.now)
       user.disabled = '0'
       expect(user.disabled).to be_falsey
     end
diff --git a/spec/system/admin/users_spec.rb b/spec/system/admin/users_spec.rb
index 647112a268..8c558436bf 100644
--- a/spec/system/admin/users_spec.rb
+++ b/spec/system/admin/users_spec.rb
@@ -91,12 +91,18 @@ describe "Managing users" do
           expect(page).to have_content "NO KEY"
         end
 
-        it "should allow to disable the user" do
+        it "should allow to disable the user and to enable it" do
+          expect(page).to have_unchecked_field "Disabled"
           check "Disabled"
           click_button "Update"
 
           expect(page).to have_content("Account updated")
           expect(page).to have_checked_field "Disabled"
+          uncheck "Disabled"
+          click_button "Update"
+
+          expect(page).to have_content("Account updated")
+          expect(page).to have_unchecked_field "Disabled"
         end
       end
     end