From bf53a02270f648e72c6994eb323192bc7b094d3d Mon Sep 17 00:00:00 2001
From: Philipp Winkler <winkler@webit.de>
Date: Wed, 13 Jul 2022 20:57:38 +0200
Subject: [PATCH] Add api key toggle view checkbox

---
 .../spree/admin/users_controller.rb           | 35 +++++-------
 app/controllers/spree/api_keys_controller.rb  | 57 +++++++++++++++++++
 app/models/spree/user.rb                      | 14 ++---
 .../spree/admin/users/_api_fields.html.haml   | 18 ++++--
 app/views/spree/users/_api_keys.html.haml     |  7 +++
 .../spree/users/_developer_settings.html.haml |  3 +
 app/views/spree/users/show.html.haml          |  6 ++
 config/locales/en.yml                         |  8 +++
 config/routes/spree.rb                        |  8 +--
 ...w_api_key_generation_view_to_spree_user.rb |  5 ++
 db/schema.rb                                  |  3 +-
 lib/open_food_network/spree_api_key_loader.rb |  6 +-
 lib/spree/core/controller_helpers/auth.rb     |  3 +-
 .../spree/admin/users_controller_spec.rb      | 14 +----
 .../spree/api_keys_controller_spec.rb         | 45 +++++++++++++++
 spec/requests/api/orders_spec.rb              |  3 +-
 spec/system/admin/users_spec.rb               | 21 +++++++
 .../account/developer_settings_spec.rb        | 51 +++++++++++++++++
 18 files changed, 248 insertions(+), 59 deletions(-)
 create mode 100644 app/controllers/spree/api_keys_controller.rb
 create mode 100644 app/views/spree/users/_api_keys.html.haml
 create mode 100644 app/views/spree/users/_developer_settings.html.haml
 create mode 100644 db/migrate/20220713195433_add_show_api_key_generation_view_to_spree_user.rb
 create mode 100644 spec/controllers/spree/api_keys_controller_spec.rb
 create mode 100644 spec/system/consumer/account/developer_settings_spec.rb

diff --git a/app/controllers/spree/admin/users_controller.rb b/app/controllers/spree/admin/users_controller.rb
index 4cbd0b3e08..77368b2f1b 100644
--- a/app/controllers/spree/admin/users_controller.rb
+++ b/app/controllers/spree/admin/users_controller.rb
@@ -48,30 +48,11 @@ module Spree
             @user.spree_roles = roles.reject(&:blank?).collect{ |r| Spree::Role.find(r) }
           end
 
-          message = if new_email_unconfirmed?
-                      Spree.t(:email_updated)
-                    else
-                      Spree.t(:account_updated)
-                    end
-          flash.now[:success] = message
+          flash.now[:success] = update_message
         end
         render :edit
       end
 
-      def generate_api_key
-        if @user.generate_spree_api_key!
-          flash[:success] = t('spree.api.key_generated')
-        end
-        redirect_to spree.edit_admin_user_path(@user)
-      end
-
-      def clear_api_key
-        if @user.clear_spree_api_key!
-          flash[:success] = t('spree.api.key_cleared')
-        end
-        redirect_to spree.edit_admin_user_path(@user)
-      end
-
       protected
 
       def collection
@@ -100,6 +81,16 @@ module Spree
 
       private
 
+      def update_message
+        return Spree.t(:show_api_key_view_toggled) if @user.show_api_key_view_previously_changed?
+
+        if new_email_unconfirmed?
+          Spree.t(:email_updated)
+        else
+          Spree.t(:account_updated)
+        end
+      end
+
       # handling raise from Admin::ResourceController#destroy
       def user_destroy_with_orders_error
         render status: :forbidden, text: Spree.t(:error_user_destroy_with_orders)
@@ -137,7 +128,9 @@ module Spree
       end
 
       def user_params
-        ::PermittedAttributes::User.new(params).call([:enterprise_limit])
+        ::PermittedAttributes::User.new(params).call(
+          %i[enterprise_limit show_api_key_view]
+        )
       end
     end
   end
diff --git a/app/controllers/spree/api_keys_controller.rb b/app/controllers/spree/api_keys_controller.rb
new file mode 100644
index 0000000000..e852afc23d
--- /dev/null
+++ b/app/controllers/spree/api_keys_controller.rb
@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+module Spree
+  class ApiKeysController < ::BaseController
+    include Spree::Core::ControllerHelpers
+    include I18nHelper
+
+    prepend_before_action :load_object
+
+    def create
+      @user.generate_api_key
+
+      if @user.save
+        flash[:success] = t('spree.api.key_generated')
+      end
+
+      redirect_to redirect_path
+    end
+
+    def destroy
+      @user.spree_api_key = nil
+
+      if @user.save
+        flash[:success] = t('spree.api.key_cleared')
+      end
+
+      redirect_to redirect_path
+    end
+
+    private
+
+    def load_object
+      @user ||= find_user
+      if @user
+        authorize! params[:action].to_sym, @user
+      else
+        redirect_to main_app.login_path
+      end
+    end
+
+    def find_user
+      Spree::User.find_by(id: params[:id]) || spree_current_user
+    end
+
+    def redirect_path
+      if request.referer.blank? || request.referer.include?(spree.account_path)
+        developer_settings_path
+      else
+        request.referer
+      end
+    end
+
+    def developer_settings_path
+      "#{spree.account_path}#/developer_settings"
+    end
+  end
+end
diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb
index 7d008ad310..b032836217 100644
--- a/app/models/spree/user.rb
+++ b/app/models/spree/user.rb
@@ -71,6 +71,10 @@ module Spree
       set_reset_password_token
     end
 
+    def generate_api_key
+      self.spree_api_key = SecureRandom.hex(24)
+    end
+
     def known_users
       if admin?
         Spree::User.where(nil)
@@ -132,16 +136,6 @@ module Spree
       end
     end
 
-    def generate_spree_api_key!
-      self.spree_api_key = SecureRandom.hex(24)
-      save!
-    end
-
-    def clear_spree_api_key!
-      self.spree_api_key = nil
-      save!
-    end
-
     def last_incomplete_spree_order
       spree_orders.incomplete.where(created_by_id: id).order('created_at DESC').first
     end
diff --git a/app/views/spree/admin/users/_api_fields.html.haml b/app/views/spree/admin/users/_api_fields.html.haml
index bd4bf6eb3f..441a410eba 100644
--- a/app/views/spree/admin/users/_api_fields.html.haml
+++ b/app/views/spree/admin/users/_api_fields.html.haml
@@ -1,17 +1,23 @@
 %fieldset.omega.six.columns
   %legend= t('spree.api.access')
+  = form_with(model: @user, url: spree.admin_user_path(@user)) do |form|
+    = form.check_box :show_api_key_view, onchange: "this.form.submit()"
+    = form.label :show_api_key_view, t('spree.api.toggle_api_key_view')
+
   - if @user.spree_api_key.present?
     .field
       = label_tag t('spree.api.key')
       = ":"
       = @user.spree_api_key
+
     .filter-actions.actions
-      = form_tag spree.clear_api_key_admin_user_path(@user), method: :put do
-        = button t('spree.api.clear_key'), 'icon-trash'
-      = form_tag spree.generate_api_key_admin_user_path(@user), method: :put do
-        = button t('spree.api.regenerate_key'), 'icon-refresh'
+      = form_with(model: @user, url: spree.api_key_path(id: @user), method: :delete) do |form|
+        = form.button t('spree.api.clear_key'), class: 'icon-trash', icon: 'icon-trash'
+      = form_with(model: @user, url: spree.api_keys_path(id: @user), method: :post) do |form|
+        = form.button t('spree.api.regenerate_key'), class: 'icon-refresh', icon: 'icon-refresh'
+
   - else
     .no-objects-found= t('spree.api.no_key')
     .filter-actions.actions
-      = form_tag spree.generate_api_key_admin_user_path(@user), method: :put do
-        = button t('spree.api.generate_key'), 'icon-key'
+      = form_with(model: @user, url: spree.api_keys_path(id: @user), method: :post) do |form|
+        = form.button t('spree.api.generate_key'), class: 'icon-key', icon: 'icon-key'
diff --git a/app/views/spree/users/_api_keys.html.haml b/app/views/spree/users/_api_keys.html.haml
new file mode 100644
index 0000000000..0644f1c79e
--- /dev/null
+++ b/app/views/spree/users/_api_keys.html.haml
@@ -0,0 +1,7 @@
+%hr
+%h3= t('.title')
+%br
+%p
+  = text_field_tag :api_key, @user.spree_api_key, disabled: true, class: 'title'
+= form_tag spree.api_keys_path(@user), method: :post, class: 'inline' do
+  = button_tag(t('.regenerate_key'), type: 'submit', class: "button primary")
diff --git a/app/views/spree/users/_developer_settings.html.haml b/app/views/spree/users/_developer_settings.html.haml
new file mode 100644
index 0000000000..b98ed527d5
--- /dev/null
+++ b/app/views/spree/users/_developer_settings.html.haml
@@ -0,0 +1,3 @@
+%script{ type: "text/ng-template", id: "account/developer_settings.html" }
+  %h3= t('.title')
+  = render partial: 'api_keys'
diff --git a/app/views/spree/users/show.html.haml b/app/views/spree/users/show.html.haml
index 6fa0d875e2..dcf1b3c4c0 100644
--- a/app/views/spree/users/show.html.haml
+++ b/app/views/spree/users/show.html.haml
@@ -20,6 +20,7 @@
     = render 'cards'
     = render 'transactions'
     = render 'settings'
+    = render 'developer_settings' if @user.show_api_key_view
 
   .row.tabset-ctrl#account-tabs{ style: 'margin-bottom: 100px', navigate: 'true', selected: 'orders', prefix: 'account' }
     .small.12.medium-3.columns.tab{ name: "orders" }
@@ -31,6 +32,11 @@
       %a=t('.tabs.transactions')
     .small.12.medium-3.columns.tab{ name: "settings" }
       %a=t('.tabs.settings')
+    // the api_keys partial is the only content for now, so we have to hide the whole tab for now
+    // if there is new content, we will need to handle this inside the developer_settings partial
+    - if @user.show_api_key_view
+      .small.12.medium-3.columns.tab{ name: "developer_settings" }
+        %a=t('.tabs.developer_settings')
     .small-12.columns.tab-view
 
   = render partial: "shared/footer"
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 959e95dc97..4b206dd59f 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -3638,6 +3638,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using
     # TODO: remove 'account_updated' key once we get to Spree 2.0
     account_updated: "Account updated!"
     email_updated: "The account will be updated once the new email is confirmed."
+    show_api_key_view_toggled: "Show API key view has been changed!"
     my_account: "My account"
     date: "Date"
     time: "Time"
@@ -4192,10 +4193,16 @@ See the %{link} to find out more about %{sitename}'s features and to start using
         connection_failed: "Could not connect to PayPal."
         generic_error: "PayPal failed. %{reasons}"
     users:
+      api_keys:
+        regenerate_key: "Regenerate Key"
+        title: API key
+      developer_settings:
+        title: Developer Settings
       form:
         account_settings: Account Settings
       show:
         tabs:
+          developer_settings: Developer Settings
           orders: Orders
           cards: Credit Cards
           transactions: Transactions
@@ -4256,6 +4263,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using
       shipment:
         cannot_ready: "Cannot ready shipment."
       invalid_taxonomy_id: "Invalid taxonomy id."
+      toggle_api_key_view: "Show API key view for user"
     activerecord:
       models:
         spree/payment:
diff --git a/config/routes/spree.rb b/config/routes/spree.rb
index eed705f5d6..2a02783eb5 100644
--- a/config/routes/spree.rb
+++ b/config/routes/spree.rb
@@ -18,6 +18,7 @@ Spree::Core::Engine.routes.draw do
              :path_names => { :sign_out => 'logout' },
              :path_prefix => :user
 
+  resources :api_keys, :only => [:create, :destroy]
   resources :users, :only => [:edit, :update]
 
   devise_scope :spree_user do
@@ -114,13 +115,6 @@ Spree::Core::Engine.routes.draw do
       end
     end
 
-    resources :users do
-      member do
-        put :generate_api_key
-        put :clear_api_key
-      end
-    end
-
     # Configuration section
     resource :general_settings
     resource :mail_methods, :only => [:edit, :update] do
diff --git a/db/migrate/20220713195433_add_show_api_key_generation_view_to_spree_user.rb b/db/migrate/20220713195433_add_show_api_key_generation_view_to_spree_user.rb
new file mode 100644
index 0000000000..d09f779c75
--- /dev/null
+++ b/db/migrate/20220713195433_add_show_api_key_generation_view_to_spree_user.rb
@@ -0,0 +1,5 @@
+class AddShowApiKeyGenerationViewToSpreeUser < ActiveRecord::Migration[6.1]
+  def change
+    add_column :spree_users, :show_api_key_view, :boolean, null: false, default: false
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 401e3906ff..3f8ddb08f3 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_29_080906) do
+ActiveRecord::Schema.define(version: 2022_07_13_195433) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -1058,6 +1058,7 @@ ActiveRecord::Schema.define(version: 2022_06_29_080906) do
     t.datetime "confirmation_sent_at"
     t.string "unconfirmed_email", limit: 255
     t.datetime "disabled_at"
+    t.boolean "show_api_key_view", default: false, null: false
     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/lib/open_food_network/spree_api_key_loader.rb b/lib/open_food_network/spree_api_key_loader.rb
index c3a532acbf..b5ed294ade 100644
--- a/lib/open_food_network/spree_api_key_loader.rb
+++ b/lib/open_food_network/spree_api_key_loader.rb
@@ -4,7 +4,11 @@ module OpenFoodNetwork
   module SpreeApiKeyLoader
     def load_spree_api_key
       if spree_current_user
-        spree_current_user.generate_spree_api_key! unless spree_current_user.spree_api_key
+        if spree_current_user.spree_api_key.blank?
+          spree_current_user.generate_api_key
+          spree_current_user.save
+        end
+
         @spree_api_key = spree_current_user.spree_api_key
       else
         @spree_api_key = nil
diff --git a/lib/spree/core/controller_helpers/auth.rb b/lib/spree/core/controller_helpers/auth.rb
index 98b1aa70b5..b309a46d4d 100644
--- a/lib/spree/core/controller_helpers/auth.rb
+++ b/lib/spree/core/controller_helpers/auth.rb
@@ -61,7 +61,8 @@ module Spree
 
           return unless user.respond_to?(:spree_api_key) && user.spree_api_key.blank?
 
-          user.generate_spree_api_key!
+          user.generate_api_key
+          user.save
         end
       end
     end
diff --git a/spec/controllers/spree/admin/users_controller_spec.rb b/spec/controllers/spree/admin/users_controller_spec.rb
index ca2cfad956..d2de374982 100644
--- a/spec/controllers/spree/admin/users_controller_spec.rb
+++ b/spec/controllers/spree/admin/users_controller_spec.rb
@@ -19,18 +19,10 @@ describe Spree::Admin::UsersController do
       expect(response).to render_template :index
     end
 
-    it "allows admins to update a user's API key" do
+    it "allows admins to update a user's show api key view" do
       user.spree_roles << Spree::Role.find_or_create_by(name: 'admin')
-      expect(test_user).to receive(:generate_spree_api_key!).and_return(true)
-      spree_put :generate_api_key, id: test_user.id
-      expect(response).to redirect_to(spree.edit_admin_user_path(test_user))
-    end
-
-    it "allows admins to clear a user's API key" do
-      user.spree_roles << Spree::Role.find_or_create_by(name: 'admin')
-      expect(test_user).to receive(:clear_spree_api_key!).and_return(true)
-      spree_put :clear_api_key, id: test_user.id
-      expect(response).to redirect_to(spree.edit_admin_user_path(test_user))
+      spree_put :update, id: test_user.id, user: { show_api_key_view: true }
+      expect(response).to render_template :edit
     end
 
     it 'should deny access to users without an admin role' do
diff --git a/spec/controllers/spree/api_keys_controller_spec.rb b/spec/controllers/spree/api_keys_controller_spec.rb
new file mode 100644
index 0000000000..68c5f35b5f
--- /dev/null
+++ b/spec/controllers/spree/api_keys_controller_spec.rb
@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Spree::ApiKeysController, type: :controller, performance: true do
+  routes { Spree::Core::Engine.routes }
+
+  include AuthenticationHelper
+  include ControllerRequestsHelper
+
+  let(:user) { create(:user) }
+  let(:redirect_path) { "#{spree.account_path}#/developer_settings" }
+
+  before do
+    allow(controller).to receive(:spree_current_user) { user }
+  end
+
+  describe "create" do
+    it "creates a new api key" do
+      expect { spree_post :create }.to change { user.reload.spree_api_key }
+      expect(user.spree_api_key).to be_present
+    end
+
+    it "redirects to the api keys tab on account page " do
+      spree_post :create
+      expect(response).to redirect_to redirect_path
+    end
+  end
+
+  describe "destroy" do
+    before do
+      user.generate_api_key
+      user.save
+    end
+
+    it "clears the api key" do
+      expect { spree_delete :destroy, id: user.id }.to change { user.reload.spree_api_key }.to(nil)
+    end
+
+    it "redirects to the api keys tab on account page " do
+      spree_delete :destroy, id: user.id
+      expect(response).to redirect_to redirect_path
+    end
+  end
+end
diff --git a/spec/requests/api/orders_spec.rb b/spec/requests/api/orders_spec.rb
index fc61c5c099..e0a0a5d9b7 100644
--- a/spec/requests/api/orders_spec.rb
+++ b/spec/requests/api/orders_spec.rb
@@ -57,7 +57,8 @@ describe 'api/v0/orders', swagger_doc: 'v0/swagger.yaml', type: :request do
 
           let(:user) { order_dist_1.distributor.owner }
           let(:'X-Spree-Token') do
-            user.generate_spree_api_key!
+            user.generate_api_key
+            user.save
             user.spree_api_key
           end
 
diff --git a/spec/system/admin/users_spec.rb b/spec/system/admin/users_spec.rb
index 8c558436bf..2b706846c8 100644
--- a/spec/system/admin/users_spec.rb
+++ b/spec/system/admin/users_spec.rb
@@ -104,6 +104,27 @@ describe "Managing users" do
           expect(page).to have_content("Account updated")
           expect(page).to have_unchecked_field "Disabled"
         end
+
+        it "should toggle the api key generation view" do
+          user = Spree::User.find_by(email: "a@example.com")
+
+          expect(page).to have_content "NO KEY"
+          find_button("Generate API key").click
+          expect(page).to have_content("Key generated")
+
+          expect(page).to have_unchecked_field "Show API key view for user"
+          expect(user.show_api_key_view).to be_falsey
+
+          check "Show API key view for user"
+          expect(page).to have_content("Show API key view has been changed!")
+          expect(page).to have_checked_field "Show API key view for user"
+          expect(user.reload.show_api_key_view).to be_truthy
+
+          uncheck "Show API key view for user"
+          expect(page).to have_content("Show API key view has been changed!")
+          expect(page).to have_unchecked_field "Show API key view for user"
+          expect(user.reload.show_api_key_view).to be_falsey
+        end
       end
     end
 
diff --git a/spec/system/consumer/account/developer_settings_spec.rb b/spec/system/consumer/account/developer_settings_spec.rb
new file mode 100644
index 0000000000..f7ba9eb33e
--- /dev/null
+++ b/spec/system/consumer/account/developer_settings_spec.rb
@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+require "system_helper"
+
+describe "Developer Settings", js: true do
+  include AuthenticationHelper
+  include WebHelper
+
+  describe "as a logged in user" do
+    before do
+      login_as user
+      visit "/account"
+    end
+
+    context "when show_api_key_view is true" do
+      let(:spree_api_key) { SecureRandom.hex(24) }
+      let(:user) { create(:user, show_api_key_view: true, spree_api_key: spree_api_key) }
+
+      it "shows the developer settings tab" do
+        find("a", text: "DEVELOPER SETTINGS").click
+        expect(page).to have_content "Developer Settings"
+      end
+
+      context "when the user has an api key" do
+        before do
+          find("a", text: "DEVELOPER SETTINGS").click
+        end
+
+        it "shows the api key" do
+          expect(page).to have_input "api_key", with: spree_api_key
+        end
+
+        it "lets the user regenerate the api key" do
+          click_button "Regenerate Key"
+          expect(page).to have_content "Key generated"
+          expect(page).to have_input "api_key", with: user.reload.spree_api_key
+        end
+      end
+    end
+
+    context "when show_api_key_view is false" do
+      let(:user) { create(:user, show_api_key_view: false) }
+
+      it "does not show the developer settings tab" do
+        within("#account-tabs") do
+          expect(page).to_not have_selector("a", text: "DEVELOPER SETTINGS")
+        end
+      end
+    end
+  end
+end