From 9a4051f37b62a75067d02fae4b0f962e248452b9 Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 15 Jul 2024 16:53:30 +1000 Subject: [PATCH 01/13] Move discover regen to named partial To make way for a new type of connected app. If only we could use [relative partial paths](https://github.com/rails/rails/issues/1143) --- app/jobs/connect_app_job.rb | 2 +- .../form/_connected_apps.html.haml | 31 +------------ .../connected_apps/_discover_regen.html.haml | 30 +++++++++++++ config/locales/en.yml | 45 ++++++++++--------- 4 files changed, 55 insertions(+), 53 deletions(-) create mode 100644 app/views/admin/enterprises/form/connected_apps/_discover_regen.html.haml diff --git a/app/jobs/connect_app_job.rb b/app/jobs/connect_app_job.rb index 2474343a29..a93d0b4001 100644 --- a/app/jobs/connect_app_job.rb +++ b/app/jobs/connect_app_job.rb @@ -19,7 +19,7 @@ class ConnectAppJob < ApplicationJob selector = "#connected-app-discover-regen.enterprise_#{enterprise.id}" html = ApplicationController.render( - partial: "admin/enterprises/form/connected_apps", + partial: "admin/enterprises/form/connected_apps/discover_regen", locals: { enterprise: }, ) diff --git a/app/views/admin/enterprises/form/_connected_apps.html.haml b/app/views/admin/enterprises/form/_connected_apps.html.haml index 2ffbceee81..b0f8f608b3 100644 --- a/app/views/admin/enterprises/form/_connected_apps.html.haml +++ b/app/views/admin/enterprises/form/_connected_apps.html.haml @@ -1,30 +1 @@ -%div{ id: "connected-app-discover-regen", class: "enterprise_#{enterprise.id}" } - .connected-app__head - %div - %h3= t ".title" - %p= t ".tagline" - %div - - if enterprise.connected_apps.empty? - = button_to t(".enable"), admin_enterprise_connected_apps_path(enterprise.id), method: :post, disabled: !managed_by_user?(enterprise) - -# This is only seen by super-admins: - %em= t(".need_to_be_manager") unless managed_by_user?(enterprise) - - elsif enterprise.connected_apps.connecting.present? - %button{ disabled: true } - %i.spinner.fa.fa-spin.fa-circle-o-notch -   - = t ".loading" - - else - = button_to t(".disable"), admin_enterprise_connected_app_path(0, enterprise_id: enterprise.id), method: :delete - - .connected-app__connection - - if enterprise.connected_apps.ready.present? - .connected-app__note - - link = enterprise.connected_apps[0].data["link"] - %p= t ".note" - %div - %a{ href: link, target: "_blank", class: "button secondary" } - = t ".link_label" - - %hr - .connected-app__description - = t ".description_html" += render partial: "/admin/enterprises/form/connected_apps/discover_regen", locals: { enterprise: } diff --git a/app/views/admin/enterprises/form/connected_apps/_discover_regen.html.haml b/app/views/admin/enterprises/form/connected_apps/_discover_regen.html.haml new file mode 100644 index 0000000000..2ffbceee81 --- /dev/null +++ b/app/views/admin/enterprises/form/connected_apps/_discover_regen.html.haml @@ -0,0 +1,30 @@ +%div{ id: "connected-app-discover-regen", class: "enterprise_#{enterprise.id}" } + .connected-app__head + %div + %h3= t ".title" + %p= t ".tagline" + %div + - if enterprise.connected_apps.empty? + = button_to t(".enable"), admin_enterprise_connected_apps_path(enterprise.id), method: :post, disabled: !managed_by_user?(enterprise) + -# This is only seen by super-admins: + %em= t(".need_to_be_manager") unless managed_by_user?(enterprise) + - elsif enterprise.connected_apps.connecting.present? + %button{ disabled: true } + %i.spinner.fa.fa-spin.fa-circle-o-notch +   + = t ".loading" + - else + = button_to t(".disable"), admin_enterprise_connected_app_path(0, enterprise_id: enterprise.id), method: :delete + + .connected-app__connection + - if enterprise.connected_apps.ready.present? + .connected-app__note + - link = enterprise.connected_apps[0].data["link"] + %p= t ".note" + %div + %a{ href: link, target: "_blank", class: "button secondary" } + = t ".link_label" + + %hr + .connected-app__description + = t ".description_html" diff --git a/config/locales/en.yml b/config/locales/en.yml index d0764b7a91..de733596a3 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1358,28 +1358,29 @@ en: custom_tab_content: "Content for custom tab" connected_apps: legend: "Connected apps" - title: "Discover Regenerative" - tagline: "Allow Discover Regenerative to publish your enterprise information." - enable: "Allow data sharing" - disable: "Stop sharing" - loading: "Loading" - need_to_be_manager: "Only managers can connect apps." - note: | - Your Open Food Network account is connected to Discover Regenerative. - Add or update information on your Discover Regenerative listing here. - link_label: "Manage listing" - description_html: | -

- Eligible producers can showcase their regenerative credentials, - farming practices and more through a profile listing. - Simplifying how buyers can find regenerative produce and connect - with producers of interest. -

-

- Learn more about Discover Regenerative - -

+ discover_regen: + title: "Discover Regenerative" + tagline: "Allow Discover Regenerative to publish your enterprise information." + enable: "Allow data sharing" + disable: "Stop sharing" + loading: "Loading" + need_to_be_manager: "Only managers can connect apps." + note: | + Your Open Food Network account is connected to Discover Regenerative. + Add or update information on your Discover Regenerative listing here. + link_label: "Manage listing" + description_html: | +

+ Eligible producers can showcase their regenerative credentials, + farming practices and more through a profile listing. + Simplifying how buyers can find regenerative produce and connect + with producers of interest. +

+

+ Learn more about Discover Regenerative + +

actions: edit_profile: Settings properties: Properties From 85d5e2ee7060a1113612d43486ceb33f291ee90c Mon Sep 17 00:00:00 2001 From: David Cook Date: Tue, 16 Jul 2024 15:45:55 +1000 Subject: [PATCH 02/13] Expect single connected_app record for discover regen --- app/jobs/connect_app_job.rb | 2 +- app/models/connected_app.rb | 9 +++++++-- .../admin/enterprises/form/_connected_apps.html.haml | 3 ++- .../form/connected_apps/_discover_regen.html.haml | 8 ++++---- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/app/jobs/connect_app_job.rb b/app/jobs/connect_app_job.rb index a93d0b4001..9645ebd7d5 100644 --- a/app/jobs/connect_app_job.rb +++ b/app/jobs/connect_app_job.rb @@ -20,7 +20,7 @@ class ConnectAppJob < ApplicationJob selector = "#connected-app-discover-regen.enterprise_#{enterprise.id}" html = ApplicationController.render( partial: "admin/enterprises/form/connected_apps/discover_regen", - locals: { enterprise: }, + locals: { enterprise:, connected_app: enterprise.connected_apps.first}, ) cable_ready[channel].morph(selector:, html:).broadcast diff --git a/app/models/connected_app.rb b/app/models/connected_app.rb index 3edc7c4147..71d5908740 100644 --- a/app/models/connected_app.rb +++ b/app/models/connected_app.rb @@ -6,6 +6,11 @@ class ConnectedApp < ApplicationRecord belongs_to :enterprise - scope :connecting, -> { where(data: nil) } - scope :ready, -> { where.not(data: nil) } + def connecting? + data.nil? + end + + def ready? + !connecting? + end end diff --git a/app/views/admin/enterprises/form/_connected_apps.html.haml b/app/views/admin/enterprises/form/_connected_apps.html.haml index b0f8f608b3..5aac9c29c4 100644 --- a/app/views/admin/enterprises/form/_connected_apps.html.haml +++ b/app/views/admin/enterprises/form/_connected_apps.html.haml @@ -1 +1,2 @@ -= render partial: "/admin/enterprises/form/connected_apps/discover_regen", locals: { enterprise: } += render partial: "/admin/enterprises/form/connected_apps/discover_regen", + locals: { enterprise:, connected_app: enterprise.connected_apps.first } diff --git a/app/views/admin/enterprises/form/connected_apps/_discover_regen.html.haml b/app/views/admin/enterprises/form/connected_apps/_discover_regen.html.haml index 2ffbceee81..2b6ef45851 100644 --- a/app/views/admin/enterprises/form/connected_apps/_discover_regen.html.haml +++ b/app/views/admin/enterprises/form/connected_apps/_discover_regen.html.haml @@ -4,11 +4,11 @@ %h3= t ".title" %p= t ".tagline" %div - - if enterprise.connected_apps.empty? + - if connected_app.nil? = button_to t(".enable"), admin_enterprise_connected_apps_path(enterprise.id), method: :post, disabled: !managed_by_user?(enterprise) -# This is only seen by super-admins: %em= t(".need_to_be_manager") unless managed_by_user?(enterprise) - - elsif enterprise.connected_apps.connecting.present? + - elsif connected_app&.connecting? %button{ disabled: true } %i.spinner.fa.fa-spin.fa-circle-o-notch   @@ -17,9 +17,9 @@ = button_to t(".disable"), admin_enterprise_connected_app_path(0, enterprise_id: enterprise.id), method: :delete .connected-app__connection - - if enterprise.connected_apps.ready.present? + - if connected_app&.ready? .connected-app__note - - link = enterprise.connected_apps[0].data["link"] + - link = connected_app.data["link"] %p= t ".note" %div %a{ href: link, target: "_blank", class: "button secondary" } From bbe22bbfeb7b974d54c863ddafa2b8a475e67f12 Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 15 Jul 2024 17:43:38 +1000 Subject: [PATCH 03/13] AddTypeToConnectedApps --- db/migrate/20240715072649_add_type_to_connected_apps.rb | 5 +++++ db/schema.rb | 1 + 2 files changed, 6 insertions(+) create mode 100644 db/migrate/20240715072649_add_type_to_connected_apps.rb diff --git a/db/migrate/20240715072649_add_type_to_connected_apps.rb b/db/migrate/20240715072649_add_type_to_connected_apps.rb new file mode 100644 index 0000000000..5d75f999c8 --- /dev/null +++ b/db/migrate/20240715072649_add_type_to_connected_apps.rb @@ -0,0 +1,5 @@ +class AddTypeToConnectedApps < ActiveRecord::Migration[7.0] + def change + add_column :connected_apps, :type, :string, default: "ConnectedApp", null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index d18db6103f..2310ca953f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -68,6 +68,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_07_18_150852) do t.json "data" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.string "type", default: "ConnectedApp", null: false t.index ["enterprise_id"], name: "index_connected_apps_on_enterprise_id" end From 9b37eacb8dc62ec9b32825a1df73800aa652b4de Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 15 Jul 2024 17:49:08 +1000 Subject: [PATCH 04/13] add scope for discover_regen --- app/jobs/connect_app_job.rb | 2 +- app/models/connected_app.rb | 2 ++ app/views/admin/enterprises/form/_connected_apps.html.haml | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/jobs/connect_app_job.rb b/app/jobs/connect_app_job.rb index 9645ebd7d5..fa1b802d05 100644 --- a/app/jobs/connect_app_job.rb +++ b/app/jobs/connect_app_job.rb @@ -20,7 +20,7 @@ class ConnectAppJob < ApplicationJob selector = "#connected-app-discover-regen.enterprise_#{enterprise.id}" html = ApplicationController.render( partial: "admin/enterprises/form/connected_apps/discover_regen", - locals: { enterprise:, connected_app: enterprise.connected_apps.first}, + locals: { enterprise:, connected_app: enterprise.connected_apps.discover_regen.first }, ) cable_ready[channel].morph(selector:, html:).broadcast diff --git a/app/models/connected_app.rb b/app/models/connected_app.rb index 71d5908740..d2e6ffa8b7 100644 --- a/app/models/connected_app.rb +++ b/app/models/connected_app.rb @@ -6,6 +6,8 @@ class ConnectedApp < ApplicationRecord belongs_to :enterprise + scope :discover_regen, -> { where(type: "ConnectedApp") } + def connecting? data.nil? end diff --git a/app/views/admin/enterprises/form/_connected_apps.html.haml b/app/views/admin/enterprises/form/_connected_apps.html.haml index 5aac9c29c4..27e3d5a567 100644 --- a/app/views/admin/enterprises/form/_connected_apps.html.haml +++ b/app/views/admin/enterprises/form/_connected_apps.html.haml @@ -1,2 +1,2 @@ = render partial: "/admin/enterprises/form/connected_apps/discover_regen", - locals: { enterprise:, connected_app: enterprise.connected_apps.first } + locals: { enterprise:, connected_app: enterprise.connected_apps.discover_regen.first } From 9d89b4726bac8d40fb7566feef4cb2bcca9d6be3 Mon Sep 17 00:00:00 2001 From: David Cook Date: Tue, 16 Jul 2024 13:04:23 +1000 Subject: [PATCH 05/13] Move connect logic to model Then subtypes can override as needed. --- app/controllers/admin/connected_apps_controller.rb | 12 +----------- app/models/connected_app.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/app/controllers/admin/connected_apps_controller.rb b/app/controllers/admin/connected_apps_controller.rb index 2cbaca7c9b..9ddf7d9405 100644 --- a/app/controllers/admin/connected_apps_controller.rb +++ b/app/controllers/admin/connected_apps_controller.rb @@ -6,11 +6,7 @@ module Admin authorize! :admin, enterprise app = ConnectedApp.create!(enterprise_id: enterprise.id) - - ConnectAppJob.perform_later( - app, spree_current_user.spree_api_key, - channel: SessionChannel.for_request(request), - ) + app.connect(api_key: spree_current_user.spree_api_key, channel: SessionChannel.for_request(request)) render_panel end @@ -21,12 +17,6 @@ module Admin app = enterprise.connected_apps.first app.destroy - WebhookDeliveryJob.perform_later( - app.data["destroy"], - "disconnect-app", - nil - ) - render_panel end diff --git a/app/models/connected_app.rb b/app/models/connected_app.rb index d2e6ffa8b7..85cdc6f9a5 100644 --- a/app/models/connected_app.rb +++ b/app/models/connected_app.rb @@ -5,6 +5,7 @@ # Here we store keys and links to access the app. class ConnectedApp < ApplicationRecord belongs_to :enterprise + after_destroy :disconnect scope :discover_regen, -> { where(type: "ConnectedApp") } @@ -15,4 +16,16 @@ class ConnectedApp < ApplicationRecord def ready? !connecting? end + + def connect(api_key:, channel:) + ConnectAppJob.perform_later(self, api_key, channel:) + end + + def disconnect + WebhookDeliveryJob.perform_later( + data["destroy"], + "disconnect-app", + nil + ) + end end From 5d0f55b8c3a7693bbf56a079abc9f24c35eda79f Mon Sep 17 00:00:00 2001 From: David Cook Date: Tue, 16 Jul 2024 13:19:24 +1000 Subject: [PATCH 06/13] Re-organise spec Best viewed with whitespac ignored. --- .../can_be_enabled_and_disabled.yml | 0 .../admin/enterprises/connected_apps_spec.rb | 50 ++++++++++--------- 2 files changed, 26 insertions(+), 24 deletions(-) rename spec/fixtures/vcr_cassettes/Connected_Apps/{ => Discover_Regenerative}/can_be_enabled_and_disabled.yml (100%) diff --git a/spec/fixtures/vcr_cassettes/Connected_Apps/can_be_enabled_and_disabled.yml b/spec/fixtures/vcr_cassettes/Connected_Apps/Discover_Regenerative/can_be_enabled_and_disabled.yml similarity index 100% rename from spec/fixtures/vcr_cassettes/Connected_Apps/can_be_enabled_and_disabled.yml rename to spec/fixtures/vcr_cassettes/Connected_Apps/Discover_Regenerative/can_be_enabled_and_disabled.yml diff --git a/spec/system/admin/enterprises/connected_apps_spec.rb b/spec/system/admin/enterprises/connected_apps_spec.rb index 652982b0ed..cf63dc7908 100644 --- a/spec/system/admin/enterprises/connected_apps_spec.rb +++ b/spec/system/admin/enterprises/connected_apps_spec.rb @@ -28,36 +28,38 @@ RSpec.describe "Connected Apps", feature: :connected_apps, vcr: true do expect(page).to have_content "CONNECTED APPS" end - it "can be enabled and disabled" do - visit edit_admin_enterprise_path(enterprise) + describe "Discover Regenerative" do + it "can be enabled and disabled" do + visit edit_admin_enterprise_path(enterprise) - scroll_to :bottom - click_link "Connected apps" - expect(page).to have_content "Discover Regenerative" + scroll_to :bottom + click_link "Connected apps" + expect(page).to have_content "Discover Regenerative" - click_button "Allow data sharing" - expect(page).not_to have_button "Allow data sharing" - expect(page).to have_button "Loading", disabled: true + click_button "Allow data sharing" + expect(page).not_to have_button "Allow data sharing" + expect(page).to have_button "Loading", disabled: true - perform_enqueued_jobs(only: ConnectAppJob) - expect(page).not_to have_button "Loading", disabled: true - expect(page).to have_content "account is connected" - expect(page).to have_link "Manage listing" + perform_enqueued_jobs(only: ConnectAppJob) + expect(page).not_to have_button "Loading", disabled: true + expect(page).to have_content "account is connected" + expect(page).to have_link "Manage listing" - click_button "Stop sharing" - expect(page).to have_button "Allow data sharing" - expect(page).not_to have_button "Stop sharing" - expect(page).not_to have_content "account is connected" - expect(page).not_to have_link "Manage listing" - end + click_button "Stop sharing" + expect(page).to have_button "Allow data sharing" + expect(page).not_to have_button "Stop sharing" + expect(page).not_to have_content "account is connected" + expect(page).not_to have_link "Manage listing" + end - it "can't be enabled by non-manager" do - login_as create(:admin_user) + it "can't be enabled by non-manager" do + login_as create(:admin_user) - visit "#{edit_admin_enterprise_path(enterprise)}#/connected_apps_panel" - expect(page).to have_content "Discover Regenerative" + visit "#{edit_admin_enterprise_path(enterprise)}#/connected_apps_panel" + expect(page).to have_content "Discover Regenerative" - expect(page).to have_button("Allow data sharing", disabled: true) - expect(page).to have_content "Only managers can connect apps." + expect(page).to have_button("Allow data sharing", disabled: true) + expect(page).to have_content "Only managers can connect apps." + end end end From 27e53f9dcc3796c4ea97381141477a81a65d4a04 Mon Sep 17 00:00:00 2001 From: David Cook Date: Tue, 16 Jul 2024 14:30:27 +1000 Subject: [PATCH 07/13] Scope spec to section Because there's going to be a new section with the same button label --- .../connected_apps/_discover_regen.html.haml | 2 +- .../admin/enterprises/connected_apps_spec.rb | 49 +++++++++++++------ 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/app/views/admin/enterprises/form/connected_apps/_discover_regen.html.haml b/app/views/admin/enterprises/form/connected_apps/_discover_regen.html.haml index 2b6ef45851..5011de7d1f 100644 --- a/app/views/admin/enterprises/form/connected_apps/_discover_regen.html.haml +++ b/app/views/admin/enterprises/form/connected_apps/_discover_regen.html.haml @@ -1,4 +1,4 @@ -%div{ id: "connected-app-discover-regen", class: "enterprise_#{enterprise.id}" } +%section.connected_app{ id: "connected-app-discover-regen", class: "enterprise_#{enterprise.id}" } .connected-app__head %div %h3= t ".title" diff --git a/spec/system/admin/enterprises/connected_apps_spec.rb b/spec/system/admin/enterprises/connected_apps_spec.rb index cf63dc7908..ba9fd3ce7c 100644 --- a/spec/system/admin/enterprises/connected_apps_spec.rb +++ b/spec/system/admin/enterprises/connected_apps_spec.rb @@ -34,32 +34,49 @@ RSpec.describe "Connected Apps", feature: :connected_apps, vcr: true do scroll_to :bottom click_link "Connected apps" - expect(page).to have_content "Discover Regenerative" - click_button "Allow data sharing" - expect(page).not_to have_button "Allow data sharing" - expect(page).to have_button "Loading", disabled: true + within section_containing_heading "Discover Regenerative" do + click_button "Allow data sharing" + end - perform_enqueued_jobs(only: ConnectAppJob) - expect(page).not_to have_button "Loading", disabled: true - expect(page).to have_content "account is connected" - expect(page).to have_link "Manage listing" + # (page is reloaded so we need to evaluate within block again) + within section_containing_heading "Discover Regenerative" do + expect(page).not_to have_button "Allow data sharing" + expect(page).to have_button "Loading", disabled: true - click_button "Stop sharing" - expect(page).to have_button "Allow data sharing" - expect(page).not_to have_button "Stop sharing" - expect(page).not_to have_content "account is connected" - expect(page).not_to have_link "Manage listing" + perform_enqueued_jobs(only: ConnectAppJob) + end + + within section_containing_heading "Discover Regenerative" do + expect(page).not_to have_button "Loading", disabled: true + expect(page).to have_content "account is connected" + expect(page).to have_link "Manage listing" + + click_button "Stop sharing" + end + + within section_containing_heading "Discover Regenerative" do + expect(page).to have_button "Allow data sharing" + expect(page).not_to have_button "Stop sharing" + expect(page).not_to have_content "account is connected" + expect(page).not_to have_link "Manage listing" + end end it "can't be enabled by non-manager" do login_as create(:admin_user) visit "#{edit_admin_enterprise_path(enterprise)}#/connected_apps_panel" - expect(page).to have_content "Discover Regenerative" - expect(page).to have_button("Allow data sharing", disabled: true) - expect(page).to have_content "Only managers can connect apps." + within section_containing_heading "Discover Regenerative" do + expect(page).to have_button("Allow data sharing", disabled: true) + expect(page).to have_content "Only managers can connect apps." + end end end + + def section_containing_heading(heading) + page.find("h3", text: heading).ancestor("section") + end end + From d3c5e2365a5161c14de86185373370b6f68d235b Mon Sep 17 00:00:00 2001 From: David Cook Date: Tue, 16 Jul 2024 12:34:47 +1000 Subject: [PATCH 08/13] Add AffiliateSalesData model Using namespace subfolder to help organise it and show the inheritance. Hmm, instead of scopes, we could have different has_many relationships on the Enterprise. Maybe it should be in a concern. We can refactor later I guess. --- app/models/connected_app.rb | 1 + app/models/connected_apps/affiliate_sales_data.rb | 11 +++++++++++ 2 files changed, 12 insertions(+) create mode 100644 app/models/connected_apps/affiliate_sales_data.rb diff --git a/app/models/connected_app.rb b/app/models/connected_app.rb index 85cdc6f9a5..4d45d10c89 100644 --- a/app/models/connected_app.rb +++ b/app/models/connected_app.rb @@ -8,6 +8,7 @@ class ConnectedApp < ApplicationRecord after_destroy :disconnect scope :discover_regen, -> { where(type: "ConnectedApp") } + scope :affiliate_sales_data, -> { where(type: "ConnectedApps::AffiliateSalesData") } def connecting? data.nil? diff --git a/app/models/connected_apps/affiliate_sales_data.rb b/app/models/connected_apps/affiliate_sales_data.rb new file mode 100644 index 0000000000..a9425a1303 --- /dev/null +++ b/app/models/connected_apps/affiliate_sales_data.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +# An enterprise can opt-in for their data to be included in the affiliate_sales_data endpoint +# +module ConnectedApps + class AffiliateSalesData < ConnectedApp + def connect; end + + def disconnect; end + end +end From 1742d2807fe5fae903f32c2bacafe4fe48eb8beb Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 15 Jul 2024 17:15:42 +1000 Subject: [PATCH 09/13] Add affiliate sales data to form --- .../enterprises/form/_connected_apps.html.haml | 2 ++ .../_affiliate_sales_data.html.haml | 15 +++++++++++++++ app/webpacker/css/admin/connected_apps.scss | 10 +++++++++- config/locales/en.yml | 16 ++++++++++++++++ 4 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 app/views/admin/enterprises/form/connected_apps/_affiliate_sales_data.html.haml diff --git a/app/views/admin/enterprises/form/_connected_apps.html.haml b/app/views/admin/enterprises/form/_connected_apps.html.haml index 27e3d5a567..1c7904b2ed 100644 --- a/app/views/admin/enterprises/form/_connected_apps.html.haml +++ b/app/views/admin/enterprises/form/_connected_apps.html.haml @@ -1,2 +1,4 @@ = render partial: "/admin/enterprises/form/connected_apps/discover_regen", locals: { enterprise:, connected_app: enterprise.connected_apps.discover_regen.first } += render partial: "/admin/enterprises/form/connected_apps/affiliate_sales_data", + locals: { enterprise:, connected_app: enterprise.connected_apps.affiliate_sales_data.first } diff --git a/app/views/admin/enterprises/form/connected_apps/_affiliate_sales_data.html.haml b/app/views/admin/enterprises/form/connected_apps/_affiliate_sales_data.html.haml new file mode 100644 index 0000000000..1674d00564 --- /dev/null +++ b/app/views/admin/enterprises/form/connected_apps/_affiliate_sales_data.html.haml @@ -0,0 +1,15 @@ +%section.connected_app + .connected-app__head + %div + %h3= t ".title" + %p= t ".tagline" + %div + - if connected_app.nil? + = button_to t(".enable"), admin_enterprise_connected_apps_path(enterprise.id), method: :post, disabled: !managed_by_user?(enterprise) + -# This is only seen by super-admins: + %em= t(".need_to_be_manager") unless managed_by_user?(enterprise) + - else + = button_to t(".disable"), admin_enterprise_connected_app_path(0, enterprise_id: enterprise.id), method: :delete + %hr + .connected-app__description + = t ".description_html" diff --git a/app/webpacker/css/admin/connected_apps.scss b/app/webpacker/css/admin/connected_apps.scss index c2cb2c9421..be3356009d 100644 --- a/app/webpacker/css/admin/connected_apps.scss +++ b/app/webpacker/css/admin/connected_apps.scss @@ -2,10 +2,18 @@ max-width: 615px; } +.connected_app { + margin-bottom: 2rem; + + &:not(:first-of-type) { + border-top: 1px solid $color-border; + } +} + .connected-app__head { display: flex; justify-content: space-between; - margin-bottom: 1em; + margin: 1em 0; h3 { margin-bottom: 1em; diff --git a/config/locales/en.yml b/config/locales/en.yml index de733596a3..7eb8c3dbf4 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1358,6 +1358,22 @@ en: custom_tab_content: "Content for custom tab" connected_apps: legend: "Connected apps" + affiliate_sales_data: + title: "INRAE / UFC QUE CHOISIR Research" + tagline: "Allow this research project to access your orders data anonymously" + enable: "Allow data sharing" + disable: "Stop sharing" + loading: "Loading" + need_to_be_manager: "Only managers can connect apps." + description_html: | +

+ INRAE and UFC QUE CHOISIR are teaming up to study food prices in short food systems and compare them with prices in the supermarket, for a given set of products. The data that is used by INRAE is mixed with data coming from other short food chain platforms in France. No individual product prices will be publicly disclosed through this project. +

+

+ Learn more about this research project + +

discover_regen: title: "Discover Regenerative" tagline: "Allow Discover Regenerative to publish your enterprise information." From da7bbcf82fb2cf2d66065d6818d157f34abb06ad Mon Sep 17 00:00:00 2001 From: David Cook Date: Tue, 16 Jul 2024 15:51:46 +1000 Subject: [PATCH 10/13] Enable/disable affiliate sales data --- .../admin/connected_apps_controller.rb | 14 +++-- .../connected_apps/affiliate_sales_data.rb | 4 +- .../_affiliate_sales_data.html.haml | 4 +- .../connected_apps/_discover_regen.html.haml | 2 +- .../admin/enterprises/connected_apps_spec.rb | 52 ++++++++++++++++--- 5 files changed, 61 insertions(+), 15 deletions(-) diff --git a/app/controllers/admin/connected_apps_controller.rb b/app/controllers/admin/connected_apps_controller.rb index 9ddf7d9405..3531c7371a 100644 --- a/app/controllers/admin/connected_apps_controller.rb +++ b/app/controllers/admin/connected_apps_controller.rb @@ -5,8 +5,12 @@ module Admin def create authorize! :admin, enterprise - app = ConnectedApp.create!(enterprise_id: enterprise.id) - app.connect(api_key: spree_current_user.spree_api_key, channel: SessionChannel.for_request(request)) + attributes = {} + attributes[:type] = connected_app_params[:type] if connected_app_params[:type] + + app = ConnectedApp.create!(enterprise_id: enterprise.id, **attributes) + app.connect(api_key: spree_current_user.spree_api_key, + channel: SessionChannel.for_request(request)) render_panel end @@ -14,7 +18,7 @@ module Admin def destroy authorize! :admin, enterprise - app = enterprise.connected_apps.first + app = enterprise.connected_apps.find(params.require(:id)) app.destroy render_panel @@ -29,5 +33,9 @@ module Admin def render_panel redirect_to "#{edit_admin_enterprise_path(enterprise)}#/connected_apps_panel" end + + def connected_app_params + params.permit(:type) + end end end diff --git a/app/models/connected_apps/affiliate_sales_data.rb b/app/models/connected_apps/affiliate_sales_data.rb index a9425a1303..51d9d1b5df 100644 --- a/app/models/connected_apps/affiliate_sales_data.rb +++ b/app/models/connected_apps/affiliate_sales_data.rb @@ -4,7 +4,9 @@ # module ConnectedApps class AffiliateSalesData < ConnectedApp - def connect; end + def connect(_opts) + update! data: true # not-nil value indicates it is ready + end def disconnect; end end diff --git a/app/views/admin/enterprises/form/connected_apps/_affiliate_sales_data.html.haml b/app/views/admin/enterprises/form/connected_apps/_affiliate_sales_data.html.haml index 1674d00564..52ec355a35 100644 --- a/app/views/admin/enterprises/form/connected_apps/_affiliate_sales_data.html.haml +++ b/app/views/admin/enterprises/form/connected_apps/_affiliate_sales_data.html.haml @@ -5,11 +5,11 @@ %p= t ".tagline" %div - if connected_app.nil? - = button_to t(".enable"), admin_enterprise_connected_apps_path(enterprise.id), method: :post, disabled: !managed_by_user?(enterprise) + = button_to t(".enable"), admin_enterprise_connected_apps_path(enterprise.id, type: "ConnectedApps::AffiliateSalesData"), method: :post, disabled: !managed_by_user?(enterprise) -# This is only seen by super-admins: %em= t(".need_to_be_manager") unless managed_by_user?(enterprise) - else - = button_to t(".disable"), admin_enterprise_connected_app_path(0, enterprise_id: enterprise.id), method: :delete + = button_to t(".disable"), admin_enterprise_connected_app_path(connected_app.id, enterprise_id: enterprise.id), method: :delete %hr .connected-app__description = t ".description_html" diff --git a/app/views/admin/enterprises/form/connected_apps/_discover_regen.html.haml b/app/views/admin/enterprises/form/connected_apps/_discover_regen.html.haml index 5011de7d1f..1bab8f75ec 100644 --- a/app/views/admin/enterprises/form/connected_apps/_discover_regen.html.haml +++ b/app/views/admin/enterprises/form/connected_apps/_discover_regen.html.haml @@ -14,7 +14,7 @@   = t ".loading" - else - = button_to t(".disable"), admin_enterprise_connected_app_path(0, enterprise_id: enterprise.id), method: :delete + = button_to t(".disable"), admin_enterprise_connected_app_path(connected_app.id, enterprise_id: enterprise.id), method: :delete .connected-app__connection - if connected_app&.ready? diff --git a/spec/system/admin/enterprises/connected_apps_spec.rb b/spec/system/admin/enterprises/connected_apps_spec.rb index ba9fd3ce7c..4f1a67b3f7 100644 --- a/spec/system/admin/enterprises/connected_apps_spec.rb +++ b/spec/system/admin/enterprises/connected_apps_spec.rb @@ -29,25 +29,25 @@ RSpec.describe "Connected Apps", feature: :connected_apps, vcr: true do end describe "Discover Regenerative" do + let(:section_heading) { self.class.description } + it "can be enabled and disabled" do visit edit_admin_enterprise_path(enterprise) scroll_to :bottom click_link "Connected apps" - within section_containing_heading "Discover Regenerative" do + within section_containing_heading do click_button "Allow data sharing" end # (page is reloaded so we need to evaluate within block again) - within section_containing_heading "Discover Regenerative" do + within section_containing_heading do expect(page).not_to have_button "Allow data sharing" expect(page).to have_button "Loading", disabled: true perform_enqueued_jobs(only: ConnectAppJob) - end - within section_containing_heading "Discover Regenerative" do expect(page).not_to have_button "Loading", disabled: true expect(page).to have_content "account is connected" expect(page).to have_link "Manage listing" @@ -55,7 +55,7 @@ RSpec.describe "Connected Apps", feature: :connected_apps, vcr: true do click_button "Stop sharing" end - within section_containing_heading "Discover Regenerative" do + within section_containing_heading do expect(page).to have_button "Allow data sharing" expect(page).not_to have_button "Stop sharing" expect(page).not_to have_content "account is connected" @@ -68,15 +68,51 @@ RSpec.describe "Connected Apps", feature: :connected_apps, vcr: true do visit "#{edit_admin_enterprise_path(enterprise)}#/connected_apps_panel" - within section_containing_heading "Discover Regenerative" do + within section_containing_heading do expect(page).to have_button("Allow data sharing", disabled: true) expect(page).to have_content "Only managers can connect apps." end end end - def section_containing_heading(heading) + describe "Affiliate Sales Data" do + let(:section_heading) { "INRAE / UFC QUE CHOISIR Research" } + + it "can be enabled and disabled" do + visit edit_admin_enterprise_path(enterprise) + + scroll_to :bottom + click_link "Connected apps" + + within section_containing_heading do + click_button "Allow data sharing" + end + + # (page is reloaded so we need to evaluate within block again) + within section_containing_heading do + expect(page).not_to have_button "Allow data sharing" + click_button "Stop sharing" + end + + within section_containing_heading do + expect(page).to have_button "Allow data sharing" + expect(page).not_to have_button "Stop sharing" + end + end + + it "can't be enabled by non-manager" do + login_as create(:admin_user) + + visit "#{edit_admin_enterprise_path(enterprise)}#/connected_apps_panel" + + within section_containing_heading do + expect(page).to have_button("Allow data sharing", disabled: true) + expect(page).to have_content "Only managers can connect apps." + end + end + end + + def section_containing_heading(heading = section_heading) page.find("h3", text: heading).ancestor("section") end end - From e9d7a0b0994b25a10018fe69f55426f00e585d42 Mon Sep 17 00:00:00 2001 From: David Cook Date: Wed, 17 Jul 2024 12:04:54 +1000 Subject: [PATCH 11/13] Add User#affiliate_enterprises --- app/models/spree/user.rb | 8 ++++++++ spec/models/spree/user_spec.rb | 27 +++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index f7ceb5ddca..0d03670e53 100644 --- a/app/models/spree/user.rb +++ b/app/models/spree/user.rb @@ -156,6 +156,14 @@ module Spree self.disabled_at = value == '1' ? Time.zone.now : nil end + def affiliate_enterprises + return [] unless Flipper.enabled?(:affiliate_sales_data, self) + + Enterprise.joins(:connected_apps) + .where(connected_apps: { type: "ConnectedApps::AffiliateSalesData" }) + .where.not(connected_apps: { data: nil }) + end + protected def password_required? diff --git a/spec/models/spree/user_spec.rb b/spec/models/spree/user_spec.rb index c53b8d7845..aa13e6f230 100644 --- a/spec/models/spree/user_spec.rb +++ b/spec/models/spree/user_spec.rb @@ -268,4 +268,31 @@ RSpec.describe Spree::User do expect(user.disabled).to be_falsey end end + + describe "#affiliate_enterprises" do + let(:user) { create(:user) } + let(:affiliate_enterprise) { create(:enterprise) } + let(:other_enterprise) { create(:enterprise) } + subject{ user.affiliate_enterprises } + + before do + ConnectedApps::AffiliateSalesData.create(enterprise: affiliate_enterprise, data: true) + end + + context "user does not have feature" do + it { is_expected.to be_empty } + end + + context "user has feature affiliate_sales_data" do + before do + Flipper.enable_actor(:affiliate_sales_data, user) + user.reload + end + + it "includes only affiliate enterprises" do + is_expected.to include affiliate_enterprise + is_expected.not_to include other_enterprise + end + end + end end From df81e8ed3558b8c86fd9ac4471d2324e5bf7694b Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 25 Jul 2024 17:37:44 +1000 Subject: [PATCH 12/13] Add task to connect all enterprises Example usage: rake ofn:enterprises:activate_connected_app_type[affiliate_sales_data] --- lib/tasks/enterprises.rake | 12 ++++++++++++ spec/lib/tasks/enterprises_rake_spec.rb | 22 ++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/lib/tasks/enterprises.rake b/lib/tasks/enterprises.rake index fa735853c2..86c767dddd 100644 --- a/lib/tasks/enterprises.rake +++ b/lib/tasks/enterprises.rake @@ -12,6 +12,18 @@ namespace :ofn do enterprise.destroy end + namespace :enterprises do + desc "Activate connected app type for ALL enterprises" + task :activate_connected_app_type, [:type] => :environment do |_task, args| + Enterprise.find_each do |enterprise| + next if enterprise.connected_apps.public_send(args.type.underscore).exists? + + "ConnectedApps::#{args.type.camelize}".constantize.new(enterprise:).connect({}) + puts "Enterprise #{enterprise.id} connected." + end + end + end + namespace :dev do desc 'export enterprises to CSV' task export_enterprises: :environment do diff --git a/spec/lib/tasks/enterprises_rake_spec.rb b/spec/lib/tasks/enterprises_rake_spec.rb index 0ebf97b7d7..2e98502f0e 100644 --- a/spec/lib/tasks/enterprises_rake_spec.rb +++ b/spec/lib/tasks/enterprises_rake_spec.rb @@ -20,4 +20,26 @@ RSpec.describe 'enterprises.rake' do end end end + + describe ':enterprises' do + describe ':activate_connected_app_type' do + it 'updates only disconnected enterprises' do + # enterprise with affiliate sales data + enterprise_asd = create(:enterprise) + enterprise_asd.connected_apps.create type: 'ConnectedApps::AffiliateSalesData' + # enterprise with different type + enterprise_diff = create(:enterprise) + enterprise_diff.connected_apps.create + + expect { + Rake.application.invoke_task( + "ofn:enterprises:activate_connected_app_type[affiliate_sales_data]" + ) + }.to change { ConnectedApps::AffiliateSalesData.count }.by(1) + + expect(enterprise_asd.connected_apps.affiliate_sales_data.count).to eq 1 + expect(enterprise_diff.connected_apps.affiliate_sales_data.count).to eq 1 + end + end + end end From c5fc621aa408230b5e29af1973b6f8da678d5b68 Mon Sep 17 00:00:00 2001 From: David Cook Date: Tue, 30 Jul 2024 15:21:29 +1000 Subject: [PATCH 13/13] Use scope to determine which enterprises are ready to be affiliated --- app/models/connected_app.rb | 3 +++ app/models/spree/user.rb | 4 +--- spec/models/spree/user_spec.rb | 3 +++ 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/models/connected_app.rb b/app/models/connected_app.rb index 4d45d10c89..9c99ba7dcc 100644 --- a/app/models/connected_app.rb +++ b/app/models/connected_app.rb @@ -10,6 +10,9 @@ class ConnectedApp < ApplicationRecord scope :discover_regen, -> { where(type: "ConnectedApp") } scope :affiliate_sales_data, -> { where(type: "ConnectedApps::AffiliateSalesData") } + scope :connecting, -> { where(data: nil) } + scope :ready, -> { where.not(data: nil) } + def connecting? data.nil? end diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index 0d03670e53..bef7609933 100644 --- a/app/models/spree/user.rb +++ b/app/models/spree/user.rb @@ -159,9 +159,7 @@ module Spree def affiliate_enterprises return [] unless Flipper.enabled?(:affiliate_sales_data, self) - Enterprise.joins(:connected_apps) - .where(connected_apps: { type: "ConnectedApps::AffiliateSalesData" }) - .where.not(connected_apps: { data: nil }) + Enterprise.joins(:connected_apps).merge(ConnectedApps::AffiliateSalesData.ready) end protected diff --git a/spec/models/spree/user_spec.rb b/spec/models/spree/user_spec.rb index aa13e6f230..a2a0ab3897 100644 --- a/spec/models/spree/user_spec.rb +++ b/spec/models/spree/user_spec.rb @@ -272,11 +272,13 @@ RSpec.describe Spree::User do describe "#affiliate_enterprises" do let(:user) { create(:user) } let(:affiliate_enterprise) { create(:enterprise) } + let(:other_connected_enterprise) { create(:enterprise) } let(:other_enterprise) { create(:enterprise) } subject{ user.affiliate_enterprises } before do ConnectedApps::AffiliateSalesData.create(enterprise: affiliate_enterprise, data: true) + ConnectedApp.create(enterprise: other_connected_enterprise, data: true) end context "user does not have feature" do @@ -291,6 +293,7 @@ RSpec.describe Spree::User do it "includes only affiliate enterprises" do is_expected.to include affiliate_enterprise + is_expected.not_to include other_connected_enterprise is_expected.not_to include other_enterprise end end