From a48fd0828c5e12c3d19225223fe1e355559bf003 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Wed, 15 Feb 2023 16:25:43 +1100 Subject: [PATCH 01/18] Add Voucher model A voucher belongs to an enterprise and an enterprise can have many vouchers --- app/models/enterprise.rb | 1 + app/models/voucher.rb | 7 +++++++ db/migrate/20230215034821_create_vouchers.rb | 12 ++++++++++++ db/schema.rb | 11 +++++++++++ spec/models/enterprise_spec.rb | 1 + spec/models/voucher_spec.rb | 18 ++++++++++++++++++ 6 files changed, 50 insertions(+) create mode 100644 app/models/voucher.rb create mode 100644 db/migrate/20230215034821_create_vouchers.rb create mode 100644 spec/models/voucher_spec.rb diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index 3d7d646c6c..a5d4fe6ff6 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -65,6 +65,7 @@ class Enterprise < ApplicationRecord has_many :inventory_items has_many :tag_rules has_one :stripe_account, dependent: :destroy + has_many :vouchers delegate :latitude, :longitude, :city, :state_name, to: :address diff --git a/app/models/voucher.rb b/app/models/voucher.rb new file mode 100644 index 0000000000..a36da2f218 --- /dev/null +++ b/app/models/voucher.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: false + +class Voucher < ApplicationRecord + belongs_to :enterprise + + validates :code, presence: true, uniqueness: { scope: :enterprise_id } +end diff --git a/db/migrate/20230215034821_create_vouchers.rb b/db/migrate/20230215034821_create_vouchers.rb new file mode 100644 index 0000000000..acd2e247a7 --- /dev/null +++ b/db/migrate/20230215034821_create_vouchers.rb @@ -0,0 +1,12 @@ +class CreateVouchers < ActiveRecord::Migration[6.1] + def change + create_table :vouchers do |t| + t.string :code, null: false, limit: 255 + t.datetime :expiry_date + + t.timestamps + end + add_reference :vouchers, :enterprise, foreign_key: true + add_index :vouchers, [:code, :enterprise_id], unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 75cccc9216..d125bec563 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1197,6 +1197,16 @@ ActiveRecord::Schema[7.0].define(version: 2023_02_13_160135) do t.index ["user_id"], name: "index_webhook_endpoints_on_user_id" end + create_table "vouchers", force: :cascade do |t| + t.string "code", limit: 255, null: false + t.datetime "expiry_date" + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.bigint "enterprise_id" + t.index ["code", "enterprise_id"], name: "index_vouchers_on_code_and_enterprise_id", unique: true + t.index ["enterprise_id"], name: "index_vouchers_on_enterprise_id" + end + add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id" add_foreign_key "active_storage_variant_records", "active_storage_blobs", column: "blob_id" add_foreign_key "adjustment_metadata", "enterprises", name: "adjustment_metadata_enterprise_id_fk" @@ -1302,4 +1312,5 @@ ActiveRecord::Schema[7.0].define(version: 2023_02_13_160135) do add_foreign_key "variant_overrides", "enterprises", column: "hub_id", name: "variant_overrides_hub_id_fk" add_foreign_key "variant_overrides", "spree_variants", column: "variant_id", name: "variant_overrides_variant_id_fk" add_foreign_key "webhook_endpoints", "spree_users", column: "user_id" + add_foreign_key "vouchers", "enterprises" end diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index f6290a8687..5600216689 100644 --- a/spec/models/enterprise_spec.rb +++ b/spec/models/enterprise_spec.rb @@ -24,6 +24,7 @@ describe Enterprise do it { is_expected.to have_many(:distributed_orders) } it { is_expected.to belong_to(:address) } it { is_expected.to belong_to(:business_address) } + it { is_expected.to have_many(:vouchers) } it "destroys enterprise roles upon its own demise" do e = create(:enterprise) diff --git a/spec/models/voucher_spec.rb b/spec/models/voucher_spec.rb new file mode 100644 index 0000000000..9d9590455f --- /dev/null +++ b/spec/models/voucher_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Voucher do + describe 'associations' do + it { is_expected.to belong_to(:enterprise) } + end + + describe 'validations' do + subject { Voucher.new(code: 'new_code', enterprise: enterprise) } + + let(:enterprise) { build(:enterprise) } + + it { is_expected.to validate_presence_of(:code) } + it { is_expected.to validate_uniqueness_of(:code).scoped_to(:enterprise_id) } + end +end From a62687b1a76a44344bf7eb55f81bf3003c62453e Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Wed, 15 Feb 2023 16:28:40 +1100 Subject: [PATCH 02/18] Fix warning : The implicit block expectation syntax is deprecated --- spec/models/enterprise_spec.rb | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index 5600216689..48f184e034 100644 --- a/spec/models/enterprise_spec.rb +++ b/spec/models/enterprise_spec.rb @@ -741,13 +741,9 @@ describe Enterprise do it "assigns permalink when initialized" do allow(Enterprise).to receive(:find_available_permalink).and_return("available_permalink") expect(Enterprise).to receive(:find_available_permalink).with("Name To Turn Into A Permalink") - expect( - lambda { enterprise.send(:initialize_permalink) } - ).to change{ - enterprise.permalink - }.to( - "available_permalink" - ) + expect do + enterprise.send(:initialize_permalink) + end.to change { enterprise.permalink }.to("available_permalink") end describe "finding a permalink" do From f9f6793d1013a925ef3d447f76e7af5f83049783 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Wed, 22 Feb 2023 11:13:58 +1100 Subject: [PATCH 03/18] Add template and basic controller for backoffice voucher pages --- app/controllers/admin/vouchers_controller.rb | 10 ++++++ app/helpers/admin/enterprises_helper.rb | 1 + .../enterprises/form/_vouchers.html.haml | 33 +++++++++++++++++++ app/views/admin/vouchers/new.html.haml | 22 +++++++++++++ app/views/spree/admin/shared/_tabs.html.haml | 2 +- config/locales/en.yml | 19 +++++++++++ config/routes/admin.rb | 3 ++ 7 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 app/controllers/admin/vouchers_controller.rb create mode 100644 app/views/admin/enterprises/form/_vouchers.html.haml create mode 100644 app/views/admin/vouchers/new.html.haml diff --git a/app/controllers/admin/vouchers_controller.rb b/app/controllers/admin/vouchers_controller.rb new file mode 100644 index 0000000000..594c0b28da --- /dev/null +++ b/app/controllers/admin/vouchers_controller.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module Admin + class VouchersController < ResourceController + + def new + @enterprise = Enterprise.find_by permalink: params[:enterprise_id] + end + end +end diff --git a/app/helpers/admin/enterprises_helper.rb b/app/helpers/admin/enterprises_helper.rb index e95f4589ed..42b532d56f 100644 --- a/app/helpers/admin/enterprises_helper.rb +++ b/app/helpers/admin/enterprises_helper.rb @@ -34,6 +34,7 @@ module Admin { name: 'shipping_methods', icon_class: "icon-truck", show: show_shipping_methods }, { name: 'payment_methods', icon_class: "icon-money", show: show_payment_methods }, { name: 'enterprise_fees', icon_class: "icon-tasks", show: show_enterprise_fees }, + { name: 'vouchers', icon_class: "icon-ticket", show: true }, { name: 'enterprise_permissions', icon_class: "icon-plug", show: true, href: admin_enterprise_relationships_path }, { name: 'inventory_settings', icon_class: "icon-list-ol", show: is_shop }, diff --git a/app/views/admin/enterprises/form/_vouchers.html.haml b/app/views/admin/enterprises/form/_vouchers.html.haml new file mode 100644 index 0000000000..843dc5e51a --- /dev/null +++ b/app/views/admin/enterprises/form/_vouchers.html.haml @@ -0,0 +1,33 @@ +.text-right + %a.button{ href: "#{new_admin_enterprise_voucher_path(@enterprise)}"} + = t('.add_new') +%br + +- if @enterprise.vouchers.present? + %table + %thead + %tr + %th= t('.voucher_code') + %th= t('.rate') + %th= t('.label') + %th= t('.purpose') + %th= t('.expiry') + %th= t('.use_limit') + %th= t('.customers') + %th= t('.net_value') + %tbody + - @enterprise.vouchers.each do |voucher| + %tr + %td= voucher.code + %td= voucher.rate + %td + %td + %td + %td + %td + %td + +- else + %p.text-center + = t('.no_voucher_yet') + diff --git a/app/views/admin/vouchers/new.html.haml b/app/views/admin/vouchers/new.html.haml new file mode 100644 index 0000000000..c62ed5acdc --- /dev/null +++ b/app/views/admin/vouchers/new.html.haml @@ -0,0 +1,22 @@ +.row + .sixteen.columns.alpha + .four.columns.alpha.text-right + %a.button{ href: "#{edit_admin_enterprise_path(@enterprise)}#!#vouchers_panel"} + = t('.back') + .twelve.columns.omega + .row + .eight.columns.text-center + %legend= t(".legend") + .four.columns.text-right + %input.red{ type: "button", value: t('.save') } + .row + .alpha.four.columns + = label :voucher, :code, t('.voucher_code') + .omega.eight.columns + %textarea.fullwidth{ id: 'voucher_code', name: 'voucher_code', rows: 6 } + .row + .alpha.four.columns + = label :voucher, :amount, t('.voucher_amount') + .omega.eight.columns + $ + %input{ type: 'text', id: 'voucher_amount', name: 'voucher_amount', value: 10, disabled: true } diff --git a/app/views/spree/admin/shared/_tabs.html.haml b/app/views/spree/admin/shared/_tabs.html.haml index 6613a24643..91efb77da3 100644 --- a/app/views/spree/admin/shared/_tabs.html.haml +++ b/app/views/spree/admin/shared/_tabs.html.haml @@ -4,7 +4,7 @@ = tab :orders, :subscriptions, :customer_details, :adjustments, :payments, :return_authorizations, url: admin_orders_path('q[s]' => 'completed_at desc'), icon: 'icon-shopping-cart' = tab :reports, url: main_app.admin_reports_path, icon: 'icon-file' = tab :general_settings, :mail_methods, :tax_categories, :tax_rates, :tax_settings, :zones, :countries, :states, :payment_methods, :taxonomies, :shipping_methods, :shipping_categories, :enterprise_fees, :contents, :invoice_settings, :matomo_settings, :stripe_connect_settings, label: 'configuration', icon: 'icon-wrench', url: edit_admin_general_settings_path -= tab :enterprises, :enterprise_relationships, :oidc_settings, url: main_app.admin_enterprises_path += tab :enterprises, :enterprise_relationships, :vouchers, :oidc_settings, url: main_app.admin_enterprises_path = tab :customers, url: main_app.admin_customers_path = tab :enterprise_groups, url: main_app.admin_enterprise_groups_path, label: 'groups' - if can? :admin, Spree::User diff --git a/config/locales/en.yml b/config/locales/en.yml index 5a9001e7a4..75e2de9367 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1144,6 +1144,17 @@ en: add_unregistered_user: "Add an unregistered user" email_confirmed: "Email confirmed" email_not_confirmed: "Email not confirmed" + vouchers: + legend: Vouchers + rate: Rate + label: Label + purpose: Purpose + expiry: Expiry + use_limit: Use/Limit + customers: Customer + net_value: Net Value + add_new: Add New + no_voucher_yet: No Vouchers yet actions: edit_profile: Settings properties: Properties @@ -1382,6 +1393,7 @@ en: tag_rules: "Tag Rules" shop_preferences: "Shop Preferences" users: "Users" + vouchers: Vouchers enterprise_group: primary_details: "Primary Details" users: "Users" @@ -1590,6 +1602,13 @@ en: schedules: destroy: associated_subscriptions_error: This schedule cannot be deleted because it has associated subscriptions + vouchers: + new: + legend: New Voucher + back: Back + save: Save + voucher_code: Voucher Code + voucher_amount: Amount # Admin controllers controllers: diff --git a/config/routes/admin.rb b/config/routes/admin.rb index b28f1eae57..852c7a3436 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -40,6 +40,9 @@ Openfoodnetwork::Application.routes.draw do end resources :tag_rules, only: [:destroy] + + # TODO do we need to remove more routes + resources :vouchers, only: [:new, :create] end resources :enterprise_relationships From a4add889a873c0ab974c79d0c796d41384d64267 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Wed, 22 Feb 2023 12:06:13 +1100 Subject: [PATCH 04/18] Add form on the new voucher page --- app/controllers/admin/vouchers_controller.rb | 1 + app/views/admin/vouchers/new.html.haml | 45 ++++++++++---------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/app/controllers/admin/vouchers_controller.rb b/app/controllers/admin/vouchers_controller.rb index 594c0b28da..ae25f0bede 100644 --- a/app/controllers/admin/vouchers_controller.rb +++ b/app/controllers/admin/vouchers_controller.rb @@ -5,6 +5,7 @@ module Admin def new @enterprise = Enterprise.find_by permalink: params[:enterprise_id] + @voucher = Voucher.new end end end diff --git a/app/views/admin/vouchers/new.html.haml b/app/views/admin/vouchers/new.html.haml index c62ed5acdc..f35befdd67 100644 --- a/app/views/admin/vouchers/new.html.haml +++ b/app/views/admin/vouchers/new.html.haml @@ -1,22 +1,23 @@ -.row - .sixteen.columns.alpha - .four.columns.alpha.text-right - %a.button{ href: "#{edit_admin_enterprise_path(@enterprise)}#!#vouchers_panel"} - = t('.back') - .twelve.columns.omega - .row - .eight.columns.text-center - %legend= t(".legend") - .four.columns.text-right - %input.red{ type: "button", value: t('.save') } - .row - .alpha.four.columns - = label :voucher, :code, t('.voucher_code') - .omega.eight.columns - %textarea.fullwidth{ id: 'voucher_code', name: 'voucher_code', rows: 6 } - .row - .alpha.four.columns - = label :voucher, :amount, t('.voucher_amount') - .omega.eight.columns - $ - %input{ type: 'text', id: 'voucher_amount', name: 'voucher_amount', value: 10, disabled: true } += form_with model: @voucher, url: admin_enterprise_vouchers_path(@enterprise), html: { name: "voucher_form" } do |f| + .row + .sixteen.columns.alpha + .four.columns.alpha.text-right + %a.button{ href: "#{edit_admin_enterprise_path(@enterprise)}#!#vouchers_panel"} + = t('.back') + .twelve.columns.omega + .row + .eight.columns.text-center + %legend= t(".legend") + .four.columns.text-right + = f.submit t('.save'), class: 'red' + .row + .alpha.four.columns + = f.label :code, t('.voucher_code') + .omega.eight.columns + = f.text_area :code, rows: 6, class: 'fullwidth' + .row + .alpha.four.columns + = f.label :amount, t('.voucher_amount') + .omega.eight.columns + $ + = f.text_field :amount, value: 10, disabled: true From 4085aa22dcf67bb7ddc9c3dff7f2f28c2f4c1030 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Wed, 22 Feb 2023 14:50:35 +1100 Subject: [PATCH 05/18] Add create voucher action and system test --- app/controllers/admin/vouchers_controller.rb | 33 ++++++++-- spec/system/admin/vouchers_spec.rb | 68 ++++++++++++++++++++ 2 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 spec/system/admin/vouchers_spec.rb diff --git a/app/controllers/admin/vouchers_controller.rb b/app/controllers/admin/vouchers_controller.rb index ae25f0bede..70bf0c75a1 100644 --- a/app/controllers/admin/vouchers_controller.rb +++ b/app/controllers/admin/vouchers_controller.rb @@ -2,10 +2,35 @@ module Admin class VouchersController < ResourceController + before_action :load_enterprise - def new - @enterprise = Enterprise.find_by permalink: params[:enterprise_id] - @voucher = Voucher.new - end + def new + @voucher = Voucher.new + end + + def create + voucher_params = permitted_resource_params.merge(enterprise: @enterprise) + @voucher = Voucher.create(voucher_params) + + if @voucher.save + redirect_to( + "#{edit_admin_enterprise_path(@enterprise)}#vouchers_panel", + flash: { success: flash_message_for(@voucher, :successfully_created) } + ) + else + flash[:error] = @voucher.errors.full_messages.to_sentence + render :new + end + end + + private + + def load_enterprise + @enterprise = Enterprise.find_by permalink: params[:enterprise_id] + end + + def permitted_resource_params + params.require(:voucher).permit(:code) + end end end diff --git a/spec/system/admin/vouchers_spec.rb b/spec/system/admin/vouchers_spec.rb new file mode 100644 index 0000000000..24cba2c420 --- /dev/null +++ b/spec/system/admin/vouchers_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'system_helper' + +describe ' + As an administrator + I want to manage vouchers +' do + include WebHelper + include AuthenticationHelper + + let(:enterprise) { create(:supplier_enterprise, name: 'Feedme') } + let(:voucher_code) { 'awesomevoucher' } + + it 'lists enterprise vouchers' do + # Given an enterprise with vouchers + Voucher.create!(enterprise: enterprise, code: voucher_code) + + # When I go to the enterprise voucher tab + login_as_admin_and_visit edit_admin_enterprise_path(enterprise) + click_link 'Vouchers' + + # Then I see a list of vouchers + expect(page).to have_content voucher_code + expect(page).to have_content "10" + end + + it 'creates a voucher' do + # Given an enterprise + # When I go to the new voucher page + login_as_admin_and_visit new_admin_enterprise_voucher_path(enterprise) + + # And I fill in the fields for a new voucher click save + fill_in 'voucher_code', with: voucher_code + click_button 'Save' + + # Then I should get redirect to the entreprise voucher tab and see the created voucher + expect(page).to have_selector '.success', text: 'Voucher has been successfully created!' + + # TODO: doesn't automatically show the voucher tab + click_link 'Vouchers' + + expect(page).to have_content voucher_code + expect(page).to have_content "10" + + voucher = Voucher.where(enterprise: enterprise, code: voucher_code).first + + expect(voucher).not_to be(nil) + end + + context 'when entering invalid data' do + it 'shows an error flash message' do + # Given an enterprise + # When I go to the new voucher page + login_as_admin_and_visit new_admin_enterprise_voucher_path(enterprise) + + # And I fill in filers with invalid data click save + click_button 'Save' + + # Then I should see an error flash message + expect(page).to have_selector '.error', text: "Code can't be blank" + + vouchers = Voucher.where(enterprise: enterprise) + + expect(vouchers).to be_empty + end + end +end From e4f40d14b8dea6605f83ac8bd67a61c39b6e5302 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Wed, 22 Feb 2023 14:51:03 +1100 Subject: [PATCH 06/18] Fix enterprise voucher tab Add harcoded voucher amount Add missing translation --- app/views/admin/enterprises/form/_vouchers.html.haml | 2 +- config/locales/en.yml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/views/admin/enterprises/form/_vouchers.html.haml b/app/views/admin/enterprises/form/_vouchers.html.haml index 843dc5e51a..608ae5dd56 100644 --- a/app/views/admin/enterprises/form/_vouchers.html.haml +++ b/app/views/admin/enterprises/form/_vouchers.html.haml @@ -19,7 +19,7 @@ - @enterprise.vouchers.each do |voucher| %tr %td= voucher.code - %td= voucher.rate + %td $10 %td %td %td diff --git a/config/locales/en.yml b/config/locales/en.yml index 75e2de9367..9f54f858b6 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1146,6 +1146,7 @@ en: email_not_confirmed: "Email not confirmed" vouchers: legend: Vouchers + voucher_code: Voucher Code rate: Rate label: Label purpose: Purpose From 094fc039e9f29f5df893dab3f053a3cce46aac8a Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 7 Mar 2023 10:01:35 +1100 Subject: [PATCH 07/18] Update tabs_and_panels to display tab and panel based on the url anchor For Vouchers, this means the voucher tab and panel are displayed when you come back to entreprise edit screen from the new vourcher page --- .../controllers/tabs_and_panels_controller.js | 35 +++++- .../tabs_and_panels_controller_test.js | 102 ++++++++++++++---- 2 files changed, 116 insertions(+), 21 deletions(-) diff --git a/app/webpacker/controllers/tabs_and_panels_controller.js b/app/webpacker/controllers/tabs_and_panels_controller.js index 0a1762cedb..b6a119c3e8 100644 --- a/app/webpacker/controllers/tabs_and_panels_controller.js +++ b/app/webpacker/controllers/tabs_and_panels_controller.js @@ -12,13 +12,34 @@ export default class extends Controller { // only display the default panel this.defaultTarget.style.display = "block"; + + // Display panel specified in url anchor + const anchors = window.location.toString().split("#"); + const anchor = anchors.length > 1 ? anchors.pop() : ""; + + if (anchor != "") { + this.updateActivePanel(anchor); + + // tab + const tab_id = anchor.split("_panel").shift(); + this.updateActiveTab(tab_id); + } } changeActivePanel(event) { + this.updateActivePanel(`${event.currentTarget.id}_panel`); + } + + updateActivePanel(panel_id) { const newActivePanel = this.panelTargets.find( - (panel) => panel.id == `${event.currentTarget.id}_panel` + (panel) => panel.id == panel_id ); + if (newActivePanel === undefined) { + // No panel found + return; + } + this.currentActivePanel.style.display = "none"; newActivePanel.style.display = "block"; } @@ -28,6 +49,18 @@ export default class extends Controller { event.currentTarget.classList.add(`${this.classNameValue}`); } + updateActiveTab(tab_id) { + const newActiveTab = this.tabTargets.find((tab) => tab.id == tab_id); + + if (newActiveTab === undefined) { + // No tab found + return; + } + + this.currentActiveTab.classList.remove(`${this.classNameValue}`); + newActiveTab.classList.add(`${this.classNameValue}`); + } + get currentActiveTab() { return this.tabTargets.find((tab) => tab.classList.contains("selected")); } diff --git a/spec/javascripts/stimulus/tabs_and_panels_controller_test.js b/spec/javascripts/stimulus/tabs_and_panels_controller_test.js index 8fdfedd117..c87c08f6d0 100644 --- a/spec/javascripts/stimulus/tabs_and_panels_controller_test.js +++ b/spec/javascripts/stimulus/tabs_and_panels_controller_test.js @@ -2,16 +2,26 @@ * @jest-environment jsdom */ -import { Application } from "stimulus"; -import tabs_and_panels_controller from "../../../app/webpacker/controllers/tabs_and_panels_controller"; +import { Application } from 'stimulus'; +import tabs_and_panels_controller from '../../../app/webpacker/controllers/tabs_and_panels_controller'; -describe("EnterprisePanelController", () => { +describe('EnterprisePanelController', () => { beforeAll(() => { const application = Application.start(); - application.register("tabs-and-panels", tabs_and_panels_controller); + application.register('tabs-and-panels', tabs_and_panels_controller); }); - describe("#tabs-and-panels", () => { + describe('#tabs-and-panels', () => { + const checkDefaultPanel = () => { + const peekPanel = document.getElementById('peek_panel'); + const kaPanel = document.getElementById('ka_panel'); + const booPanel = document.getElementById('boo_panel'); + + expect(peekPanel.style.display).toBe('block'); + expect(kaPanel.style.display).toBe('none'); + expect(booPanel.style.display).toBe('none'); + } + beforeEach(() => { document.body.innerHTML = `
@@ -26,23 +36,75 @@ describe("EnterprisePanelController", () => {
`; }); - it("displays only the default panel", () => { - const peekPanel = document.getElementById("peek_panel"); - const kaPanel = document.getElementById("ka_panel"); - const booPanel = document.getElementById("boo_panel"); - - expect(peekPanel.style.display).toBe("block"); - expect(kaPanel.style.display).toBe("none"); - expect(booPanel.style.display).toBe("none"); + it('displays only the default panel', () => { + checkDefaultPanel() }); - it("displays appropriate panel when associated tab is clicked", () => { - const kaPanel = document.getElementById("ka_panel"); - const ka = document.getElementById("ka"); + describe('when tab is clicked', () => { + let ka; - expect(kaPanel.style.display).toBe("none"); - ka.click(); - expect(kaPanel.style.display).toBe("block"); - }); + beforeEach(() => { + ka = document.getElementById('ka'); + }) + + it('displays appropriate panel', () => { + const kaPanel = document.getElementById('ka_panel'); + + expect(kaPanel.style.display).toBe('none'); + ka.click(); + expect(kaPanel.style.display).toBe('block'); + }); + + it('selects the clicked tab', () => { + ka.click(); + expect(ka.classList.contains('selected')).toBe(true); + }); + }) + + describe('when anchor is specified in the url', () => { + const { location } = window; + const mockLocationToString = (panel) => { + // Mocking window.location.toString() + const url = `http://localhost:3000/admin/enterprises/great-shop/edit#!#${panel}` + const mockedToString = jest.fn() + mockedToString.mockImplementation(() => (url)) + + delete window.location + window.location = { + toString: mockedToString + } + } + + beforeAll(() => { + mockLocationToString('ka_panel') + }) + + afterAll(() => { + // cleaning up + window.location = location + }) + + it('displays the panel associated with the anchor', () => { + const kaPanel = document.getElementById('ka_panel'); + + expect(kaPanel.style.display).toBe('block'); + }) + + it('selects the tab entry associated with the anchor', () => { + const ka = document.getElementById('ka'); + + expect(ka.classList.contains('selected')).toBe(true); + }) + + describe("when anchor doesn't macht any panel", () => { + beforeAll(() => { + mockLocationToString('random_panel') + }) + + it('displays the default panel', () => { + checkDefaultPanel() + }) + }) + }) }); }); From 04748b6e0e7c5cd20274405e880cbd4e7e4daebf Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 7 Mar 2023 10:22:41 +1100 Subject: [PATCH 08/18] Move voucher value to the model Use Spree::Money to display amount with currency --- app/models/voucher.rb | 8 ++++++++ app/views/admin/enterprises/form/_vouchers.html.haml | 2 +- app/views/admin/vouchers/new.html.haml | 4 ++-- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/app/models/voucher.rb b/app/models/voucher.rb index a36da2f218..28c74ca7de 100644 --- a/app/models/voucher.rb +++ b/app/models/voucher.rb @@ -4,4 +4,12 @@ class Voucher < ApplicationRecord belongs_to :enterprise validates :code, presence: true, uniqueness: { scope: :enterprise_id } + + def value + 10 + end + + def display_value + Spree::Money.new(value) + end end diff --git a/app/views/admin/enterprises/form/_vouchers.html.haml b/app/views/admin/enterprises/form/_vouchers.html.haml index 608ae5dd56..66492b0b50 100644 --- a/app/views/admin/enterprises/form/_vouchers.html.haml +++ b/app/views/admin/enterprises/form/_vouchers.html.haml @@ -19,7 +19,7 @@ - @enterprise.vouchers.each do |voucher| %tr %td= voucher.code - %td $10 + %td= voucher.display_value %td %td %td diff --git a/app/views/admin/vouchers/new.html.haml b/app/views/admin/vouchers/new.html.haml index f35befdd67..806d49856b 100644 --- a/app/views/admin/vouchers/new.html.haml +++ b/app/views/admin/vouchers/new.html.haml @@ -19,5 +19,5 @@ .alpha.four.columns = f.label :amount, t('.voucher_amount') .omega.eight.columns - $ - = f.text_field :amount, value: 10, disabled: true + = Spree::Money.currency_symbol + = f.text_field :amount, value: @voucher.value, disabled: true From fb1ad4c65f5caadeb0c0bb58bf871107eb9a6029 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 7 Mar 2023 10:24:49 +1100 Subject: [PATCH 09/18] Remove non needed collumn from voucher list --- .../enterprises/form/_vouchers.html.haml | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/app/views/admin/enterprises/form/_vouchers.html.haml b/app/views/admin/enterprises/form/_vouchers.html.haml index 66492b0b50..8df72f951c 100644 --- a/app/views/admin/enterprises/form/_vouchers.html.haml +++ b/app/views/admin/enterprises/form/_vouchers.html.haml @@ -9,23 +9,23 @@ %tr %th= t('.voucher_code') %th= t('.rate') - %th= t('.label') - %th= t('.purpose') - %th= t('.expiry') - %th= t('.use_limit') - %th= t('.customers') - %th= t('.net_value') + /%th= t('.label') + /%th= t('.purpose') + /%th= t('.expiry') + /%th= t('.use_limit') + /%th= t('.customers') + /%th= t('.net_value') %tbody - @enterprise.vouchers.each do |voucher| %tr %td= voucher.code %td= voucher.display_value - %td - %td - %td - %td - %td - %td + /%td + /%td + /%td + /%td + /%td + /%td - else %p.text-center From 0c43d0f16a7309d58185d2f5d8ce8f9181583ab3 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 7 Mar 2023 13:54:06 +1100 Subject: [PATCH 10/18] Put vouchers admin screen behind 'vouchers' feature toggle --- app/views/admin/enterprises/_form.html.haml | 6 ++++++ app/views/admin/shared/_side_menu.html.haml | 2 +- config/routes/admin.rb | 5 +++-- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/views/admin/enterprises/_form.html.haml b/app/views/admin/enterprises/_form.html.haml index dcaae1a149..87aeaa2d29 100644 --- a/app/views/admin/enterprises/_form.html.haml +++ b/app/views/admin/enterprises/_form.html.haml @@ -9,6 +9,12 @@ %fieldset.alpha.no-border-bottom{ id: "#{item[:name]}_panel", data: { "tabs-and-panels-target": "panel" }} %legend= t(".#{ item[:name] }.legend") + - when 'vouchers' + - if feature?(:vouchers, spree_current_user) + %fieldset.alpha.no-border-bottom{ id: "#{item[:name]}_panel", data: { "tabs-and-panels-target": "panel" }} + %legend= t(".#{ item[:form_name] || item[:name] }.legend") + = render "admin/enterprises/form/#{ item[:form_name] || item[:name] }", f: f + - else %fieldset.alpha.no-border-bottom{ id: "#{item[:name]}_panel", data: { "tabs-and-panels-target": "panel" }} %legend= t(".#{ item[:form_name] || item[:name] }.legend") diff --git a/app/views/admin/shared/_side_menu.html.haml b/app/views/admin/shared/_side_menu.html.haml index 18c9e4a04b..f9f38610bd 100644 --- a/app/views/admin/shared/_side_menu.html.haml +++ b/app/views/admin/shared/_side_menu.html.haml @@ -1,7 +1,7 @@ .side_menu#side_menu - if @enterprise - enterprise_side_menu_items(@enterprise).each do |item| - - next unless item[:show] + - next if !item[:show] || (item[:name] == 'vouchers' && !feature?(:vouchers, spree_current_user)) %a.menu_item{ href: item[:href] || "##{item[:name]}_panel", id: item[:name], data: { action: "tabs-and-panels#changeActivePanel tabs-and-panels#changeActiveTab", "tabs-and-panels-target": "tab" }, class: item[:selected] } %i{ class: item[:icon_class] } %span= t(".enterprise.#{item[:name] }") diff --git a/config/routes/admin.rb b/config/routes/admin.rb index 852c7a3436..69e2186cd0 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -41,8 +41,9 @@ Openfoodnetwork::Application.routes.draw do resources :tag_rules, only: [:destroy] - # TODO do we need to remove more routes - resources :vouchers, only: [:new, :create] + constraints FeatureToggleConstraint.new(:vouchers) do + resources :vouchers, only: [:new, :create] + end end resources :enterprise_relationships From 7dc5bc87d1ec9ff9185a77aedd158c80aeca87c3 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 7 Mar 2023 16:27:58 +1100 Subject: [PATCH 11/18] Fix rubocop warnings --- app/helpers/admin/enterprises_helper.rb | 4 +++- spec/models/enterprise_spec.rb | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/helpers/admin/enterprises_helper.rb b/app/helpers/admin/enterprises_helper.rb index 42b532d56f..25dae316f8 100644 --- a/app/helpers/admin/enterprises_helper.rb +++ b/app/helpers/admin/enterprises_helper.rb @@ -14,6 +14,7 @@ module Admin producers.size == 1 ? producers.first.id : nil end + # rubocop:disable Metrics/MethodLength def enterprise_side_menu_items(enterprise) is_shop = enterprise.sells != "none" show_properties = !!enterprise.is_primary_producer @@ -34,7 +35,7 @@ module Admin { name: 'shipping_methods', icon_class: "icon-truck", show: show_shipping_methods }, { name: 'payment_methods', icon_class: "icon-money", show: show_payment_methods }, { name: 'enterprise_fees', icon_class: "icon-tasks", show: show_enterprise_fees }, - { name: 'vouchers', icon_class: "icon-ticket", show: true }, + { name: 'vouchers', icon_class: "icon-ticket", show: true }, { name: 'enterprise_permissions', icon_class: "icon-plug", show: true, href: admin_enterprise_relationships_path }, { name: 'inventory_settings', icon_class: "icon-list-ol", show: is_shop }, @@ -43,5 +44,6 @@ module Admin { name: 'users', icon_class: "icon-user", show: true } ] end + # rubocop:enable Metrics/MethodLength end end diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index 48f184e034..67e1ac2284 100644 --- a/spec/models/enterprise_spec.rb +++ b/spec/models/enterprise_spec.rb @@ -742,7 +742,7 @@ describe Enterprise do allow(Enterprise).to receive(:find_available_permalink).and_return("available_permalink") expect(Enterprise).to receive(:find_available_permalink).with("Name To Turn Into A Permalink") expect do - enterprise.send(:initialize_permalink) + enterprise.send(:initialize_permalink) end.to change { enterprise.permalink }.to("available_permalink") end From 3b7aebd6da1a134d311b552eee9c2db6df9db671 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Wed, 8 Mar 2023 10:40:33 +1100 Subject: [PATCH 12/18] Enable vouchers feature toggle when running vouchers specs --- spec/system/admin/vouchers_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/system/admin/vouchers_spec.rb b/spec/system/admin/vouchers_spec.rb index 24cba2c420..65c3229ecd 100644 --- a/spec/system/admin/vouchers_spec.rb +++ b/spec/system/admin/vouchers_spec.rb @@ -12,6 +12,10 @@ describe ' let(:enterprise) { create(:supplier_enterprise, name: 'Feedme') } let(:voucher_code) { 'awesomevoucher' } + before do + Flipper.enable(:vouchers) + end + it 'lists enterprise vouchers' do # Given an enterprise with vouchers Voucher.create!(enterprise: enterprise, code: voucher_code) From 6809198c9658314285d679496c619e3980a17ebd Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Wed, 8 Mar 2023 10:42:39 +1100 Subject: [PATCH 13/18] Vouchers specs, remove obsolete step Vouchers tabs now shows when returning from new voucher page, no need to manually click on the 'vouchers' link in the menu --- spec/system/admin/vouchers_spec.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/spec/system/admin/vouchers_spec.rb b/spec/system/admin/vouchers_spec.rb index 65c3229ecd..baefa891fa 100644 --- a/spec/system/admin/vouchers_spec.rb +++ b/spec/system/admin/vouchers_spec.rb @@ -40,10 +40,6 @@ describe ' # Then I should get redirect to the entreprise voucher tab and see the created voucher expect(page).to have_selector '.success', text: 'Voucher has been successfully created!' - - # TODO: doesn't automatically show the voucher tab - click_link 'Vouchers' - expect(page).to have_content voucher_code expect(page).to have_content "10" @@ -58,7 +54,7 @@ describe ' # When I go to the new voucher page login_as_admin_and_visit new_admin_enterprise_voucher_path(enterprise) - # And I fill in filers with invalid data click save + # And I fill in fields with invalid data and click save click_button 'Save' # Then I should see an error flash message From 6eb52aa5400e7e31e9b62ee8cb56b1dd4a4e7f2e Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Mon, 20 Mar 2023 10:37:46 +1100 Subject: [PATCH 14/18] TabsAndPanelsController specs, add missinrg panel scenario --- .../tabs_and_panels_controller_test.js | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/spec/javascripts/stimulus/tabs_and_panels_controller_test.js b/spec/javascripts/stimulus/tabs_and_panels_controller_test.js index c87c08f6d0..8b42a528dd 100644 --- a/spec/javascripts/stimulus/tabs_and_panels_controller_test.js +++ b/spec/javascripts/stimulus/tabs_and_panels_controller_test.js @@ -5,7 +5,7 @@ import { Application } from 'stimulus'; import tabs_and_panels_controller from '../../../app/webpacker/controllers/tabs_and_panels_controller'; -describe('EnterprisePanelController', () => { +describe('TabsAndPanelsController', () => { beforeAll(() => { const application = Application.start(); application.register('tabs-and-panels', tabs_and_panels_controller); @@ -59,6 +59,28 @@ describe('EnterprisePanelController', () => { ka.click(); expect(ka.classList.contains('selected')).toBe(true); }); + + describe("when panel doesn't exist", () => { + beforeEach(() => { + document.body.innerHTML = ` +
+ Peek + Ka + Boo + + +
Peek me
+
Boo three
+
`; + }); + + it('displays the current panel', () => { + const peekPanel = document.getElementById('peek_panel'); + + ka.click(); + expect(peekPanel.style.display).toBe('block'); + }) + }) }) describe('when anchor is specified in the url', () => { From 37d3c025e91f1957513db7ad92214bec7b84fb2a Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou <40413322+rioug@users.noreply.github.com> Date: Mon, 20 Mar 2023 11:03:15 +1100 Subject: [PATCH 15/18] Make sure New voucher page can be accessed from voucher panel Co-authored-by: David Cook --- spec/system/admin/vouchers_spec.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/spec/system/admin/vouchers_spec.rb b/spec/system/admin/vouchers_spec.rb index baefa891fa..227d23a05f 100644 --- a/spec/system/admin/vouchers_spec.rb +++ b/spec/system/admin/vouchers_spec.rb @@ -31,8 +31,12 @@ describe ' it 'creates a voucher' do # Given an enterprise - # When I go to the new voucher page - login_as_admin_and_visit new_admin_enterprise_voucher_path(enterprise) + # When I go to the enterprise voucher tab and click new + login_as_admin_and_visit edit_admin_enterprise_path(enterprise) + click_link 'Vouchers' + within "#vouchers_panel" do + click_link 'Add New' + end # And I fill in the fields for a new voucher click save fill_in 'voucher_code', with: voucher_code From 2e10336a47284db8d37e3de36c31cd25529a771a Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Fri, 24 Mar 2023 11:38:59 +1100 Subject: [PATCH 16/18] Add ability to manage voucher for enterprise user Update spec accordingly --- app/models/spree/ability.rb | 2 ++ spec/models/spree/ability_spec.rb | 4 ++++ spec/system/admin/vouchers_spec.rb | 14 ++++++++++---- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/app/models/spree/ability.rb b/app/models/spree/ability.rb index cfa3fa0f68..dee02528e4 100644 --- a/app/models/spree/ability.rb +++ b/app/models/spree/ability.rb @@ -179,6 +179,8 @@ module Spree can [:admin, :create], :manager_invitation can [:admin, :index], :oidc_setting + + can [:admin, :create], Voucher end def add_product_management_abilities(user) diff --git a/spec/models/spree/ability_spec.rb b/spec/models/spree/ability_spec.rb index be904d6bce..68b45fdbcd 100644 --- a/spec/models/spree/ability_spec.rb +++ b/spec/models/spree/ability_spec.rb @@ -788,6 +788,10 @@ describe Spree::Ability do is_expected.to have_ability([:admin, :known_users, :customers], for: :search) is_expected.not_to have_ability([:users], for: :search) end + + it "has the ability to manage vouchers" do + is_expected.to have_ability([:admin, :create], for: Voucher) + end end context 'enterprise owner' do diff --git a/spec/system/admin/vouchers_spec.rb b/spec/system/admin/vouchers_spec.rb index 227d23a05f..7b18eec578 100644 --- a/spec/system/admin/vouchers_spec.rb +++ b/spec/system/admin/vouchers_spec.rb @@ -3,7 +3,7 @@ require 'system_helper' describe ' - As an administrator + As an entreprise user I want to manage vouchers ' do include WebHelper @@ -11,9 +11,13 @@ describe ' let(:enterprise) { create(:supplier_enterprise, name: 'Feedme') } let(:voucher_code) { 'awesomevoucher' } + let(:enterprise_user) { create(:user, enterprise_limit: 1) } before do Flipper.enable(:vouchers) + + enterprise_user.enterprise_roles.build(enterprise: enterprise).save + login_as enterprise_user end it 'lists enterprise vouchers' do @@ -21,7 +25,8 @@ describe ' Voucher.create!(enterprise: enterprise, code: voucher_code) # When I go to the enterprise voucher tab - login_as_admin_and_visit edit_admin_enterprise_path(enterprise) + visit edit_admin_enterprise_path(enterprise) + click_link 'Vouchers' # Then I see a list of vouchers @@ -32,7 +37,8 @@ describe ' it 'creates a voucher' do # Given an enterprise # When I go to the enterprise voucher tab and click new - login_as_admin_and_visit edit_admin_enterprise_path(enterprise) + visit edit_admin_enterprise_path(enterprise) + click_link 'Vouchers' within "#vouchers_panel" do click_link 'Add New' @@ -56,7 +62,7 @@ describe ' it 'shows an error flash message' do # Given an enterprise # When I go to the new voucher page - login_as_admin_and_visit new_admin_enterprise_voucher_path(enterprise) + visit new_admin_enterprise_voucher_path(enterprise) # And I fill in fields with invalid data and click save click_button 'Save' From e1845dddac08c577ed5bbefa1014e5c1bc4a9aca Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Fri, 24 Mar 2023 14:28:29 +1100 Subject: [PATCH 17/18] Fix TabsAndPanelsController now that #! are removed from url This PR https://github.com/openfoodfoundation/openfoodnetwork/pull/9729 remove #! from url. But unfortunately, AngularJs rewrite "example.com#panel" as "example.com#/panel" thus breaking the original implementation. --- app/webpacker/controllers/tabs_and_panels_controller.js | 8 +++++++- .../stimulus/tabs_and_panels_controller_test.js | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/webpacker/controllers/tabs_and_panels_controller.js b/app/webpacker/controllers/tabs_and_panels_controller.js index b6a119c3e8..e9b208fae7 100644 --- a/app/webpacker/controllers/tabs_and_panels_controller.js +++ b/app/webpacker/controllers/tabs_and_panels_controller.js @@ -15,9 +15,15 @@ export default class extends Controller { // Display panel specified in url anchor const anchors = window.location.toString().split("#"); - const anchor = anchors.length > 1 ? anchors.pop() : ""; + let anchor = anchors.length > 1 ? anchors.pop() : ""; if (anchor != "") { + // Conveniently AngularJs rewrite "example.com#panel" to "example.com#/panel" :( + // strip the starting / if any + if (anchor[0] == "/") { + anchor = anchor.slice(1); + } + this.updateActivePanel(anchor); // tab diff --git a/spec/javascripts/stimulus/tabs_and_panels_controller_test.js b/spec/javascripts/stimulus/tabs_and_panels_controller_test.js index 8b42a528dd..523f5eb04f 100644 --- a/spec/javascripts/stimulus/tabs_and_panels_controller_test.js +++ b/spec/javascripts/stimulus/tabs_and_panels_controller_test.js @@ -87,7 +87,7 @@ describe('TabsAndPanelsController', () => { const { location } = window; const mockLocationToString = (panel) => { // Mocking window.location.toString() - const url = `http://localhost:3000/admin/enterprises/great-shop/edit#!#${panel}` + const url = `http://localhost:3000/admin/enterprises/great-shop/edit#/${panel}` const mockedToString = jest.fn() mockedToString.mockImplementation(() => (url)) From c7c19e47de2a53c2cae6befb56f7980cc78bb80c Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 28 Mar 2023 13:46:51 +1100 Subject: [PATCH 18/18] Add feature toggle description --- lib/open_food_network/feature_toggle.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/open_food_network/feature_toggle.rb b/lib/open_food_network/feature_toggle.rb index e0b8e49451..ebebffb1de 100644 --- a/lib/open_food_network/feature_toggle.rb +++ b/lib/open_food_network/feature_toggle.rb @@ -37,6 +37,9 @@ module OpenFoodNetwork "split_checkout" => <<~DESC, Replace the one-page checkout with a multi-step checkout. DESC + "vouchers" => <<~DESC, + Add voucher functionality. Voucher can be managed via Enterprise settings. + DESC }.freeze # Move your feature entry from CURRENT_FEATURES to RETIRED_FEATURES when