From aaa8f3f57226beffc7f068251a35c812ea9f51a5 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Thu, 23 Nov 2023 14:56:13 +1100
Subject: [PATCH] Per review, move logic to display ToS banner to a helper

It easier to understand when we can see the logic to display the banner
in the view.
---
 .../spree/admin/base_controller.rb            | 22 +-----------------
 app/helpers/admin/terms_of_service_helper.rb  | 23 +++++++++++++++++++
 .../admin/_terms_of_service_banner.html.haml  | 13 +++++------
 app/views/spree/layouts/_admin_body.html.haml |  2 +-
 4 files changed, 31 insertions(+), 29 deletions(-)
 create mode 100644 app/helpers/admin/terms_of_service_helper.rb

diff --git a/app/controllers/spree/admin/base_controller.rb b/app/controllers/spree/admin/base_controller.rb
index 399609d40f..c03756814e 100644
--- a/app/controllers/spree/admin/base_controller.rb
+++ b/app/controllers/spree/admin/base_controller.rb
@@ -9,6 +9,7 @@ module Spree
       helper 'admin/injection'
       helper 'admin/orders'
       helper 'admin/enterprises'
+      helper 'admin/terms_of_service'
       helper 'enterprise_fees'
       helper 'angular_form'
 
@@ -19,7 +20,6 @@ module Spree
       before_action :authorize_admin
       before_action :set_locale
       before_action :warn_invalid_order_cycles, if: :html_request?
-      before_action :check_updated_tos_accepted, if: :html_request?
 
       # Warn the user when they have an active order cycle with hubs that are not ready
       # for checkout (ie. does not have valid shipping and payment methods).
@@ -111,26 +111,6 @@ module Spree
         name = controller_name.classify
         "::Api::Admin::#{prefix}#{name}Serializer".constantize
       end
-
-      def check_updated_tos_accepted
-        @terms_of_service_banner = false
-
-        return unless spree_user_signed_in?
-
-        return if Spree::Config.enterprises_require_tos == false
-
-        return if accepted_tos?
-
-        @terms_of_service_banner = true
-      end
-
-      def accepted_tos?
-        file_uploaded_at = TermsOfServiceFile.updated_at
-
-        current_spree_user.terms_of_service_accepted_at.present? &&
-          current_spree_user.terms_of_service_accepted_at > file_uploaded_at &&
-          current_spree_user.terms_of_service_accepted_at < DateTime.now
-      end
     end
   end
 end
diff --git a/app/helpers/admin/terms_of_service_helper.rb b/app/helpers/admin/terms_of_service_helper.rb
new file mode 100644
index 0000000000..38683f1757
--- /dev/null
+++ b/app/helpers/admin/terms_of_service_helper.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+module Admin
+  module TermsOfServiceHelper
+    def tos_need_accepting?
+      return false unless spree_user_signed_in?
+
+      return false if Spree::Config.enterprises_require_tos == false
+
+      !accepted_tos?
+    end
+
+    private
+
+    def accepted_tos?
+      file_uploaded_at = TermsOfServiceFile.updated_at
+
+      current_spree_user.terms_of_service_accepted_at.present? &&
+        current_spree_user.terms_of_service_accepted_at > file_uploaded_at &&
+        current_spree_user.terms_of_service_accepted_at < DateTime.now
+    end
+  end
+end
diff --git a/app/views/admin/_terms_of_service_banner.html.haml b/app/views/admin/_terms_of_service_banner.html.haml
index 03a961ebcd..409d1ccf7d 100644
--- a/app/views/admin/_terms_of_service_banner.html.haml
+++ b/app/views/admin/_terms_of_service_banner.html.haml
@@ -1,9 +1,8 @@
 .banner-container#banner-container
-  - if @terms_of_service_banner == true
-    .terms-of-service-banner
-      .column-left
-        %p= t("admin.terms_of_service_have_been_updated_html", tos_link: link_to(t("admin.terms_of_service"), TermsOfServiceFile.current_url, target: "_blank"))
-      .column-right
-        %button{ data: { reflex: "click->Enterprise::User#accept_terms_of_services", id: current_spree_user&.id } }
-          = t("admin.accept_terms_of_service")
+  .terms-of-service-banner
+    .column-left
+      %p= t("admin.terms_of_service_have_been_updated_html", tos_link: link_to(t("admin.terms_of_service"), TermsOfServiceFile.current_url, target: "_blank"))
+    .column-right
+      %button{ data: { reflex: "click->Enterprise::User#accept_terms_of_services", id: current_spree_user&.id } }
+        = t("admin.accept_terms_of_service")
       
diff --git a/app/views/spree/layouts/_admin_body.html.haml b/app/views/spree/layouts/_admin_body.html.haml
index e730e1286b..63ed3a5284 100644
--- a/app/views/spree/layouts/_admin_body.html.haml
+++ b/app/views/spree/layouts/_admin_body.html.haml
@@ -59,7 +59,7 @@
                 %span= yield :sidebar_title
             = yield :sidebar
 
-  = render partial: "admin/terms_of_service_banner"
+  = render "admin/terms_of_service_banner" if tos_need_accepting?
     
 %script
   = raw "Spree.api_key = \"#{spree_current_user.try(:spree_api_key).to_s}\";"