From b9d1cff4060aeef39dacb968dc372fd2d972bc7c Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Wed, 16 Oct 2024 10:42:34 +0200 Subject: [PATCH] FEATURE: add supplier resource filter support --- .../suppliers_controller.rb | 8 +- .../resources_spec.rb | 2 +- .../subscriptions_spec.rb | 17 +- ...subscription_client_resource_fabricator.rb | 12 +- spec/helpers/discourse_helper.rb | 6 +- .../discourse_subscription_client_helper.rb | 7 +- spec/rails_helper.rb | 8 +- .../subscriptions_controller_spec.rb | 4 +- .../suppliers_controller_spec.rb | 159 ++++++++++++------ 9 files changed, 139 insertions(+), 84 deletions(-) diff --git a/app/controllers/discourse_subscription_client/suppliers_controller.rb b/app/controllers/discourse_subscription_client/suppliers_controller.rb index 215484f..acba786 100644 --- a/app/controllers/discourse_subscription_client/suppliers_controller.rb +++ b/app/controllers/discourse_subscription_client/suppliers_controller.rb @@ -7,7 +7,13 @@ class SuppliersController < AdminController before_action :find_supplier, only: %i[authorize destroy] def index - render_serialized(SubscriptionClientSupplier.all, SupplierSerializer, root: "suppliers") + suppliers = SubscriptionClientSupplier.all + if params[:resource] + suppliers = suppliers + .joins(:resources) + .where(resources: { name: params[:resource] }) + end + render_serialized(suppliers, SupplierSerializer, root: "suppliers") end def authorize diff --git a/spec/components/discourse_subscription_client/resources_spec.rb b/spec/components/discourse_subscription_client/resources_spec.rb index adcdce8..abc2121 100644 --- a/spec/components/discourse_subscription_client/resources_spec.rb +++ b/spec/components/discourse_subscription_client/resources_spec.rb @@ -5,7 +5,7 @@ let!(:products) { { "subscription-plugin": [{ product_id: "prod_CBTNpi3fqWWkq0", product_slug: "business" }] } } before do - DiscourseSubscriptionClient.stub(:root) { File.expand_path("../../fixtures", __dir__) } + allow(DiscourseSubscriptionClient).to receive(:root).and_return(File.expand_path("../../fixtures", __dir__)) allow_any_instance_of(DiscourseSubscriptionClient::Resources).to receive(:find_plugins).and_return([{ name: "subscription-plugin", supplier_url: "https://coop.pavilion.tech" }]) end diff --git a/spec/components/discourse_subscription_client/subscriptions_spec.rb b/spec/components/discourse_subscription_client/subscriptions_spec.rb index 75baedd..400a4d4 100644 --- a/spec/components/discourse_subscription_client/subscriptions_spec.rb +++ b/spec/components/discourse_subscription_client/subscriptions_spec.rb @@ -27,7 +27,7 @@ end it "updates subscriptions" do - stub_subscription_request(200, resource, response_body) + stub_subscription_request(200, [resource], response_body) described_class.update subscription = SubscriptionClientSubscription.find_by(product_id: response_body[:subscriptions][0][:product_id]) @@ -36,13 +36,13 @@ end it "deactivates subscriptions" do - stub_subscription_request(200, resource, response_body) + stub_subscription_request(200, [resource], response_body) described_class.update expect(old_subscription.reload.subscribed).to eq(false) end it "reactivates subscriptions" do - stub_subscription_request(200, resource, response_body) + stub_subscription_request(200, [resource], response_body) described_class.update subscription = SubscriptionClientSubscription.find_by(product_id: response_body[:subscriptions][0][:product_id]) @@ -57,7 +57,7 @@ it "deactivates subscriptions when no subscriptions are returned" do stub_subscription_request( 404, - resource, + [resource], { error: "Failed to load discourse-custom-wizard subscriptions for #{user.username}: no subscriptions found for #{resource.name}" } @@ -68,16 +68,15 @@ end it "deactivates subscriptions when there is a connection error" do - stub_subscription_request(404, resource, {}) + stub_subscription_request(404, [resource], {}) described_class.update expect(SubscriptionClientSubscription.exists?(product_id: response_body[:subscriptions][0][:product_id])).to eq(false) end it "triggers an event" do - stub_subscription_request(200, resource, response_body) - DiscourseEvent - .should_receive(:trigger) + stub_subscription_request(200, [resource], response_body) + expect(DiscourseEvent).to receive(:trigger) .with( :subscription_client_subscriptions_updated, instance_of(DiscourseSubscriptionClient::Subscriptions::UpdateResult) @@ -100,7 +99,7 @@ end it "updates resources" do - stub_subscription_request(200, resource, resources_response_body) + stub_subscription_request(200, [resource], resources_response_body) described_class.update resource.reload diff --git a/spec/fabricators/subscription_client_resource_fabricator.rb b/spec/fabricators/subscription_client_resource_fabricator.rb index 0e7a564..107f9ef 100644 --- a/spec/fabricators/subscription_client_resource_fabricator.rb +++ b/spec/fabricators/subscription_client_resource_fabricator.rb @@ -1,17 +1,15 @@ # frozen_string_literal: true Fabricator(:subscription_client_resource) do + transient :products + supplier(fabricator: :subscription_client_supplier) name { sequence(:name) { |i| "resource-#{i}" } } - after_create do |resource| + after_create do |resource, transients| + products = transients[:products] || [{ product_id: "prod_CBTNpi3fqWWkq0", product_slug: "business" }] resource.supplier.products ||= {} - resource.supplier.products[resource.name] = [ - { - product_id: "prod_CBTNpi3fqWWkq0", - product_slug: "business" - } - ] + resource.supplier.products[resource.name] = products resource.supplier.save! end end diff --git a/spec/helpers/discourse_helper.rb b/spec/helpers/discourse_helper.rb index f61744a..d218909 100644 --- a/spec/helpers/discourse_helper.rb +++ b/spec/helpers/discourse_helper.rb @@ -13,8 +13,8 @@ def create_discourse_user(admin: false, moderator: false) end def sign_in(user) - ApplicationController.any_instance.stub(:current_user) { user } - Guardian.any_instance.stub(:is_admin?) { true } if user.admin - Guardian.any_instance.stub(:is_staff?) { true } if user.moderator || user.admin + allow_any_instance_of(ApplicationController).to receive(:current_user).and_return(user) + allow_any_instance_of(Guardian).to receive(:is_admin?).and_return(true) if user.admin + allow_any_instance_of(Guardian).to receive(:is_staff?).and_return(true) if user.moderator || user.admin end end diff --git a/spec/helpers/discourse_subscription_client_helper.rb b/spec/helpers/discourse_subscription_client_helper.rb index 34887cc..636ac4b 100644 --- a/spec/helpers/discourse_subscription_client_helper.rb +++ b/spec/helpers/discourse_subscription_client_helper.rb @@ -20,9 +20,10 @@ def invalid_subscription } end - def stub_subscription_request(status, resource, body) - url = resource.supplier.url - stub_request(:get, "#{url}/subscription-server/user-subscriptions?resources%5B%5D=#{resource.name}").to_return(status: status, body: body.to_json) + def stub_subscription_request(status, resources, body) + url = resources.first.supplier.url + stub_request(:get, "#{url}/subscription-server/user-subscriptions?#{{ resources: resources.map(&:name) }.to_query}") + .to_return(status: status, body: body.to_json) end def stub_server_request(server_url, supplier: nil, products: [], status: 200) diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index bb89275..016953e 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -65,10 +65,10 @@ def freeze_time(now = Time.now) raise "nested freeze time not supported" if block_given? && TrackTimeStub.stubbed - DateTime.stub(:now) { datetime } - Time.stub(:now) { time } - Date.stub(:today) { datetime.to_date } - TrackTimeStub.stub(:stubbed) { true } + allow(DateTime).to receive(:now).and_return(datetime) + allow(Time).to receive(:now).and_return(time) + allow(Date).to receive(:today).and_return(datetime.to_date) + allow(TrackTimeStub).to receive(:stubbed).and_return(true) if block_given? begin diff --git a/spec/requests/discourse_subscription_client/subscriptions_controller_spec.rb b/spec/requests/discourse_subscription_client/subscriptions_controller_spec.rb index 6be3629..3a11907 100644 --- a/spec/requests/discourse_subscription_client/subscriptions_controller_spec.rb +++ b/spec/requests/discourse_subscription_client/subscriptions_controller_spec.rb @@ -40,7 +40,7 @@ end it "updates subscriptions and fixes legacy supplier data" do - stub_subscription_request(200, resource, subscription_response) + stub_subscription_request(200, [resource], subscription_response) old_supplier_record = SubscriptionClientSupplier.first old_supplier_record.update(products: nil) @@ -52,7 +52,7 @@ end it "updates subscriptions" do - stub_subscription_request(200, resource, { subscriptions: [] }) + stub_subscription_request(200, [resource], { subscriptions: [] }) post "/admin/plugins/subscription-client/subscriptions.json" expect(response.status).to eq(200) expect(subscription.reload.subscribed).to eq(false) diff --git a/spec/requests/discourse_subscription_client/suppliers_controller_spec.rb b/spec/requests/discourse_subscription_client/suppliers_controller_spec.rb index a29acaf..772f1aa 100644 --- a/spec/requests/discourse_subscription_client/suppliers_controller_spec.rb +++ b/spec/requests/discourse_subscription_client/suppliers_controller_spec.rb @@ -4,8 +4,25 @@ let(:admin) { create_discourse_user(admin: true) } let(:moderator) { create_discourse_user(moderator: true) } let!(:supplier) { Fabricate(:subscription_client_supplier) } - let!(:resource) { Fabricate(:subscription_client_resource, supplier: supplier) } - let!(:products) { { "subscription-plugin": [{ product_id: "prod_CBTNpi3fqWWkq0", product_slug: "business" }] } } + let!(:another_supplier) { Fabricate(:subscription_client_supplier) } + let!(:products) do + { + "subscription-plugin" => [{ product_id: "prod_CBTNpi3fqWWkq0", product_slug: "business" }], + "another-subscription-plugin" => [{ product_id: "prod_CBTNpi3fqWWkq1", product_slug: "standard" }] + } + end + let!(:resource) do + Fabricate(:subscription_client_resource, + name: "subscription-plugin", + supplier: supplier, + products: [products["subscription-plugin"]]) + end + let!(:another_resource) do + Fabricate(:subscription_client_resource, + name: "another-subscription-plugin", + supplier: supplier, + products: [products["another-subscription-plugin"]]) + end let(:subscription_response) do { subscriptions: [ @@ -22,70 +39,103 @@ include_context "session double" - context "with admin" do - before do - sign_in(admin) - end + before(:each) do + allow_any_instance_of(DiscourseSubscriptionClient::Resources).to receive(:find_plugins).and_return( + [ + { name: resource.name, supplier_url: supplier.url }, + { name: another_resource.name, supplier_url: supplier.url } + ] + ) + stub_server_request(supplier.url, supplier: supplier, products: products, status: 200) + end - before(:each) do - allow_any_instance_of(DiscourseSubscriptionClient::Resources).to receive(:find_plugins).and_return([{ name: resource.name, supplier_url: supplier.url }]) - stub_server_request(supplier.url, supplier: supplier, products: products, status: 200) - end + describe "#index" do + context "with admin" do + before { sign_in(admin) } - it "lists suppliers" do - get "/admin/plugins/subscription-client/suppliers.json" - expect(response.status).to eq(200) - expect(response.parsed_body["suppliers"].size).to eq(1) - expect(response.parsed_body["suppliers"].first["name"]).to eq(supplier.name) - end + it "lists suppliers" do + get "/admin/plugins/subscription-client/suppliers.json" + expect(response.status).to eq(200) + expect(response.parsed_body["suppliers"].size).to eq(2) + expect(response.parsed_body["suppliers"].first["name"]).to eq(supplier.name) + end - it "authorizes" do - get "/admin/plugins/subscription-client/suppliers/authorize", params: { supplier_id: supplier.id } - expect(response.status).to eq(302) - expect(@request.cookie_jar[:user_api_request_id].present?).to eq(true) + it "filters suppliers by resource" do + get "/admin/plugins/subscription-client/suppliers.json?resource=another-subscription-plugin" + expect(response.status).to eq(200) + expect(response.parsed_body["suppliers"].size).to eq(1) + expect(response.parsed_body["suppliers"].first["name"]).to eq(supplier.name) + end end + end + + describe "#authorize" do + context "with admin" do + before { sign_in(admin) } + + it "authorizes" do + get "/admin/plugins/subscription-client/suppliers/authorize", params: { supplier_id: supplier.id } + expect(response.status).to eq(302) + expect(@request.cookie_jar[:user_api_request_id].present?).to eq(true) + end - it "authorizes and stores requested landing page" do - get "/admin/plugins/subscription-client/suppliers/authorize", params: { supplier_id: supplier.id, final_landing_path: "/admin/wizards/wizard" } - expect(response.status).to eq(302) - expect(session[:final_landing_path]).to eq("/admin/wizards/wizard") + it "authorizes and stores requested landing page" do + get "/admin/plugins/subscription-client/suppliers/authorize", params: { + supplier_id: supplier.id, + final_landing_path: "/admin/wizards/wizard" + } + expect(response.status).to eq(302) + expect(session[:final_landing_path]).to eq("/admin/wizards/wizard") + end end + end - it "handles authorization callbacks" do - session_hash[:final_landing_path] = "/admin/plugins/subscription-client/subscriptions" - request_id = cookies[:user_api_request_id] = DiscourseSubscriptionClient::Authorization.request_id(supplier.id) - payload = generate_auth_payload(admin.id, request_id) - stub_subscription_request(200, resource, subscription_response) + describe "#authorize_callback" do + context "with admin" do + before { sign_in(admin) } + + it "handles authorization callbacks" do + session_hash[:final_landing_path] = "/admin/plugins/subscription-client/subscriptions" + request_id = cookies[:user_api_request_id] = DiscourseSubscriptionClient::Authorization.request_id(supplier.id) + payload = generate_auth_payload(admin.id, request_id) + stub_subscription_request(200, [resource, another_resource], subscription_response) - get "/admin/plugins/subscription-client/suppliers/authorize/callback", params: { payload: payload } - expect(response).to redirect_to("/admin/plugins/subscription-client/subscriptions") + get "/admin/plugins/subscription-client/suppliers/authorize/callback", params: { payload: payload } + expect(response).to redirect_to("/admin/plugins/subscription-client/subscriptions") - subscription = SubscriptionClientSubscription.find_by(resource_id: resource.id) - expect(subscription.present?).to eq(true) - expect(subscription.subscribed).to eq(true) - end + subscription = SubscriptionClientSubscription.find_by(resource_id: resource.id) + expect(subscription.present?).to eq(true) + expect(subscription.subscribed).to eq(true) + end - it "handles authorization callbacks and redirects to prior requested landing path" do - session_hash[:final_landing_path] = "/admin/wizards/wizard" - request_id = cookies[:user_api_request_id] = DiscourseSubscriptionClient::Authorization.request_id(supplier.id) - payload = generate_auth_payload(admin.id, request_id) - stub_subscription_request(200, resource, subscription_response) + it "handles authorization callbacks and redirects to prior requested landing path" do + session_hash[:final_landing_path] = "/admin/wizards/wizard" + request_id = cookies[:user_api_request_id] = DiscourseSubscriptionClient::Authorization.request_id(supplier.id) + payload = generate_auth_payload(admin.id, request_id) + stub_subscription_request(200, [resource, another_resource], subscription_response) - get "/admin/plugins/subscription-client/suppliers/authorize/callback", params: { payload: payload } - expect(response).to redirect_to("/admin/wizards/wizard") + get "/admin/plugins/subscription-client/suppliers/authorize/callback", params: { payload: payload } + expect(response).to redirect_to("/admin/wizards/wizard") + end end + end + + describe "#destroy" do + context "with admin" do + before { sign_in(admin) } - it "destroys authorizations" do - request_id = DiscourseSubscriptionClient::Authorization.request_id(supplier.id) - payload = generate_auth_payload(admin.id, request_id) - DiscourseSubscriptionClient::Authorization.process_response(request_id, payload) + it "destroys authorizations" do + request_id = DiscourseSubscriptionClient::Authorization.request_id(supplier.id) + payload = generate_auth_payload(admin.id, request_id) + DiscourseSubscriptionClient::Authorization.process_response(request_id, payload) - stub_request(:post, "#{supplier.url}/user-api-key/revoke").to_return(status: 200, body: '{ "success": "OK" }') + stub_request(:post, "#{supplier.url}/user-api-key/revoke").to_return(status: 200, body: '{ "success": "OK" }') - delete "/admin/plugins/subscription-client/suppliers/authorize", params: { supplier_id: supplier.id } - expect(response.status).to eq(200) - expect(response.parsed_body["supplier_id"]).to eq(supplier.id) - expect(supplier.authorized?).to eq(false) + delete "/admin/plugins/subscription-client/suppliers/authorize", params: { supplier_id: supplier.id } + expect(response.status).to eq(200) + expect(response.parsed_body["supplier_id"]).to eq(supplier.id) + expect(supplier.authorized?).to eq(false) + end end end @@ -96,7 +146,8 @@ end before(:each) do - allow_any_instance_of(DiscourseSubscriptionClient::Resources).to receive(:find_plugins).and_return([{ name: resource.name, supplier_url: supplier.url }]) + allow_any_instance_of(DiscourseSubscriptionClient::Resources).to receive(:find_plugins) + .and_return([{ name: resource.name, supplier_url: supplier.url }]) stub_server_request(supplier.url, supplier: supplier, products: products, status: 200) end @@ -113,7 +164,7 @@ it "lists suppliers" do get "/admin/plugins/subscription-client/suppliers.json" expect(response.status).to eq(200) - expect(response.parsed_body["suppliers"].size).to eq(1) + expect(response.parsed_body["suppliers"].size).to eq(2) end it "authorizes" do @@ -126,7 +177,7 @@ session_hash[:final_landing_path] = "/admin/plugins/subscription-client/subscriptions" request_id = cookies[:user_api_request_id] = DiscourseSubscriptionClient::Authorization.request_id(supplier.id) payload = generate_auth_payload(moderator.id, request_id) - stub_subscription_request(200, resource, subscription_response) + stub_subscription_request(200, [resource, another_resource], subscription_response) get "/admin/plugins/subscription-client/suppliers/authorize/callback", params: { payload: payload } expect(response).to redirect_to("/admin/plugins/subscription-client/subscriptions")