Skip to content

Commit 4e107e0

Browse files
authored
feat(products): catalog guards, dedup and batching (#5690)
## Context Hardening on top of the catalog services (#6098) and GraphQL surface (#6099): the immutability rules, the review fixes and the list-performance work. ## Description - Gate structural edits on attachment: a product, category or filter priced by a plan or subscription cannot change identity or be deleted (`attached_to_plan_or_subscription`); filter values freeze once a subscription bills through the filter (`attached_to_subscriptions`). - Creating a filter on a fixed product fails with `not_allowed_for_product_type`. - Reject duplicate filter value sets per product (`value_already_exist` on `values`) — two identical slices would be unresolvable for the billing engine; partial overlaps stay legal. - Batch `filters_count`, `products_count` and `attached_to_plan_or_subscription` through dataloader sources — one grouped query per page instead of per row.
1 parent 2b515ca commit 4e107e0

28 files changed

Lines changed: 636 additions & 36 deletions
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# frozen_string_literal: true
2+
3+
module Sources
4+
# Batches attached_to_plan_or_subscription? across catalog rows: one grouped
5+
# query per applied-card side instead of two EXISTS per row.
6+
#
7+
# Keyed by the grouping the caller resolves through — a product id for
8+
# products and their filters (a filter is attached when its product is), a
9+
# product_category id for categories (attached through their products).
10+
# Default scopes apply on both the applied cards and the joined models, so
11+
# discarded rows stay excluded exactly as in the per-record checks.
12+
#
13+
# Usage in GraphQL types:
14+
# dataloader.with(Sources::AttachedToPlanOrSubscription, :product).load(object.id)
15+
class AttachedToPlanOrSubscription < GraphQL::Dataloader::Source
16+
GROUPINGS = {
17+
product: [:rate_card, "rate_cards.product_id"],
18+
product_category: [{rate_card: :product}, "products.product_category_id"]
19+
}.freeze
20+
21+
def initialize(group_by)
22+
@joins, @column = GROUPINGS.fetch(group_by)
23+
end
24+
25+
def fetch(ids)
26+
attached = PlanRateCard.joins(@joins).where(@column => ids).distinct.pluck(Arel.sql(@column)) |
27+
SubscriptionRateCard.joins(@joins).where(@column => ids).distinct.pluck(Arel.sql(@column))
28+
29+
ids.map { attached.include?(it) }
30+
end
31+
end
32+
end
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# frozen_string_literal: true
2+
3+
module Sources
4+
# Batches per-row COUNT fields into a single grouped query.
5+
#
6+
# Prevents N+1 queries when a count is requested for several parent records
7+
# in one GraphQL query (e.g., `products { filtersCount }`). The model's
8+
# default scope applies, so soft-deleted rows stay excluded exactly as in
9+
# the per-record `association.count`.
10+
#
11+
# Usage in GraphQL types:
12+
# dataloader.with(Sources::CountByForeignKey, ProductFilter, :product_id).load(object.id)
13+
class CountByForeignKey < GraphQL::Dataloader::Source
14+
def initialize(model, foreign_key)
15+
@model = model
16+
@foreign_key = foreign_key
17+
end
18+
19+
def fetch(ids)
20+
# reorder(nil) drops any default-scope ordering, which would otherwise
21+
# leak into the GROUP BY and fail.
22+
counts = @model.where(@foreign_key => ids).reorder(nil).group(@foreign_key).count
23+
24+
ids.map { counts.fetch(it, 0) }
25+
end
26+
end
27+
end

app/graphql/types/product_categories/object.rb

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,18 @@ class Object < Types::BaseObject
1414
field :invoice_display_name, String, null: true
1515
field :name, String, null: false
1616

17+
field :attached_to_plan_or_subscription, Boolean, null: false
1718
field :products_count, Integer, null: false
1819

1920
field :created_at, GraphQL::Types::ISO8601DateTime, null: false
2021
field :updated_at, GraphQL::Types::ISO8601DateTime, null: false
2122

23+
def attached_to_plan_or_subscription
24+
dataloader.with(Sources::AttachedToPlanOrSubscription, :product_category).load(object.id)
25+
end
26+
2227
def products_count
23-
object.products.count
28+
dataloader.with(Sources::CountByForeignKey, Product, :product_category_id).load(object.id)
2429
end
2530
end
2631
end

app/graphql/types/product_categories/update_input.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ class UpdateInput < BaseInputObject
77

88
argument :id, ID, required: true
99

10+
argument :code, String, required: false
1011
argument :description, String, required: false
1112
argument :invoice_display_name, String, required: false
1213
argument :name, String, required: false

app/graphql/types/product_filters/object.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ class Object < Types::BaseObject
1111
field :id, ID, null: false
1212
field :organization, Types::Organizations::OrganizationType
1313

14+
field :attached_to_plan_or_subscription, Boolean, null: false
1415
field :code, String, null: false
1516
field :description, String, null: true
1617
field :invoice_display_name, String, null: true
@@ -21,6 +22,12 @@ class Object < Types::BaseObject
2122

2223
field :created_at, GraphQL::Types::ISO8601DateTime, null: false
2324
field :updated_at, GraphQL::Types::ISO8601DateTime, null: false
25+
26+
# A filter is attached when its product is — resolved by product_id so
27+
# the batch is shared with the products' own attachment checks.
28+
def attached_to_plan_or_subscription
29+
dataloader.with(Sources::AttachedToPlanOrSubscription, :product).load(object.product_id)
30+
end
2431
end
2532
end
2633
end

app/graphql/types/product_filters/update_input.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ class UpdateInput < BaseInputObject
77

88
argument :id, ID, required: true
99

10+
argument :code, String, required: false
1011
argument :description, String, required: false
1112
argument :invoice_display_name, String, required: false
1213
argument :name, String, required: false

app/graphql/types/products/object.rb

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ class Object < Types::BaseObject
1111
field :id, ID, null: false
1212
field :organization, Types::Organizations::OrganizationType
1313

14+
field :attached_to_plan_or_subscription, Boolean, null: false
1415
field :code, String, null: false
1516
field :description, String, null: true
1617
field :invoice_display_name, String, null: true
@@ -25,8 +26,12 @@ class Object < Types::BaseObject
2526
field :created_at, GraphQL::Types::ISO8601DateTime, null: false
2627
field :updated_at, GraphQL::Types::ISO8601DateTime, null: false
2728

29+
def attached_to_plan_or_subscription
30+
dataloader.with(Sources::AttachedToPlanOrSubscription, :product).load(object.id)
31+
end
32+
2833
def filters_count
29-
object.filters.count
34+
dataloader.with(Sources::CountByForeignKey, ProductFilter, :product_id).load(object.id)
3035
end
3136
end
3237
end

app/graphql/types/products/update_input.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@ class UpdateInput < BaseInputObject
77

88
argument :id, ID, required: true
99

10+
argument :code, String, required: false
1011
argument :description, String, required: false
1112
argument :invoice_display_name, String, required: false
1213
argument :name, String, required: false
14+
argument :product_category_id, ID, required: false
1315
end
1416
end
1517
end

app/models/product_filter.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,14 @@ class ProductFilter < ApplicationRecord
1111

1212
has_many :values, class_name: "ProductFilterValue"
1313
has_many :billable_metric_filters, through: :values
14+
has_many :rate_cards
1415

1516
delegate :attached_to_plan_or_subscription?, to: :product
1617

18+
def attached_to_subscriptions?
19+
SubscriptionRateCard.joins(:rate_card).where(rate_cards: {product_filter_id: id}).exists?
20+
end
21+
1722
validates :name, presence: true
1823
validates :code,
1924
presence: true,

app/services/product_categories/destroy_service.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ def initialize(product_category:)
1717
def call
1818
return result.not_found_failure!(resource: "product_category") unless product_category
1919

20+
if product_category.attached_to_plan_or_subscription?
21+
return result.single_validation_failure!(field: :product_category, error_code: "attached_to_plan_or_subscription")
22+
end
23+
2024
ActiveRecord::Base.transaction do
2125
product_category.products.find_each do |product|
2226
Products::DestroyService.call!(product:)

0 commit comments

Comments
 (0)