Per review, small code improvment

This commit is contained in:
Gaetan Craig-Riou
2025-12-08 16:20:27 +11:00
parent 4073238654
commit 584b976dff
4 changed files with 15 additions and 28 deletions

View File

@@ -1,6 +1,6 @@
# frozen_string_literal: true
# Create a webhook payload for an payment status event.
# Create a webhook payload for a payment status event.
# The payload will be delivered asynchronously.
module Payments
@@ -17,11 +17,11 @@ module Payments
def self.webhook_urls(coordinator)
# url for coordinator owner
webhook_urls = coordinator.owner.webhook_endpoints.payment_status.map(&:url)
webhook_urls = coordinator.owner.webhook_endpoints.payment_status.pluck(:url)
# plus url for coordinator manager (ignore duplicate)
users_webhook_urls = coordinator.users.flat_map do |user|
user.webhook_endpoints.payment_status.map(&:url)
user.webhook_endpoints.payment_status.pluck(:url)
end
webhook_urls | users_webhook_urls

View File

@@ -22,8 +22,7 @@ RSpec.describe OrderCycles::WebhookService do
# The co-ordinating enterprise has a non-owner user with an endpoint.
# They shouldn't receive a notification.
coordinator_user = create(:user, enterprises: [coordinator])
coordinator_user.webhook_endpoints.create!(url: "http://coordinator_user_url",
webhook_type: "order_cycle_opened")
coordinator_user.webhook_endpoints.order_cycle_opened.create!(url: "http://coordinator_user_url")
expect{ subject }
.not_to enqueue_job(WebhookDeliveryJob).with("http://coordinator_user_url", any_args)
@@ -31,8 +30,7 @@ RSpec.describe OrderCycles::WebhookService do
context "coordinator owner has endpoint configured" do
before do
coordinator.owner.webhook_endpoints.create! url: "http://coordinator_owner_url",
webhook_type: "order_cycle_opened"
coordinator.owner.webhook_endpoints.order_cycle_opened.create!(url: "http://coordinator_owner_url")
end
it "creates webhook payload for order cycle coordinator" do
@@ -79,8 +77,7 @@ RSpec.describe OrderCycles::WebhookService do
let(:two_distributors) {
(1..2).map do |i|
user = create(:user)
user.webhook_endpoints.create!(url: "http://distributor#{i}_owner_url",
webhook_type: "order_cycle_opened")
user.webhook_endpoints.order_cycle_opened.create!(url: "http://distributor#{i}_owner_url")
create(:distributor_enterprise, owner: user)
end
}
@@ -112,8 +109,7 @@ RSpec.describe OrderCycles::WebhookService do
}
it "creates only one webhook payload for the user's endpoint" do
user.webhook_endpoints.create! url: "http://coordinator_owner_url",
webhook_type: "order_cycle_opened"
user.webhook_endpoints.order_cycle_opened.create!(url: "http://coordinator_owner_url")
expect{ subject }
.to enqueue_job(WebhookDeliveryJob).with("http://coordinator_owner_url", any_args)
@@ -132,8 +128,7 @@ RSpec.describe OrderCycles::WebhookService do
}
let(:supplier) {
user = create(:user)
user.webhook_endpoints.create!(url: "http://supplier_owner_url",
webhook_type: "order_cycle_opened")
user.webhook_endpoints.order_cycle_opened.create!(url: "http://supplier_owner_url")
create(:supplier_enterprise, owner: user)
}

View File

@@ -11,7 +11,7 @@ RSpec.describe Payments::WebhookPayload do
subject { described_class.new(payment:, order:, enterprise: order.distributor) }
it "returns a formated hash" do
it "returns a hash with the relevant data" do
order.line_items.update_all(tax_category_id: tax_category.id)
enterprise = order.distributor

View File

@@ -14,8 +14,8 @@ RSpec.describe Payments::WebhookService do
describe "creating payloads" do
context "with order cycle coordinator owner webhook endpoints configured" do
before do
order.order_cycle.coordinator.owner.webhook_endpoints.create!(
url: "http://coordinator.payment.url", webhook_type: "payment_status_changed"
order.order_cycle.coordinator.owner.webhook_endpoints.payment_status.create!(
url: "http://coordinator.payment.url"
)
end
@@ -80,12 +80,8 @@ RSpec.describe Payments::WebhookService do
end
it "calls endpoint for all user managing the order cycle coordinator" do
user1.webhook_endpoints.create!(
url: "http://user1.payment.url", webhook_type: "payment_status_changed"
)
user2.webhook_endpoints.create!(
url: "http://user2.payment.url", webhook_type: "payment_status_changed"
)
user1.webhook_endpoints.payment_status.create!(url: "http://user1.payment.url")
user2.webhook_endpoints.payment_status.create!(url: "http://user2.payment.url")
expect{ subject }
.to enqueue_job(WebhookDeliveryJob)
@@ -98,12 +94,8 @@ RSpec.describe Payments::WebhookService do
context "wiht duplicate webhook endpoints configured" do
it "calls each unique configured endpoint" do
user1.webhook_endpoints.create!(
url: "http://coordinator.payment.url", webhook_type: "payment_status_changed"
)
user2.webhook_endpoints.create!(
url: "http://user2.payment.url", webhook_type: "payment_status_changed"
)
user1.webhook_endpoints.payment_status.create!(url: "http://coordinator.payment.url")
user2.webhook_endpoints.payment_status.create!(url: "http://user2.payment.url")
expect{ subject }
.to enqueue_job(WebhookDeliveryJob)