From d7129fda71638e084df23cf580e14fddce90ce29 Mon Sep 17 00:00:00 2001 From: mbianco-stripe <45374579+mbianco-stripe@users.noreply.github.com> Date: Mon, 22 Aug 2022 06:22:23 -0700 Subject: [PATCH] Prorated order amendments (#689) * Updated plan * idempotent notes * Break out prorated order amendment test * Plan update * Moving stripe price precision to a constant * Core proration use case tests * Initial working version of amendment helper logic * Memoized price helper * Only set sub term on products when we know it matches the billing frequency * Initial take at add_invoice_items generation * Improved price multiplier calculation * todo docs update * Allow error context to be called w/o state * Some more debugging functions from working with cloudflare * Calculate billing periods I have a feeling we don't need this because of the cotermed thing * Docs improvement * Test expansion for standard amendment tests * Refactoring CommonHelpers to namespace behind Critic * Test factory updates, including lots of Stripe factories * Using new stripe id generator * Fixing bad boolean in order amendment logic * Another round of expansions on the prorated order amendment logic * Create customer with card helper * Fixing price test comparison * Fix error context typing error * Typing fixes * Fixing error context references * Better logging on sync record creation * Adjust order failures for improved order logic * Fix sub term through quote error test * Gracefully handle terminations We can't prorate these! * Logs so we know something isn't stalled out * Centralize price equality logic, normalize decimal values * extract out backend proration test and temp skipping it * Another round of price equality fixes * Fix for multiple quantity test * Accept price param in metered billing * Add idempotency keys to cancellation & update calls * Use translate_product to get SF context * Users could map invalid field values, use raw error * Fix sync record tests * Typing fix --- TODO | 1 + lib/integrations/error_context.rb | 13 +- lib/stripe-force/constants.rb | 1 + lib/stripe-force/translate/order.rb | 124 +++++++-- .../translate/order/amendments.rb | 160 +++++++++++ .../translate/order/contract_item.rb | 16 ++ lib/stripe-force/translate/price.rb | 93 ++++--- lib/stripe-force/translate/price/helpers.rb | 67 ++++- lib/stripe-force/translate/sanitizer.rb | 3 +- lib/stripe-force/translate/translate.rb | 12 +- scripts/console.rb | 23 ++ sept_1.md | 30 +- test/integration/amendments/_lib.rb | 5 + .../integration/amendments/test_amendments.rb | 110 +++----- .../test_backend_prorated_amendments.rb | 35 +++ .../amendments/test_proration_amendments.rb | 260 ++++++++++++++++++ test/integration/test_order_failures.rb | 12 +- test/integration/test_sync_records.rb | 59 +++- test/integration/test_translate_order.rb | 15 +- .../translate/test_subscription_term.rb | 28 +- test/support/application_integration_test.rb | 6 +- test/support/common_helpers.rb | 58 ++-- test/support/functional_test.rb | 4 +- test/support/salesforce_factory.rb | 42 ++- test/support/stripe_factory.rb | 80 +++++- test/support/unit_test.rb | 3 +- test/test_helper.rb | 2 + test/unit/test_translator.rb | 6 +- 28 files changed, 979 insertions(+), 289 deletions(-) create mode 100644 lib/stripe-force/translate/order/amendments.rb create mode 100644 test/integration/amendments/test_backend_prorated_amendments.rb create mode 100644 test/integration/amendments/test_proration_amendments.rb diff --git a/TODO b/TODO index c73207645d..3d21016c39 100644 --- a/TODO +++ b/TODO @@ -22,6 +22,7 @@ - Prices which are duplicated because of the one-price-per-subscription) have a special metadata key (salesforce_duplicate = true) - Some good test clock docs https://groups.google.com/a/stripe.com/g/cloudflare/c/VjNW1Q4KD-0 - `stripe_proration` metadata key on proration-generated prices, also contains duplicate key, and auto archive +- fyi if the filter is changed we don't retroactively pick up orders. the orders have to be updated in some fashion (just updating a note/description will do just fine) for it to be reprocessed Next: diff --git a/lib/integrations/error_context.rb b/lib/integrations/error_context.rb index 9d57d2942e..771260d233 100644 --- a/lib/integrations/error_context.rb +++ b/lib/integrations/error_context.rb @@ -18,18 +18,25 @@ def report_exception(exception) end def report_edge_case(message, stripe_resource: nil, integration_record: nil, metadata: nil) + Integrations::ErrorContext.report_error(Integrations::Errors::UnhandledEdgeCase, message, stripe_resource: stripe_resource, integration_record: integration_record, metadata: metadata) + end + + def self.report_edge_case(message, stripe_resource: nil, integration_record: nil, metadata: nil) report_error(Integrations::Errors::UnhandledEdgeCase, message, stripe_resource: stripe_resource, integration_record: integration_record, metadata: metadata) end def report_feature_usage(message, stripe_resource: nil, integration_record: nil, metadata: nil) - report_error(Integrations::Errors::FeatureUsage, message, stripe_resource: stripe_resource, integration_record: integration_record, metadata: metadata) + Integrations::ErrorContext.report_error(Integrations::Errors::FeatureUsage, message, stripe_resource: stripe_resource, integration_record: integration_record, metadata: metadata) end sig { params(error_class: T.class_of(Integrations::Errors::BaseIntegrationError), message: String, stripe_resource: T.nilable(Stripe::StripeObject), integration_record: T.untyped, metadata: T.nilable(Hash)).returns(T.nilable(T.any(Sentry::Event, T::Boolean))) } - def report_error(error_class, message, stripe_resource: nil, integration_record: nil, metadata: nil) + def self.report_error(error_class, message, stripe_resource: nil, integration_record: nil, metadata: nil) sentry_options = {tags: metadata&.delete(:tags)}.compact - log.error message, {stripe_resource: stripe_resource, integration_record: integration_record}.compact.merge(metadata || {}) + log.error message, { + stripe_resource: stripe_resource, + integration_record: integration_record, + }.compact.merge(metadata || {}) exception = error_class.new( message, diff --git a/lib/stripe-force/constants.rb b/lib/stripe-force/constants.rb index e0ba4dcd35..6ce3760c9a 100644 --- a/lib/stripe-force/constants.rb +++ b/lib/stripe-force/constants.rb @@ -5,6 +5,7 @@ module StripeForce module Constants # application constants POLL_FREQUENCY = T.let(3 * 60, Integer) + MAX_STRIPE_PRICE_PRECISION = 12 SF_ORDER = 'Order' SF_ORDER_ITEM = 'OrderItem' diff --git a/lib/stripe-force/translate/order.rb b/lib/stripe-force/translate/order.rb index 542c5bb468..36e70c15da 100644 --- a/lib/stripe-force/translate/order.rb +++ b/lib/stripe-force/translate/order.rb @@ -101,6 +101,7 @@ def create_stripe_transaction_from_sf_order(sf_order) subscription_start_date = subscription_params['start_date'] subscription_params['start_date'] = StripeForce::Utilities::SalesforceUtil.salesforce_date_to_unix_timestamp(subscription_start_date) + # TODO should probably just use the end date here and centralize the calculations used on the order side of things # TODO this should really be done *before* generating the line items and therefore creating prices phase_iterations = transform_iterations_by_billing_frequency( # TODO is the restforce gem somehow formatting everything as a float? Or is this is the real value returned from SF? @@ -206,7 +207,7 @@ def build_phase_items_from_order_amendment(previous_phase_items, order_amendment [invoice_items_in_order, aggregate_phase_items] end - sig { params(contract_structure: ContractStructure).void } + sig { params(contract_structure: ContractStructure).returns(T.nilable(Stripe::SubscriptionSchedule)) } def update_subscription_phases_from_order_amendments(contract_structure) return if contract_structure.amendments.empty? @@ -283,23 +284,104 @@ def update_subscription_phases_from_order_amendments(contract_structure) # TODO should probably use a completely different key/mapping for the phase items phase_params = extract_salesforce_params!(sf_order_amendment, Stripe::SubscriptionSchedule) - phase_params['start_date'] = StripeForce::Utilities::SalesforceUtil.salesforce_date_to_unix_timestamp(phase_params['start_date']) + string_start_date_from_salesforce = phase_params['start_date'] + start_date_as_timestamp = StripeForce::Utilities::SalesforceUtil.salesforce_date_to_unix_timestamp(string_start_date_from_salesforce) + phase_params['start_date'] = start_date_as_timestamp # TODO check for float value + # TODO should probably move this to another helper subscription_term_from_sales_force = phase_params['iterations'].to_i + # originally `iterations` was used, but this fails when subscription term is less than a single billing cycle + phase_params['end_date'] = ( + DateTime.parse(string_start_date_from_salesforce).beginning_of_day.utc + + + subscription_term_from_sales_force.months + ).to_i - - # if the order is terminated this is not used - phase_params['iterations'] = transform_iterations_by_billing_frequency( - subscription_term_from_sales_force, - T.must(aggregate_phase_items.first).stripe_params[:price] - ) + # TODO should we validate the end date vs the subscription schedule? aggregate_phase_items = OrderHelpers.remove_terminated_lines(aggregate_phase_items) + # TODO validate that all prices have the same recurrence? Stripe does this downstream, + # but at this point we assume that this check has already done, so it may make sense + # to do this check more explicitly. + + # at this point, we have the finalized list of non-prorated order lines + # this means all price data has been mapped and converted into Stripe line items + # and we can calculate the finalized billing cycle of the order amendment + + invoice_items_for_prorations = [] + + if !is_order_terminated + billing_frequency = OrderAmendment.calculate_billing_frequency_from_phase_items(@user, aggregate_phase_items) + + # if the amendment is prorated, then all line items will have prorated component + is_prorated = OrderAmendment.prorated_amendment?( + user: @user, + aggregate_phase_items: aggregate_phase_items, + subscription_schedule: subscription_schedule, + + # these params in an ideal world should be pulled from the `subscription_schedule`, but + # they are tricky to extract without additional API calls + subscription_term: subscription_term_from_sales_force, + billing_frequency: billing_frequency, + amendment_start_date: start_date_as_timestamp + ) + end + + if !is_order_terminated && is_prorated + # TODO extract this out into another helper + aggregate_phase_items.each do |phase_item| + # TODO I don't like passing the user here, maybe pass in the user with the contract item? Going to see how ugly this feels... + if PriceHelpers.metered_price?(phase_item.price(@user)) + log.info 'metered price, not prorating', + prorated_order_item_id: phase_item.order_line_id, + price_id: phase_item.price + next + end + + if PriceHelpers.tiered_price?(phase_item.price) + log.info 'tiered price, not prorating', + prorated_order_item_id: phase_item.order_line_id, + price_id: phase_item.price + next + end + + if !PriceHelpers.recurring_price?(phase_item.price) + log.info 'one time price, not prorating', + prorated_order_item_id: phase_item.order_line_id, + price_id: phase_item.price + next + end + + # we only want to prorate the items that are unique to this order + if !phase_item.from_order?(sf_order_amendment) + log.info 'line item not originated from this amendment, not prorating', + prorated_order_item_id: phase_item.order_line_id, + price_id: phase_item.price.id + next + end + + log.info 'prorating order item', prorated_order_item_id: phase_item.order_line_id + + proration_price = OrderAmendment.create_prorated_price_from_phase_item( + user: @user, + phase_item: phase_item, + subscription_term: subscription_term_from_sales_force, + billing_frequency: billing_frequency + ) + + invoice_items_for_prorations << { + quantity: phase_item.quantity, + price: proration_price.id, + # TODO metadata + # TODO proration hash + } + end + end + new_phase = Stripe::StripeObject.construct_from({ - add_invoice_items: invoice_items_in_order.map(&:stripe_params), + add_invoice_items: invoice_items_in_order.map(&:stripe_params) + invoice_items_for_prorations, # this is important, otherwise multiple phase changes in a single job run will use the same aggregate phase items items: aggregate_phase_items.deep_dup.map(&:stripe_params), @@ -334,20 +416,19 @@ def update_subscription_phases_from_order_amendments(contract_structure) # NOTE intentional decision here NOT to update any other subscription fields catch_errors_with_salesforce_context(secondary: sf_order_amendment) do if is_subscription_schedule_cancelled - # TODO should we add additional metadata here? - log.info 'cancelling subscription immediately' + log.info 'cancelling subscription immediately', sf_order_amendment_id: sf_order_amendment - subscription_schedule.cancel( + # NOTE the intention here is to void/reverse out the entire contract, this is the closest API call we have + subscription_schedule.cancel({ invoice_now: false, - prorate: false - ) + prorate: false, + }, generate_idempotency_key_with_credentials(@user, sf_order_amendment, :cancel)) else log.info 'adding phase', sf_order_amendment_id: sf_order_amendment.Id - # TODO wrap in error context subscription_schedule.proration_behavior = 'none' subscription_schedule.phases = subscription_phases - subscription_schedule.save + subscription_schedule.save({}, generate_idempotency_key_with_credentials(@user, sf_order_amendment)) end end @@ -355,6 +436,8 @@ def update_subscription_phases_from_order_amendments(contract_structure) end PriceHelpers.auto_archive_prices_on_subscription_schedule(@user, subscription_schedule) + + subscription_schedule end sig do @@ -367,6 +450,7 @@ def merge_subscription_line_items(original_aggregate_phase_items, new_phase_item # avoid mutating the input value aggregate_phase_items = original_aggregate_phase_items.dup + # TODO `termination_lines` here is what we need for credit calculation termination_lines, additive_lines = new_phase_items.partition(&:termination?) additive_lines.each do |new_subscription_item| @@ -411,7 +495,8 @@ def terminate_subscription_line_items(original_aggregate_phase_items, terminatio end end - log.debug "order amendment revision map", revision_map: revision_map + log.debug "order amendment revision map", + revision_map: revision_map.transform_values {|ci| ci.map(&:order_line_id) } # now let's terminate the related line items termination_lines.each do |termination_line| @@ -490,7 +575,9 @@ def phase_items_from_order_lines(sf_order_lines) subscription_items = [] sf_order_lines.map do |sf_order_item| - price = create_price_for_order_item(sf_order_item) + price = catch_errors_with_salesforce_context(secondary: sf_order_item) do + create_price_for_order_item(sf_order_item) + end # could occur if a coupon is required for a negative amount, although this should probably be built into the `price` method instead next if price.nil? @@ -682,6 +769,7 @@ def extract_initial_order_from_amendment(sf_order_amendment) log.info 'found initial order', initial_order_id: initial_order_id + # TODO use cache_service sf.find(SF_ORDER, initial_order_id) end diff --git a/lib/stripe-force/translate/order/amendments.rb b/lib/stripe-force/translate/order/amendments.rb new file mode 100644 index 0000000000..0d60e1ea7a --- /dev/null +++ b/lib/stripe-force/translate/order/amendments.rb @@ -0,0 +1,160 @@ +# frozen_string_literal: true +# typed: true + +# using `Order::Amendment` conflicts will too many other things +class StripeForce::Translate + module OrderAmendment + extend T::Sig + + include Kernel + include StripeForce::Constants + extend SimpleStructuredLogger + + # for now, let's not support non-co-terminated contracts + sig { params(contract_structure: ContractStructure).returns(T::Boolean) } + def self.contract_co_terminated?(contract_structure) + target_end_date = calculate_order_end_date(contract_structure.initial) + + contract_structure.amendments.all? do |sf_order| + calculate_order_end_date(sf_order) == target_end_date + end + end + + def self.calculate_order_end_date(sf_order) + # get start date + # add subscription term + end + + # NOTE at this point it's assumed that the price is NOT a metered billing item or tiered price + def self.create_prorated_price_from_phase_item(user:, phase_item:, subscription_term:, billing_frequency:) + # at this point, we know what the billing amount is per billing cycle, since that has alread been defined + # on the price object at the order line. We calculate the percentage of the original line price that should + # be billed and multiply that against the decimal price on the stripe price. + + # TODO validate that it isn't a tiered or metered billing price + + # TODO we'll need to have some sort of logic for backend prorations to calculate the amount before + # a billing cycle that needs to be billed for, but let's deal with that later... + prorated_subscription_term = subscription_term % billing_frequency + proration_percentage = BigDecimal(prorated_subscription_term) / BigDecimal(billing_frequency) + stripe_price = phase_item.price(@user) + unit_amount_decimal = BigDecimal(stripe_price.unit_amount_decimal) + prorated_billing_amount = unit_amount_decimal * proration_percentage + + proration_price = OrderHelpers.duplicate_stripe_price(user, stripe_price) do |duplicated_stripe_price| + duplicated_stripe_price.metadata[StripeForce::Utilities::Metadata.metadata_key(user, MetadataKeys::PRORATION_PRICE)] = true + # since we are explicitly doing pricing math here, we should define the max + duplicated_stripe_price.unit_amount_decimal = prorated_billing_amount.to_s("#{MAX_STRIPE_PRICE_PRECISION}F") + + # this price should be a one-time price, this must be done by removing the recurring field + Integrations::Utilities::StripeUtil.delete_field_from_stripe_object( + duplicated_stripe_price, + :recurring + ) + + # TODO we should generate some sort of custom description here, but there's no place to put it + end + + log.info 'proration price created', stripe_proration_id: proration_price.id + + proration_price + end + + sig { params(user: StripeForce::User, aggregate_phase_items: T::Array[ContractItemStructure]).returns(Integer) } + def self.calculate_billing_frequency_from_phase_items(user, aggregate_phase_items) + # TODO maybe we add this stuff to the contract stucture? Feels weird to calculate it way down here + # NOTE right now, all prices on an object must have the exact same billing cycle, this may change in the future + stripe_price_id = T.must(aggregate_phase_items.first).stripe_params[:price] + stripe_price = Stripe::Price.retrieve(stripe_price_id, user.stripe_credentials) + billing_frequency_in_months = StripeForce::Utilities::StripeUtil.billing_frequency_of_price_in_months(stripe_price) + billing_frequency_in_months + end + + # all dates returned are middate utc + sig { params(user: StripeForce::User, original_subscription_schedule: Stripe::SubscriptionSchedule, billing_frequency: Integer).returns(T::Array[DateTime]) } + def self.calculate_billing_cycle_dates(user, original_subscription_schedule, billing_frequency) + subscription_id = T.cast(original_subscription_schedule.subscription, String) + + upcoming_invoice = Stripe::Invoice.upcoming({ + subscription: subscription_id, + }, user.stripe_credentials) + + # all timestamps are in utc + next_billing_timestamp = upcoming_invoice.created + + # we don't know the state of the subscription schedule when it is passed in + # it could be mutated by upstream logic and not yet passed to stripe + # this is important because we need the end date of the subscription in order to know + # when to stop calculating the next future billing dates. Since we assume all contracts + # co-term, we can safely rely on the end date of the last phase being the date we need to calculate to. + # in order to make sure the sub schedule has the correct ending date, we need to pull it again. + subscription_schedule = Stripe::SubscriptionSchedule.retrieve(original_subscription_schedule.id, user.stripe_credentials) + last_phase = T.must(subscription_schedule.phases.last) + final_billing_timestamp = T.must(last_phase.end_date) + + # TODO file a JIRA against the billing team to fix this! + # NOTE this is going to be a very risky operation, but there's nothing we can do right now + # add the billing_frequency until we are past the last billing date + + next_billing_date = Time.at(next_billing_timestamp).utc.to_datetime + future_billing_dates = [next_billing_timestamp] + + billing_date = next_billing_date + while billing_date.to_i <= final_billing_timestamp + billing_date += billing_frequency.months + future_billing_dates << billing_date.to_i + end + + future_billing_dates + end + + sig do + params( + user: StripeForce::User, + aggregate_phase_items: T::Array[ContractItemStructure], + subscription_schedule: Stripe::SubscriptionSchedule, + subscription_term: Integer, + billing_frequency: Integer, + # as unix timestamp + amendment_start_date: Integer + ).returns(T::Boolean) + end + def self.prorated_amendment?(user:, aggregate_phase_items:, subscription_schedule:, subscription_term:, billing_frequency:, amendment_start_date:) + if aggregate_phase_items.empty? + log.info 'no subscription items, cannot be a proration' + return false + end + + if aggregate_phase_items.all?(&:new_order_line?) + log.info "all order lines are new, cannot be prorated order" + return false + end + + # if the subscription term does not match the billing frequency of the stripe item, then there will be some proration + if (subscription_term % billing_frequency) != 0 + log.info 'billing frequency is not divisible by subscription term, assuming proration', + subscription_term: subscription_term, + billing_frequency: billing_frequency + return true + end + + # is the next billing date aligned with the start of the subscription? if not, there will be prorations + billing_dates = calculate_billing_cycle_dates(user, subscription_schedule, billing_frequency) + + # we only care about the date, not the time + # TODO we need to be very careful about date comparison, there's a good chance a nuance here will cause us issues + if billing_dates.none? {|d| d == amendment_start_date } + # TODO need to do more thinking here, but I think this specific case may be impossible since we are enforcing coterm + # let's track this and then possibly remove this codepath in the future + Integrations::ErrorContext.report_edge_case("start date is not on the next or future billing dates") + + log.info 'start date is not on the next or future billing dates', + amendment_start_date: amendment_start_date, + billing_dates: billing_dates + return true + end + + false + end + end +end diff --git a/lib/stripe-force/translate/order/contract_item.rb b/lib/stripe-force/translate/order/contract_item.rb index 9e033db6a7..053b44be39 100644 --- a/lib/stripe-force/translate/order/contract_item.rb +++ b/lib/stripe-force/translate/order/contract_item.rb @@ -38,6 +38,17 @@ def self.from_order_line_and_params(sf_order_line, stripe_params) ) end + sig { params(user: T.nilable(StripeForce::User)).returns(Stripe::Price) } + def price(user=nil) + if @price.nil? && user.nil? + raise Integrations::Errors::ImpossibleInternalError.new("query should never return an empty result") + end + + # sorbet doesn't like instance memoized instance variables + @price ||= Stripe::Price.retrieve(self.stripe_params[:price], T.unsafe(user).stripe_credentials) + @price + end + def reduce_quantity if self.quantity < 0 raise "termination lines should never be reduced" @@ -61,5 +72,10 @@ def fully_terminated? def new_order_line? revised_order_line_id.blank? end + + sig { params(sf_order: Restforce::SObject).returns(T::Boolean) } + def from_order?(sf_order) + self.order_line['OrderId'] == sf_order.Id + end end end diff --git a/lib/stripe-force/translate/price.rb b/lib/stripe-force/translate/price.rb index 8ee736e2ea..5218fb8281 100644 --- a/lib/stripe-force/translate/price.rb +++ b/lib/stripe-force/translate/price.rb @@ -25,38 +25,22 @@ def create_price_from_pricebook(sf_pricebook_entry) stripe_price end - # TODO maybe look at "can edit price" boolean on the product? But what about custom mappings? - # TODO https://jira.corp.stripe.com/browse/PLATINT-1485 def pricebook_and_order_line_identical?(sf_pricebook_entry, sf_order_item, sf_product) # generate the full params that would be sent to the price object and compare them - # if they aren't *exactly* the same, this will trigger a new price to be created - # we'll probably need to do something a bit smarter here in the future but it's not obvious what - # additional logic we should include here at the moment. - pricebook_params = generate_price_params_from_sf_object(sf_pricebook_entry, sf_product).to_hash - order_item_params = generate_price_params_from_sf_object(sf_order_item, sf_product).to_hash - - # metadata *could* have important financial data for downstream systems - # however, most of the time users will not pull this information directly from prices - # (they may use products instead) and a users mappings could change over time, so we don't - # want to factor it in to our equality test - pricebook_params.delete(:metadata) - order_item_params.delete(:metadata) - - # creating prices unnecessarily can be problematic for users: many prices without clarity about why they exist - # for this reason, let's log the diff in debug mode so we can easily determine exactly WHY the new price was created - if pricebook_params != order_item_params - log.debug 'price diff', diff: HashDiff::Comparison.new(pricebook_params, order_item_params).diff - end + # if they aren't *exactly* the same, this will trigger a new price to be created. + pricebook_params = generate_price_params_from_sf_object(sf_pricebook_entry, sf_product) + order_item_params = generate_price_params_from_sf_object(sf_order_item, sf_product) - pricebook_params == order_item_params + PriceHelpers.price_billing_amounts_equal?(pricebook_params, order_item_params) end + # main entry point to creating prices from line items from the order logic # TODO what if the list price is updated in SF? We shouldn't probably create a new price object def create_price_for_order_item(sf_order_item) log.info 'translating price from a order line origin', salesforce_object: sf_order_item sf_product = sf.find(SF_PRODUCT, sf_order_item.Product2Id) - product = create_product_from_sf_product(sf_product) + product = translate_product(sf_product) sf_pricebook_entry = sf.find(SF_PRICEBOOK_ENTRY, sf_order_item.PricebookEntryId) @@ -398,39 +382,62 @@ def generate_price_params_from_sf_object(sf_object, sf_product) # stripe_price.recurring = {} end - # we should only transform licensed prices that are not customized - if is_recurring_item && - !is_tiered_price && - sf_object.sobject_type == SF_ORDER_ITEM && - !PriceHelpers.using_custom_order_line_price_field?(@user) + # prices are only transformed when they are tied to order lines, we trust pricebook prices as is + # only licensed prices should be customized, tiered + metered billing prices should be left as is + is_recurring_licensed_price_from_order_line = is_recurring_item && + !is_tiered_price && + sf_object.sobject_type == SF_ORDER_ITEM + + if is_recurring_licensed_price_from_order_line && !PriceHelpers.using_custom_order_line_price_field?(@user) + # the formula for calculating the adjusted price is: + # billing price = order line unit price / quantity / (subscription term / billing frequency) log.info 'custom price not used, adjusting unit_amount_decimal', sf_order_item_id: sf_object.Id billing_frequency = StripeForce::Utilities::StripeUtil.billing_frequency_of_price_in_months(stripe_price) - # TODO this is a hack and we should really extract this through the extract params - quote_subscription_term_path = 'Order.SBQQ__Quote__c.SBQQ__SubscriptionTerm__c' - quote_subscription_term = mapper.extract_key_path_for_record(sf_object, quote_subscription_term_path) - - if quote_subscription_term.nil? - raise Integrations::Errors::MissingRequiredFields.new( - salesforce_object: sf_object, - missing_salesforce_fields: [quote_subscription_term_path] - ) - end + # TODO use cache_service + sf_order = sf.find(SF_ORDER, sf_object['OrderId']) - # it's looking like these values are never really aligned and we should ignore the line item - if sf_object['SBQQ__SubscriptionTerm__c'] == quote_subscription_term - report_edge_case("subscription term on quote matches line item") - end + subscription_term = extract_subscription_term_from_order!(sf_order) + price_multiplier = BigDecimal(subscription_term) / BigDecimal(billing_frequency) + # TODO should we adjust based on the quantity? Most likely, let's wait until tests fail - # TODO need to stop hardcoding this value! # NOTE in most cases, this value should be the same as `sf_object['SBQQ__ProrateMultiplier__c']` if the user has product-level subscription term enabled - price_multiplier = determine_subscription_term_multiplier_for_billing_frequency(quote_subscription_term, billing_frequency) + # price_multiplier = determine_subscription_term_multiplier_for_billing_frequency(quote_subscription_term, billing_frequency) stripe_price.unit_amount_decimal = T.cast(stripe_price.unit_amount_decimal, BigDecimal) / price_multiplier end stripe_price end + + # ideally this would not be an instance method, but having the `mapper` state will reduce API calls and simplify the logic here for now + sig { params(sf_order: Restforce::SObject).returns(Integer) } + def extract_subscription_term_from_order!(sf_order) + subscription_term_stripe_path = ['subscription_schedule', 'iterations'] + subscription_term_order_path = @user.field_mappings.dig(*subscription_term_stripe_path) || + @user.required_mappings.dig(*subscription_term_stripe_path) + + # quote_subscription_term = T.cast(mapper.extract_key_path_for_record(sf_order, subscription_term_order_path), T.nilable(String)) + quote_subscription_term = T.cast(mapper.extract_key_path_for_record(sf_order, subscription_term_order_path), T.nilable(T.any(String, Float))) + + if quote_subscription_term.nil? + raise Integrations::Errors::MissingRequiredFields.new( + salesforce_object: sf_order, + missing_salesforce_fields: [subscription_term_order_path] + ) + end + + # it's looking like these values are never really aligned and we should ignore the line item + if sf_order['SBQQ__SubscriptionTerm__c'] == quote_subscription_term + report_edge_case("subscription term on quote matches line item") + end + + if !Integrations::Utilities::StripeUtil.is_integer_value?(quote_subscription_term) + raise StripeForce::Errors::RawUserError.new("Subscription term is specified as a decimal value") + end + + quote_subscription_term.to_i + end end diff --git a/lib/stripe-force/translate/price/helpers.rb b/lib/stripe-force/translate/price/helpers.rb index 8b55275ba7..99bb9e7ce8 100644 --- a/lib/stripe-force/translate/price/helpers.rb +++ b/lib/stripe-force/translate/price/helpers.rb @@ -120,15 +120,72 @@ def self.transform_salesforce_billing_type_to_usage_type(raw_usage_type) end end - # TODO right now, this is NOT used everywhere there is a price comparison, we should leverage this more broadly across the codebase sig { params(price_1: Stripe::Price, price_2: Stripe::Price).returns(T::Boolean) } def self.price_billing_amounts_equal?(price_1, price_2) - # TODO in some cases the `unit_amount_decimal` is already a bigd, we should handle this more cleanly - billing_amounts_equal = BigDecimal(price_1.unit_amount_decimal.to_s) == BigDecimal(price_2.unit_amount_decimal.to_s) && + # metadata *could* have important financial data for downstream systems + # however, most of the time users will not pull this information directly from prices + # (they may use products instead) and a users mappings could change over time, so we don't + # want to factor it in to our equality test. If we did, prices would never match and it would + # create a massive amount of price objects in their Stripe account. + + # these values could be nil, which is fine as long as they are both nil + # if they are not both nil (comparing an existing stripe price to a to-be-created price) + # we need to relax the checks a bit: the comparisons should below should infer the correct + # pricing types (metered vs licensed) + + fields_to_check_if_both_are_set = %i{ + product + tax_behavior + billing_scheme + type + } + + simple_field_check_passed = fields_to_check_if_both_are_set.all? do |field_sym| + # if one of the fields is set, skip the comparison + price_1[field_sym].blank? != price_2[field_sym].blank? || + price_1[field_sym] == price_2[field_sym] + end + + if !simple_field_check_passed + log.info 'price not equal, simple field comparison check failed', diff: HashDiff::Comparison.new(price_1.to_hash, price_2.to_hash).diff + return false + end + + is_price_equal = if price_1[:billing_scheme].nil? || price_1.billing_scheme == 'per_unit' + # TODO we do not expect this occur, if it does we'll need to improve the error message here + if price_1.unit_amount_decimal.nil? || price_2.unit_amount_decimal.nil? + raise StripeForce::Errors::RawUserError.new("unit_amount_decimal nil on price objects") + end + + price_1_decimal = price_1.unit_amount_decimal + if !price_1_decimal.is_a?(BigDecimal) + price_1_decimal = BigDecimal(price_1_decimal) + end + price_1_decimal = price_1_decimal.round(MAX_STRIPE_PRICE_PRECISION) + + price_2_decimal = price_2.unit_amount_decimal + if !price_2_decimal.is_a?(BigDecimal) + price_2_decimal = BigDecimal(price_2_decimal) + end + price_2_decimal = price_2_decimal.round(MAX_STRIPE_PRICE_PRECISION) + + price_1_decimal == price_2_decimal + elsif price_1.billing_scheme == 'tiered' + # TODO we will probably need to do some sort of normalization here on tiering amounts + # TODO probably need to think through transformed_quantity here and if it could effect this + price_1.tiers == price_2.tiers && price_1.tiers_mode == price_2.tiers_mode + else + raise StripeForce::Errors::RawUserError.new("Unexpected billing_scheme on price encountered #{price_1.billing_scheme}") + end + + billing_amounts_equal = is_price_equal && + price_1.currency == price_2.currency && price_1.recurring.present? == price_2.recurring.present? && - price_1.recurring.interval == price_2.recurring[:interval] && - price_1.recurring.interval_count == price_2.recurring[:interval_count] + price_1.recurring[:interval] == price_2.recurring[:interval] && + price_1.recurring[:interval_count] == price_2.recurring[:interval_count] + # creating prices unnecessarily can be problematic for users: many prices without clarity about why they exist + # for this reason, let's log the diff in debug mode so we can easily determine exactly WHY the new price was created if !billing_amounts_equal log.info 'price not equal', diff: HashDiff::Comparison.new(price_1.to_hash, price_2.to_hash).diff end diff --git a/lib/stripe-force/translate/sanitizer.rb b/lib/stripe-force/translate/sanitizer.rb index 8e56603697..7e212b4265 100644 --- a/lib/stripe-force/translate/sanitizer.rb +++ b/lib/stripe-force/translate/sanitizer.rb @@ -5,6 +5,7 @@ module StripeForce class Sanitizer extend T::Sig include Integrations::Log + include StripeForce::Constants sig { params(user: StripeForce::User).void } def initialize(user) @@ -24,7 +25,7 @@ def sanitize(stripe_record) private def sanitize_price(stripe_price) if stripe_price[:unit_amount_decimal] && !Integrations::Utilities::StripeUtil.is_integer_value?(stripe_price[:unit_amount_decimal]) # Stripe only supports 12 digits - stripe_price[:unit_amount_decimal] = stripe_price[:unit_amount_decimal].round(12) + stripe_price[:unit_amount_decimal] = stripe_price[:unit_amount_decimal].round(MAX_STRIPE_PRICE_PRECISION) end end diff --git a/lib/stripe-force/translate/translate.rb b/lib/stripe-force/translate/translate.rb index 01f80965d1..80a89f03cb 100644 --- a/lib/stripe-force/translate/translate.rb +++ b/lib/stripe-force/translate/translate.rb @@ -179,12 +179,12 @@ def create_user_failure(salesforce_object:, message:) compound_external_id = generate_compound_external_id(@origin_salesforce_object, salesforce_object) - log.debug 'creating sync record for failure' + log.debug 'creating sync record for failure', external_id: compound_external_id # interestingly enough, if the external ID field does not exist we'll get a NOT_FOUND response # https://developer.salesforce.com/docs/atlas.en-us.api_rest.meta/api_rest/dome_upsert.htm - sf.upsert!( + sf_sync_record_id = sf.upsert!( prefixed_stripe_field(SYNC_RECORD), prefixed_stripe_field(SyncRecordFields::COMPOUND_ID.serialize), { @@ -200,6 +200,8 @@ def create_user_failure(salesforce_object:, message:) SyncRecordFields::RESOLUTION_STATUS => SyncRecordResolutionStatuses::ERROR, }.transform_keys(&:serialize).transform_keys(&method(:prefixed_stripe_field)) ) + + log.debug 'sync record created', sync_record_id: sf_sync_record_id end sig { params(salesforce_object: Restforce::SObject, stripe_object: Stripe::APIResource).void } @@ -370,7 +372,7 @@ def retrieve_from_stripe(stripe_class, sf_object) end end - # TODO this should be dynamic and pulled from the mapper + # TODO this should be dynamic and pulled from the mapper https://jira.corp.stripe.com/browse/PLATINT-1485 # according to the salesforce documentation, if this field is non-nil ("empty") than it's a subscription item def recurring_item?(sf_object) if ![SF_ORDER_ITEM, SF_PRODUCT].include?(sf_object.sobject_type) @@ -381,9 +383,11 @@ def recurring_item?(sf_object) !sf_object[CPQ_QUOTE_SUBSCRIPTION_PRICING].nil? end - # service period and billing frequency are decoupled in CPQ + # service period and billing frequency are decoupled in CPQ, but they are the same in Stripe # both values should be in months, but we want to support days in the future def determine_subscription_term_multiplier_for_billing_frequency(subscription_term, billing_frequency) + # billing price = order line unit price / quantity / (subscription term / billing frequency) + # if specified iterations is less than the billing frequency of the stripe price then if subscription_term < billing_frequency # TODO we should probably create a invoice item price instead? Unsure of the best approach here, we would want the invoice item to be tied to a product? diff --git a/scripts/console.rb b/scripts/console.rb index fa2a4ec551..4b540cafc5 100755 --- a/scripts/console.rb +++ b/scripts/console.rb @@ -138,6 +138,29 @@ def activate_pricebooks end end +def touch_order(sf_order) + if sf_order.Description.present? + raise "description is not empty" + end + + @sf.update!(SF_ORDER, { + SF_ID => sf_order.Id, + "Description" => "stripe-force: #{Time.now.to_i}" + }) +end + +def ensure_order_is_included_in_custom_where_clause(sf_order) + order_poller = StripeForce::OrderPoller.new(@user) + custom_soql = order_poller.send(:user_specified_where_clause_for_object) + results = @sf.query("SELECT Id FROM #{SF_ORDER} WHERE Id = '#{sf_order.Id}' " + custom_soql) + + if results.first.empty? + puts "Order is not included in custom soql" + else + puts "Order is included in custom soql" + end +end + require_relative '../test/support/salesforce_debugging' include SalesforceDebugging diff --git a/sept_1.md b/sept_1.md index b18abb31e3..a1ade6a798 100644 --- a/sept_1.md +++ b/sept_1.md @@ -3,7 +3,6 @@ - [x] Refactor existing amendment work to make it easier to modify for the new structure - [x] Duplicate price if duplicate price ID exists on the subscription - We want to use the same price as much as we can, but we should't optimize for this -- [ ] Fill in test templates to offer more feedback when refactoring ### Order Amendment: @@ -14,14 +13,29 @@ - Enforce LIFO queue on quantity termination - [x] Ensure all metered billing items only have a quantity of 1 - [x] Conditionally use collasping system, only on termination -- Update & expand all tests -- Need to update lots of docs, although this can be punted (much harder to do later though...) +- [ ] Need to update lots of docs, although this can be punted (much harder to do later though...) ### Prorated Order Amendments -- Think on "backend" prorations which is not something we thought about +- [x] extract out one-time invoicing into separate method +- [ ] Refactor to pull out order amendment helpers into separate module +- [x] Helper to detect if order is prorated + - If there are all new line items on the order, then it doesn't matter + - If the order start date does NOT align with the next billing date, then it's definitely prorated + - If the order start date does align, does the subscription term align with the billing cycle +- [x] Will CF always have co-termed amendments? + - Yes, sounds like this may be a CPQ thing + - [ ] enforce this in code +- [x] Split unit price on line to get bill cycle amount +- [ ] Use `end_date` vs iterations in initial phase and rip out associated logic +- [ ] Improve custom price field detection to be more reliable +- [ ] Think on "backend" prorations which is not something we thought about - We can create two phases on the initial subscription -- Sync w/CF on custom price field and if it's needed -- Split unit price on line to get bill cycle amount -- Create invoice based on webhook -- Test new API endpoints from billing + - Validate bug we encountered when prototyping +- [x] Sync w/CF on custom price field and if it's needed + - They need this and are set on using it +- [x] Accept webhooks +- [x] Test calculating normal billing cycle price when order line is prorated +- [ ] Create invoice based on webhook +- [ ] Test new API endpoints from billing +- [ ] Test clock based tests to validate things are working properly diff --git a/test/integration/amendments/_lib.rb b/test/integration/amendments/_lib.rb index 013aa0f617..2e93e2940f 100644 --- a/test/integration/amendments/_lib.rb +++ b/test/integration/amendments/_lib.rb @@ -5,6 +5,8 @@ class Critic::OrderAmendmentFunctionalTest < Critic::FunctionalTest def create_contract_from_order(sf_order) + log.info 'creating contract' + sf.update!(SF_ORDER, { SF_ID => sf_order.Id, SF_ORDER_CONTRACTED => true, @@ -27,10 +29,13 @@ def create_contract_from_order(sf_order) # this API is tricky: requires empty JSON object and Content-Type set correctly def create_quote_data_from_contract_amendment(sf_contract) + log.info 'creating amendment quote' JSON.parse(sf.patch("services/apexrest/SBQQ/ServiceRouter?loader=SBQQ.ContractManipulationAPI.ContractAmender&uid=#{sf_contract.Id}", {}, {"Content-Type" => "application/json"}).body) end def create_order_from_quote_data(sf_quote_data) + log.info 'creating order from quote' + sf_quote_id = calculate_and_save_cpq_quote(sf_quote_data) # the amendment quote process doesn't seem to pick a pricebook, so we need to manually do this diff --git a/test/integration/amendments/test_amendments.rb b/test/integration/amendments/test_amendments.rb index 11e8531059..f63b482a17 100644 --- a/test/integration/amendments/test_amendments.rb +++ b/test/integration/amendments/test_amendments.rb @@ -9,7 +9,18 @@ class Critic::OrderAmendmentTranslation < Critic::OrderAmendmentFunctionalTest end it 'creates a new phase from an order amendment with monthly billed products' do - sf_order = create_subscription_order + # initial order: 1yr contract, monthly billed + # amendment: starts month 9, lasts 3 months, adds quantity 2 + + monthly_price = 10_00 + contract_term = 12 + amendment_term = 3 + start_date = now_time + (contract_term - amendment_term).months + end_date = start_date + amendment_term.months + initial_start_date = now_time + + sf_product_id, sf_pricebook_id = salesforce_recurring_product_with_price(price: monthly_price) + sf_order = create_subscription_order(sf_product_id: sf_product_id) sf_contract = create_contract_from_order(sf_order) # although the associated order is contracted, this does not @@ -27,17 +38,11 @@ class Critic::OrderAmendmentTranslation < Critic::OrderAmendmentFunctionalTest amendment_data = create_quote_data_from_contract_amendment(sf_contract) - # increase quantity + # increase quantity by 2 amendment_data["lineItems"].first["record"]["SBQQ__Quantity__c"] = 3 - # the quote is generated by the contract CPQ API, so we need to set these fields manually - # let's have the second phase start in 9mo - start_date = now_time + 9.months - end_date = start_date + 3.months - initial_start_date = now_time - - amendment_data["record"][CPQ_QUOTE_SUBSCRIPTION_START_DATE] = start_date.strftime("%Y-%m-%d") - amendment_data["record"][CPQ_QUOTE_SUBSCRIPTION_TERM] = 3 + amendment_data["record"][CPQ_QUOTE_SUBSCRIPTION_START_DATE] = format_date_for_salesforce(start_date) + amendment_data["record"][CPQ_QUOTE_SUBSCRIPTION_TERM] = amendment_term sf_order_amendment = create_order_from_quote_data(amendment_data) @@ -63,7 +68,6 @@ class Critic::OrderAmendmentTranslation < Critic::OrderAmendmentFunctionalTest sf_order.refresh stripe_id = sf_order[prefixed_stripe_field(GENERIC_STRIPE_ID)] - subscription_schedule = Stripe::SubscriptionSchedule.retrieve(stripe_id, @user.stripe_credentials) assert_equal(2, subscription_schedule.phases.count) @@ -100,6 +104,18 @@ class Critic::OrderAmendmentTranslation < Critic::OrderAmendmentFunctionalTest refute_equal(first_phase_item.price, second_phase_item_2.price) assert_equal(first_phase_item.metadata['salesforce_order_item_id'], second_phase_item_1.metadata['salesforce_order_item_id']) refute_equal(first_phase_item.metadata['salesforce_order_item_id'], second_phase_item_2.metadata['salesforce_order_item_id']) + + # let's be paranoid here and make sure the price is exactly what we expect + phase_price = Stripe::Price.retrieve(T.cast(first_phase_item.price, String), @user.stripe_credentials) + assert_equal(monthly_price.to_s, phase_price.unit_amount_decimal) + assert_equal('recurring', phase_price.type) + assert(phase_price.active) + + # enforces none of the duplicate/auto-archive/etc metadata is in place + phase_price_metadata = phase_price.metadata.to_hash + phase_price_metadata.delete(:salesforce_pricebook_entry_id) + phase_price_metadata.delete(:salesforce_pricebook_entry_link) + assert_empty(phase_price_metadata) end # usage products do NOT have a quantity in Stripe, which introduces additional complexity @@ -172,7 +188,6 @@ class Critic::OrderAmendmentTranslation < Critic::OrderAmendmentFunctionalTest sf_product_id, sf_pricebook_id = salesforce_recurring_product_with_price( additional_product_fields: { CPQ_QUOTE_BILLING_FREQUENCY => CPQBillingFrequencyOptions::ANNUAL.serialize, - CPQ_QUOTE_SUBSCRIPTION_TERM => nil, } ) @@ -233,73 +248,6 @@ class Critic::OrderAmendmentTranslation < Critic::OrderAmendmentFunctionalTest assert_equal(first_phase_item.price, second_phase_item_single_quantity.price) end - it 'creates a new phase with a duration shorter than the billing frequency' do - skip("full order proration support is needed here since iterations will be 1, which will extend the contract longer than it should") - - start_date = now_time + 18.months - amendment_term = 6 - end_date = start_date + amendment_term.months - initial_start_date = now_time - - sf_product_id, sf_pricebook_id = salesforce_recurring_product_with_price( - additional_product_fields: { - CPQ_QUOTE_BILLING_FREQUENCY => CPQBillingFrequencyOptions::ANNUAL.serialize, - } - ) - - sf_order = create_salesforce_order( - sf_product_id: sf_product_id, - additional_quote_fields: { - CPQ_QUOTE_SUBSCRIPTION_START_DATE => initial_start_date.strftime("%Y-%m-%d"), - # 2yr term, two billing cycles - CPQ_QUOTE_SUBSCRIPTION_TERM => 24.0, - } - ) - - sf_contract = create_contract_from_order(sf_order) - amendment_data = create_quote_data_from_contract_amendment(sf_contract) - - # increase quantity - amendment_data["lineItems"].first["record"]["SBQQ__Quantity__c"] = 3 - - amendment_data["record"][CPQ_QUOTE_SUBSCRIPTION_START_DATE] = start_date.strftime("%Y-%m-%d") - amendment_data["record"][CPQ_QUOTE_SUBSCRIPTION_TERM] = amendment_term - - sf_order_amendment = create_order_from_quote_data(amendment_data) - sf_order_amendment_contract = create_contract_from_order(sf_order_amendment) - - StripeForce::Translate.perform_inline(@user, sf_order_amendment.Id) - - sf_order.refresh - stripe_id = sf_order[prefixed_stripe_field(GENERIC_STRIPE_ID)] - subscription_schedule = Stripe::SubscriptionSchedule.retrieve(stripe_id, @user.stripe_credentials) - - assert_equal(2, subscription_schedule.phases.count) - - first_phase = T.must(subscription_schedule.phases.first) - second_phase = T.must(subscription_schedule.phases[1]) - - # first phase should start now and end in 9mo - assert_equal(0, first_phase.start_date - initial_start_date.to_i) - assert_equal(0, first_phase.end_date - start_date.to_i) - - # second phase should start at the end date - assert_equal(0, second_phase.start_date - start_date.to_i) - assert_equal(0, second_phase.end_date - end_date.to_i) - - assert_equal(1, first_phase.items.count) - assert_equal(1, second_phase.items.count) - - first_phase_item = T.must(first_phase.items.first) - second_phase_item = T.must(second_phase.items.first) - - assert_equal(1, first_phase_item.quantity) - assert_equal(3, second_phase_item.quantity) - - # price should NOT be identical since it's a shorter duration! - # assert_equal(first_phase_item.price, second_phase_item.price) - end - # this can occur if contracts are created async and the apex jobs are backed up # in this case, we'll just process the initial order we have without pulling the contract it 'does not fail if a contract does not yet exist for an order' do @@ -327,6 +275,10 @@ class Critic::OrderAmendmentTranslation < Critic::OrderAmendmentFunctionalTest end + it 'uses the latest metadata on an order line represented in a previous subscription schedule phase' do + + end + it 'supports adding one-off line items on a order amendment' do end diff --git a/test/integration/amendments/test_backend_prorated_amendments.rb b/test/integration/amendments/test_backend_prorated_amendments.rb new file mode 100644 index 0000000000..860e5e421e --- /dev/null +++ b/test/integration/amendments/test_backend_prorated_amendments.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true +# typed: true + +require_relative './_lib' + +class Critic::BackendProratedAmendmentTranslation < Critic::OrderAmendmentFunctionalTest + before do + @user = make_user(save: true) + end + + it 'throws an error if the subscription term is not divisible by billing frequency and is greater than one' do + skip("this case will be handled in an upcoming release, right now this functionality is undefined") + + sf_product_id, sf_pricebook_entry_id = salesforce_recurring_product_with_price( + additional_product_fields: { + CPQ_QUOTE_BILLING_FREQUENCY => CPQBillingFrequencyOptions::ANNUAL.serialize, + CPQ_QUOTE_SUBSCRIPTION_TERM => 12, + } + ) + + sf_order = create_salesforce_order( + sf_product_id: sf_product_id, + additional_quote_fields: { + CPQ_QUOTE_SUBSCRIPTION_TERM => 13, + CPQ_QUOTE_SUBSCRIPTION_START_DATE => now_time_formatted_for_salesforce, + } + ) + + exception = assert_raises(Integrations::Errors::UserError) do + StripeForce::Translate.perform_inline(@user, sf_order.Id) + end + + assert_match("Prorated order amendments are not yet supported", exception.message) + end +end diff --git a/test/integration/amendments/test_proration_amendments.rb b/test/integration/amendments/test_proration_amendments.rb new file mode 100644 index 0000000000..6dd3889f7d --- /dev/null +++ b/test/integration/amendments/test_proration_amendments.rb @@ -0,0 +1,260 @@ +# frozen_string_literal: true +# typed: true + +require_relative './_lib' + +class Critic::ProratedAmendmentTranslation < Critic::OrderAmendmentFunctionalTest + before do + @user = make_user(save: true) + end + + it 'creates a new phase with a duration longer than the billing frequency, but not divisible' do + # initial order: 2yr contract, one annual item + # second order: +2 quantity, revising the existing item, 6-24mo + + yearly_price = 120_00 + contract_term = 24 + amendment_term = 18 + amendment_start_date = now_time + (contract_term - amendment_term).months + amendment_end_date = amendment_start_date + amendment_term.months + initial_start_date = now_time + + sf_product_id, sf_pricebook_id = salesforce_recurring_product_with_price( + price: yearly_price, + additional_product_fields: { + CPQ_QUOTE_BILLING_FREQUENCY => CPQBillingFrequencyOptions::ANNUAL.serialize, + CPQ_QUOTE_SUBSCRIPTION_TERM => nil, + # CPQ_QUOTE_SUBSCRIPTION_TERM => 12, + } + ) + + sf_order = create_salesforce_order( + sf_product_id: sf_product_id, + additional_quote_fields: { + CPQ_QUOTE_SUBSCRIPTION_START_DATE => format_date_for_salesforce(initial_start_date), + CPQ_QUOTE_SUBSCRIPTION_TERM => contract_term, + } + ) + + sf_contract = create_contract_from_order(sf_order) + amendment_data = create_quote_data_from_contract_amendment(sf_contract) + + # increase quantity + amendment_data["lineItems"].first["record"]["SBQQ__Quantity__c"] = 3 + + amendment_data["record"][CPQ_QUOTE_SUBSCRIPTION_START_DATE] = format_date_for_salesforce(amendment_start_date) + amendment_data["record"][CPQ_QUOTE_SUBSCRIPTION_TERM] = amendment_term + + sf_order_amendment = create_order_from_quote_data(amendment_data) + sf_order_amendment_contract = create_contract_from_order(sf_order_amendment) + + StripeForce::Translate.perform_inline(@user, sf_order_amendment.Id) + + sf_order.refresh + stripe_id = sf_order[prefixed_stripe_field(GENERIC_STRIPE_ID)] + subscription_schedule = Stripe::SubscriptionSchedule.retrieve(stripe_id, @user.stripe_credentials) + + assert_equal(2, subscription_schedule.phases.count) + + first_phase = T.must(subscription_schedule.phases.first) + second_phase = T.must(subscription_schedule.phases[1]) + + # first phase should start now and end in 9mo + assert_equal(0, first_phase.start_date - initial_start_date.to_i) + assert_equal(0, first_phase.end_date - amendment_start_date.to_i) + + # second phase should start at the end date + assert_equal(0, second_phase.start_date - amendment_start_date.to_i) + assert_equal(0, second_phase.end_date - amendment_end_date.to_i) + + # first phase should have one sub item and no proration items + assert_equal(1, first_phase.items.count) + assert_empty(first_phase.add_invoice_items) + + # second phase should have two items (one original, one addition) + # it should have a single proration item + assert_equal(2, second_phase.items.count) + assert_equal(1, second_phase.add_invoice_items.count) + + first_phase_item = T.must(first_phase.items.first) + second_phase_item = T.must(second_phase.items.detect {|i| i.quantity == 1 }) + second_phase_item_additive = T.must(second_phase.items.detect {|i| i.quantity == 2 }) + + assert_equal(1, first_phase_item.quantity) + assert_equal(1, second_phase_item.quantity) + assert_equal(2, second_phase_item_additive.quantity) + + # metadata on the phase item representing the same order line should be identical + # additionally, the price IDs should be the same since the pricing information did not change + assert_equal(first_phase_item.metadata, second_phase_item.metadata) + assert_equal(first_phase_item.price, second_phase_item.price) + + # the additive price order line should have different metadata, but the exact same pricing information + refute_equal(second_phase_item_additive.metadata, first_phase_item.metadata) + second_phase_item_additive_price = Stripe::Price.retrieve(T.cast(second_phase_item_additive.price, String), @user.stripe_credentials) + first_phase_item_price = Stripe::Price.retrieve(T.cast(first_phase_item.price, String), @user.stripe_credentials) + assert_equal(second_phase_item_additive_price.unit_amount_decimal, first_phase_item_price.unit_amount_decimal) + assert_equal(second_phase_item_additive_price.type, first_phase_item_price.type) + assert_equal(second_phase_item_additive_price.product, first_phase_item_price.product) + + # now, let's take a look at the prorated items! + assert_equal(1, second_phase.add_invoice_items.count) + prorated_item = T.must(second_phase.add_invoice_items.first) + assert_equal(2, prorated_item.quantity) + prorated_price = Stripe::Price.retrieve(T.cast(prorated_item.price, String), @user.stripe_credentials) + + assert_equal('one_time', prorated_price.type) + assert_equal(prorated_price.product, first_phase_item_price.product) + assert_equal("true", prorated_price.metadata['salesforce_auto_archive']) + assert_equal("true", prorated_price.metadata['salesforce_duplicate']) + assert_equal("true", prorated_price.metadata['salesforce_proration']) + assert_equal(second_phase_item_additive_price.id, prorated_price.metadata['salesforce_original_stripe_price_id']) + + # since this is an 18mo prorated item we should only bill for 6mo since the rest will be billed by stripe + assert_equal((yearly_price / (12 / 6)).to_s, prorated_price.unit_amount_decimal) + + # TODO proration hash and metadata assertion + end + + # NOTE this was the first test written and has more extensive edge cases than other amendment tests + it 'creates a new phase with a duration shorter than the billing frequency' do + # initial order: 2yr contract (two billing cycles), one annual item + # second order: +2 quantity, revising the existing item, 18-24mo + + yearly_price = 120_00 + contract_term = 24 + amendment_start_date = now_time + 18.months + amendment_term = 6 + amendment_end_date = (amendment_start_date + amendment_term.months) + initial_start_date = now_time + + sf_product_id, sf_pricebook_id = salesforce_recurring_product_with_price( + price: yearly_price, + additional_product_fields: { + CPQ_QUOTE_BILLING_FREQUENCY => CPQBillingFrequencyOptions::ANNUAL.serialize, + } + ) + + sf_order = create_salesforce_order( + sf_product_id: sf_product_id, + additional_quote_fields: { + CPQ_QUOTE_SUBSCRIPTION_START_DATE => format_date_for_salesforce(initial_start_date), + CPQ_QUOTE_SUBSCRIPTION_TERM => contract_term, + } + ) + + sf_contract = create_contract_from_order(sf_order) + amendment_data = create_quote_data_from_contract_amendment(sf_contract) + + # increase quantity + amendment_data["lineItems"].first["record"]["SBQQ__Quantity__c"] = 3 + + amendment_data["record"][CPQ_QUOTE_SUBSCRIPTION_START_DATE] = format_date_for_salesforce(amendment_start_date) + amendment_data["record"][CPQ_QUOTE_SUBSCRIPTION_TERM] = amendment_term + + sf_order_amendment = create_order_from_quote_data(amendment_data) + sf_order_amendment_contract = create_contract_from_order(sf_order_amendment) + + # api preconditions: no end date calculated on orders, end date IS calculated on the contract + assert_nil(sf_order_amendment["EndDate"]) + # TODO understand why the contract end date is one day before Stripe, pretty certain this is due to differences in what "end" really is + assert_equal(format_date_for_salesforce(amendment_end_date - 1.day), sf_order_amendment_contract['EndDate']) + + StripeForce::Translate.perform_inline(@user, sf_order_amendment.Id) + + sf_order.refresh + stripe_id = sf_order[prefixed_stripe_field(GENERIC_STRIPE_ID)] + subscription_schedule = Stripe::SubscriptionSchedule.retrieve(stripe_id, @user.stripe_credentials) + + assert_equal(2, subscription_schedule.phases.count) + + first_phase = T.must(subscription_schedule.phases.first) + second_phase = T.must(subscription_schedule.phases[1]) + + # first phase should start now and end in 9mo + assert_equal(0, first_phase.start_date - initial_start_date.to_i) + assert_equal(0, first_phase.end_date - amendment_start_date.to_i) + + # second phase should start at the end date + assert_equal(0, second_phase.start_date - amendment_start_date.to_i) + assert_equal(0, second_phase.end_date - amendment_end_date.to_i) + + # first phase should have one sub item and no proration items + assert_equal(1, first_phase.items.count) + assert_empty(first_phase.add_invoice_items) + + # second phase should have two items (one original, one addition) + # it should have a single proration item + assert_equal(2, second_phase.items.count) + assert_equal(1, second_phase.add_invoice_items.count) + + first_phase_item = T.must(first_phase.items.first) + second_phase_item = T.must(second_phase.items.detect {|i| i.quantity == 1 }) + second_phase_item_additive = T.must(second_phase.items.detect {|i| i.quantity == 2 }) + + assert_equal(1, first_phase_item.quantity) + assert_equal(1, second_phase_item.quantity) + assert_equal(2, second_phase_item_additive.quantity) + + # metadata on the phase item representing the same order line should be identical + # additionally, the price IDs should be the same since the pricing information did not change + assert_equal(first_phase_item.metadata, second_phase_item.metadata) + assert_equal(first_phase_item.price, second_phase_item.price) + + # the additive price order line should have different metadata, but the exact same pricing information + refute_equal(second_phase_item_additive.metadata, first_phase_item.metadata) + second_phase_item_additive_price = Stripe::Price.retrieve(T.cast(second_phase_item_additive.price, String), @user.stripe_credentials) + first_phase_item_price = Stripe::Price.retrieve(T.cast(first_phase_item.price, String), @user.stripe_credentials) + assert_equal(second_phase_item_additive_price.unit_amount_decimal, first_phase_item_price.unit_amount_decimal) + assert_equal(second_phase_item_additive_price.type, first_phase_item_price.type) + assert_equal(second_phase_item_additive_price.product, first_phase_item_price.product) + + # let's make sure the pricing information is actually correct + assert_equal(yearly_price.to_s, first_phase_item_price.unit_amount_decimal) + assert_equal('recurring', first_phase_item_price.type) + assert_equal('month', first_phase_item_price.recurring.interval) + assert_equal(12, first_phase_item_price.recurring.interval_count) + + # the additive line price should be auto-archived, but not the first + assert_nil(first_phase_item_price.metadata['salesforce_auto_archive']) + assert(first_phase_item_price.active) + assert_equal("true", second_phase_item_additive_price.metadata['salesforce_auto_archive']) + refute(second_phase_item_additive_price.active) + + # now, let's take a look at the prorated items! + assert_equal(1, second_phase.add_invoice_items.count) + prorated_item = T.must(second_phase.add_invoice_items.first) + assert_equal(2, prorated_item.quantity) + prorated_price = Stripe::Price.retrieve(T.cast(prorated_item.price, String), @user.stripe_credentials) + + assert_equal('one_time', prorated_price.type) + assert_equal((yearly_price / (12 / amendment_term)).to_s, prorated_price.unit_amount_decimal) + assert_equal(prorated_price.product, first_phase_item_price.product) + assert_equal("true", prorated_price.metadata['salesforce_auto_archive']) + assert_equal("true", prorated_price.metadata['salesforce_duplicate']) + assert_equal("true", prorated_price.metadata['salesforce_proration']) + assert_equal(second_phase_item_additive_price.id, prorated_price.metadata['salesforce_original_stripe_price_id']) + # TODO assertions on proration and metadata + end + + # scenario where the start date is not on a billing cycle boundary but the subscription_term % billing_frequency == 0 + # TODO I am struggling to think of a situation where this could be true, might be impossible with the coterm requirement + # initial order: 1yr contract, one quarterly item + # second order: +1 quantity, revising the existing item, 6-24mo + + it 'bills the incremental quantity at a new price' do + + end + + it 'errors on a non-cotermed invoice' do + + end + + it 'excludes metered billing, tiered pricing, and one-time invoice items from proration calculation' do + + end + + it 'creates a new phase without prorations when all items are new' do + # if all line items are new + end +end diff --git a/test/integration/test_order_failures.rb b/test/integration/test_order_failures.rb index 57534b07f1..86119efc8e 100644 --- a/test/integration/test_order_failures.rb +++ b/test/integration/test_order_failures.rb @@ -56,7 +56,7 @@ class Critic::OrderFailureTest < Critic::FunctionalTest end # it looks like there are field validations in place to protect against the term being nil'd out on the order line - it 'creates a user error when the subscription term does not exist on the order line level' do + it 'creates a user error when the subscription term does not exist' do sf_order = create_salesforce_order(additional_quote_fields: { CPQ_QUOTE_SUBSCRIPTION_START_DATE => now_time_formatted_for_salesforce, # omit subscription term @@ -66,17 +66,15 @@ class Critic::OrderFailureTest < Critic::FunctionalTest SalesforceTranslateRecordJob.translate(@user, sf_order) end - order_items = sf_get_related(sf_order, SF_ORDER_ITEM) - assert_equal(1, order_items.size) - - sync_record = get_sync_record_by_secondary_id(order_items.first.Id) + sync_record = get_sync_record_by_secondary_id(sf_order.Id) assert_equal(SF_ORDER, sync_record[prefixed_stripe_field(SyncRecordFields::PRIMARY_OBJECT_TYPE.serialize)]) - assert_equal(SF_ORDER_ITEM, sync_record[prefixed_stripe_field(SyncRecordFields::SECONDARY_OBJECT_TYPE.serialize)]) + assert_equal(SF_ORDER, sync_record[prefixed_stripe_field(SyncRecordFields::SECONDARY_OBJECT_TYPE.serialize)]) assert_equal(sf_order.Id, sync_record[prefixed_stripe_field(SyncRecordFields::PRIMARY_RECORD_ID.serialize)]) + assert_equal(sf_order.Id, sync_record[prefixed_stripe_field(SyncRecordFields::SECONDARY_RECORD_ID.serialize)]) assert_match(sf_order.Id, sync_record[prefixed_stripe_field(SyncRecordFields::COMPOUND_ID.serialize)]) - assert_match("The following required fields are missing from this Salesforce record: Order.SBQQ__Quote__c.SBQQ__SubscriptionTerm__c", sync_record[prefixed_stripe_field(SyncRecordFields::RESOLUTION_MESSAGE.serialize)]) + assert_match("The following required fields are missing from this Salesforce record: SBQQ__Quote__c.SBQQ__SubscriptionTerm__c", sync_record[prefixed_stripe_field(SyncRecordFields::RESOLUTION_MESSAGE.serialize)]) assert_match(@user.salesforce_instance_url, sync_record[prefixed_stripe_field('Primary_Record__c')]) assert_match(@user.salesforce_instance_url, sync_record[prefixed_stripe_field('Secondary_Record__c')]) diff --git a/test/integration/test_sync_records.rb b/test/integration/test_sync_records.rb index 038ceeab15..f21a7e2ab2 100644 --- a/test/integration/test_sync_records.rb +++ b/test/integration/test_sync_records.rb @@ -15,6 +15,7 @@ class Critic::SyncRecords < Critic::FunctionalTest sf_order = create_salesforce_order( additional_quote_fields: { CPQ_QUOTE_SUBSCRIPTION_START_DATE => format_date_for_salesforce(start_date), + # omit subscription term } ) @@ -22,18 +23,16 @@ class Critic::SyncRecords < Critic::FunctionalTest SalesforceTranslateRecordJob.translate(@user, sf_order) end - order_items = sf_get_related(sf_order, SF_ORDER_ITEM) - assert_equal(1, order_items.size) - - sync_record = get_sync_record_by_secondary_id(order_items.first.Id) + sync_records = get_sync_records_by_primary_id(sf_order.Id) + assert_equal(1, sync_records.count) + sync_record = T.must(sync_records.first) assert_equal(SF_ORDER, sync_record[prefixed_stripe_field(SyncRecordFields::PRIMARY_OBJECT_TYPE.serialize)]) - assert_equal(SF_ORDER_ITEM, sync_record[prefixed_stripe_field(SyncRecordFields::SECONDARY_OBJECT_TYPE.serialize)]) + assert_equal(SF_ORDER, sync_record[prefixed_stripe_field(SyncRecordFields::SECONDARY_OBJECT_TYPE.serialize)]) assert_equal(sf_order.Id, sync_record[prefixed_stripe_field(SyncRecordFields::PRIMARY_RECORD_ID.serialize)]) assert_equal(SyncRecordResolutionStatuses::ERROR.serialize, sync_record[prefixed_stripe_field(SyncRecordFields::RESOLUTION_STATUS.serialize)]) - assert_match("The following required fields are missing from this Salesforce record: Order.SBQQ__Quote__c.SBQQ__SubscriptionTerm__c", sync_record[prefixed_stripe_field(SyncRecordFields::RESOLUTION_MESSAGE.serialize)]) - + assert_match("The following required fields are missing from this Salesforce record: SBQQ__Quote__c.SBQQ__SubscriptionTerm__c", sync_record[prefixed_stripe_field(SyncRecordFields::RESOLUTION_MESSAGE.serialize)]) # fix the missing subscription term in order to test the sync record upsert & associated trigger sf_quote_id = sf_order[SF_ORDER_QUOTE] @@ -42,30 +41,60 @@ class Critic::SyncRecords < Critic::FunctionalTest CPQ_QUOTE_SUBSCRIPTION_TERM => subscription_term, }) - sf_order = create_order_from_cpq_quote(sf_quote_id) - # Retranslate SalesforceTranslateRecordJob.translate(@user, sf_order) # Assert Order Sync Record Success - sync_record = get_sync_record_by_secondary_id((sf_order.Id)) + sync_records = get_sync_records_by_primary_id(sf_order.Id) + assert_equal(1, sync_records.count) + sync_record = T.must(sync_records.first) assert_equal(SF_ORDER, sync_record[prefixed_stripe_field(SyncRecordFields::PRIMARY_OBJECT_TYPE.serialize)]) assert_equal(SF_ORDER, sync_record[prefixed_stripe_field(SyncRecordFields::SECONDARY_OBJECT_TYPE.serialize)]) assert_equal(sf_order.Id, sync_record[prefixed_stripe_field(SyncRecordFields::PRIMARY_RECORD_ID.serialize)]) assert_equal(SyncRecordResolutionStatuses::SUCCESS.serialize, sync_record[prefixed_stripe_field(SyncRecordFields::RESOLUTION_STATUS.serialize)]) + end + + it 'updates secondary id failures when the primary succeeds' do + @user.field_defaults = { + "price" => { + "billing_scheme" => "bad_value", + }, + } + @user.save - # Assert Order Item Sync Record Success - order_items = sf_get_related(sf_order, SF_ORDER_ITEM) - assert_equal(1, order_items.size) + sf_product_id, sf_pricebook_id = salesforce_recurring_product_with_price + sf_order = create_subscription_order - sync_record = get_sync_record_by_secondary_id(order_items.first.Id) + exception = assert_raises(StripeForce::Errors::UserError) do + SalesforceTranslateRecordJob.translate(@user, sf_order) + end + + sync_records = get_sync_records_by_primary_id(sf_order.Id) + assert_equal(1, sync_records.count) + sync_record = T.must(sync_records.first) assert_equal(SF_ORDER, sync_record[prefixed_stripe_field(SyncRecordFields::PRIMARY_OBJECT_TYPE.serialize)]) assert_equal(SF_ORDER_ITEM, sync_record[prefixed_stripe_field(SyncRecordFields::SECONDARY_OBJECT_TYPE.serialize)]) assert_equal(sf_order.Id, sync_record[prefixed_stripe_field(SyncRecordFields::PRIMARY_RECORD_ID.serialize)]) - assert_equal(SyncRecordResolutionStatuses::RESOLVED.serialize, sync_record[prefixed_stripe_field(SyncRecordFields::RESOLUTION_STATUS.serialize)]) + refute_equal(sf_order.Id, sync_record[prefixed_stripe_field(SyncRecordFields::SECONDARY_RECORD_ID.serialize)]) + assert_equal(SyncRecordResolutionStatuses::ERROR.serialize, sync_record[prefixed_stripe_field(SyncRecordFields::RESOLUTION_STATUS.serialize)]) + + # now let's fix the mapping and reprocess + @user.update(field_defaults: {}) + + SalesforceTranslateRecordJob.translate(@user, sf_order) + + original_sync_record = sync_record + + # after success there would be two records: one for the order line, one for the order, both marked as success + sync_records = get_sync_records_by_primary_id(sf_order.Id) + assert_equal(2, sync_records.count) + sync_records.each do |r| + assert_equal(sf_order.Id, r[prefixed_stripe_field(SyncRecordFields::PRIMARY_RECORD_ID.serialize)]) + assert_includes([SyncRecordResolutionStatuses::SUCCESS.serialize, SyncRecordResolutionStatuses::RESOLVED.serialize], r[prefixed_stripe_field(SyncRecordFields::RESOLUTION_STATUS.serialize)]) + end end end diff --git a/test/integration/test_translate_order.rb b/test/integration/test_translate_order.rb index 169731e0c2..56d07110a4 100644 --- a/test/integration/test_translate_order.rb +++ b/test/integration/test_translate_order.rb @@ -227,9 +227,15 @@ class Critic::OrderTranslation < Critic::FunctionalTest end it 'integrates a subscription order with multiple lines' do - sf_product_id_1, sf_pricebook_id_1 = salesforce_recurring_product_with_price - sf_product_id_2, sf_pricebook_id_2 = salesforce_recurring_metered_produce_with_price - sf_product_id_3, sf_pricebook_id_3 = salesforce_standalone_product_with_price + # price customization makes it easier to debug and ensure there aren't weird state bugs + # across different price types + monthly_price = 120_00 + one_time_price = 45_00 + metered_price = 55_00 + + sf_product_id_1, sf_pricebook_id_1 = salesforce_recurring_product_with_price(price: monthly_price) + sf_product_id_2, sf_pricebook_id_2 = salesforce_recurring_metered_produce_with_price(price_in_cents: metered_price) + sf_product_id_3, sf_pricebook_id_3 = salesforce_standalone_product_with_price(price: one_time_price) sf_account_id = create_salesforce_account @@ -271,11 +277,12 @@ class Critic::OrderTranslation < Critic::FunctionalTest phase_item_with_five = T.must(phase.items.detect {|i| i.try(:quantity) == 5 }) price_1 = Stripe::Price.retrieve(T.cast(phase_item_with_five.price, String), @user.stripe_credentials) + assert_equal(monthly_price.to_s, price_1.unit_amount_decimal) assert_equal(sf_pricebook_id_1, price_1.metadata['salesforce_pricebook_entry_id']) phase_item_with_metered = T.must(phase.items.detect {|i| !i.respond_to?(:quantity) }) price_2 = Stripe::Price.retrieve(T.cast(phase_item_with_metered.price, String), @user.stripe_credentials) - assert_equal(sf_pricebook_id_2, price_2.metadata['salesforce_pricebook_entry_id']) + assert_equal(sf_pricebook_id_2, price_2.metadata['salesforce_pricebook_entry_id'], "pricebook entry does not exist, price may be created in error from an order line") standalone_item = T.must(phase.add_invoice_items.first) price_3 = Stripe::Price.retrieve(T.cast(standalone_item.price, String), @user.stripe_credentials) diff --git a/test/integration/translate/test_subscription_term.rb b/test/integration/translate/test_subscription_term.rb index 8a240f0591..49779525a1 100644 --- a/test/integration/translate/test_subscription_term.rb +++ b/test/integration/translate/test_subscription_term.rb @@ -8,6 +8,7 @@ class Critic::SubscriptionTermTranslation < Critic::FunctionalTest @user = make_user(save: true) end + # TODO but what exactly is this testing? Document this more clearly. it 'integrates an order with annual billing with a valid subscription term' do sf_product_id, sf_pricebook_entry_id = salesforce_recurring_product_with_price( additional_product_fields: { @@ -90,6 +91,7 @@ class Critic::SubscriptionTermTranslation < Critic::FunctionalTest sf_order_items = sf_get_related(sf_order, SF_ORDER_ITEM) assert_equal(1, sf_order_items.count) + # TODO document this in our technical document # if no subscription term is specified on the product, it looks like it is assumed to match the subscription term of the order sf_order_item = sf_order_items.first assert_equal(1, sf_order_item['SBQQ__ProrateMultiplier__c']) @@ -119,30 +121,4 @@ class Critic::SubscriptionTermTranslation < Critic::FunctionalTest assert_equal('month', price.recurring.interval) assert_equal("1000", price.unit_amount_decimal) end - - describe 'errors' do - # TODO this will be supported in the future: we will determine the portion of the line which can be prorated - it 'throws an error if the subscription term is not divisible by billing frequency and is greater than one' do - sf_product_id, sf_pricebook_entry_id = salesforce_recurring_product_with_price( - additional_product_fields: { - CPQ_QUOTE_BILLING_FREQUENCY => CPQBillingFrequencyOptions::ANNUAL.serialize, - CPQ_QUOTE_SUBSCRIPTION_TERM => 12, - } - ) - - sf_order = create_salesforce_order( - sf_product_id: sf_product_id, - additional_quote_fields: { - CPQ_QUOTE_SUBSCRIPTION_TERM => 13, - CPQ_QUOTE_SUBSCRIPTION_START_DATE => now_time_formatted_for_salesforce, - } - ) - - exception = assert_raises(Integrations::Errors::UserError) do - StripeForce::Translate.perform_inline(@user, sf_order.Id) - end - - assert_match("Prorated order amendments are not yet supported", exception.message) - end - end end diff --git a/test/support/application_integration_test.rb b/test/support/application_integration_test.rb index 38cdfe09f0..bbeb34d07a 100644 --- a/test/support/application_integration_test.rb +++ b/test/support/application_integration_test.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true # typed: true -require_relative './common_helpers' - class ApplicationIntegrationTest < ActionDispatch::IntegrationTest - include CommonHelpers + include Critic::CommonHelpers + include Critic::StripeFactory + include Critic::SalesforceFactory def parsed_json JSON.parse(response.body) diff --git a/test/support/common_helpers.rb b/test/support/common_helpers.rb index a29206a56c..b6ee4d1c29 100644 --- a/test/support/common_helpers.rb +++ b/test/support/common_helpers.rb @@ -1,21 +1,19 @@ # typed: true # frozen_string_literal: true -require_relative './salesforce_factory' require_relative './salesforce_debugging' -module CommonHelpers +module Critic::CommonHelpers include Kernel extend T::Sig + include Minitest::Assertions include StripeForce::Constants - include Critic::SalesforceFactory + include SalesforceDebugging # NOTE this is a little dangerous: we are only doing this for `prefixed_stripe_field` right now include StripeForce::Utilities::SalesforceUtil - include SalesforceDebugging - def sf_instance_account_id ENV.fetch('SF_INSTANCE_ID') end @@ -23,14 +21,9 @@ def sf_instance_account_id def get_sync_records_by_primary_id(primary_id) sync_record_results = sf.query("SELECT Id FROM #{prefixed_stripe_field(SYNC_RECORD)} WHERE #{prefixed_stripe_field(SyncRecordFields::PRIMARY_RECORD_ID.serialize)} = '#{primary_id}'") - - sync_records = [] - sync_record_results.each do |sync_record| - sync_record = sf.find(prefixed_stripe_field(SYNC_RECORD), sync_record.Id) - sync_records.append(sync_record) + sync_record_results.map do |sync_record| + sf.find(prefixed_stripe_field(SYNC_RECORD), sync_record.Id) end - - sync_records end # The get_sync_record_by_secondary_id does not make sense with the addition of Success Sync records, as that is the grouping ID. @@ -89,27 +82,6 @@ def make_user(sandbox: false, save: false, random_user_id: false, livemode: fals user end - sig { params(type: String, obj: T.nilable(Stripe::StripeObject)).returns(Stripe::Event) } - def create_event(type, obj=nil) - obj ||= Stripe::Charge.construct_from( - id: stripe_create_id(:ch) - ) - - Stripe::Event.construct_from({ - "id" => stripe_create_id(:evt), - "created" => Time.now.getutc.to_i, - "livemode" => false, - "type" => type, - "data" => { - "object" => JSON.parse(obj.to_json), - }, - "object" => "event", - "pending_webhooks" => 0, - "account" => stripe_create_id(:acct), - "request" => stripe_create_id(:iar), - }) - end - def stripe_create_id(prefix) # NOTE: The number after the underscore has significance for Stripe's internal routing. # While we don't expect these IDs to be used for real API calls, we want to ensure @@ -123,6 +95,26 @@ def stripe_create_id(prefix) prefix.to_s + random_id end + def sf_randomized_id + random_id = SecureRandom.alphanumeric(29) + + if ENV['CIRCLE_NODE_INDEX'] + random_id = "#{random_id}#{ENV['CIRCLE_NODE_INDEX']}" + end + + random_id + end + + sig { returns(String) } + def create_random_email + "#{sf_randomized_id}@example.com" + end + + def sf_randomized_name(sf_object_name) + node_identifier = ENV['CIRCLE_NODE_INDEX'] || "" + "REST #{sf_object_name} #{node_identifier} #{DateTime.now}" + end + # Helper to poll for an expected result sig do params( diff --git a/test/support/functional_test.rb b/test/support/functional_test.rb index b793d60669..9c4f0a1648 100644 --- a/test/support/functional_test.rb +++ b/test/support/functional_test.rb @@ -3,7 +3,9 @@ # ActiveSupport::TestCase gives us the ability to run tests by line number class Critic::FunctionalTest < ActiveSupport::TestCase - include CommonHelpers + include Critic::CommonHelpers + include Critic::StripeFactory + include Critic::SalesforceFactory def setup common_setup diff --git a/test/support/salesforce_factory.rb b/test/support/salesforce_factory.rb index 03412c2c05..aaee5d4b9a 100644 --- a/test/support/salesforce_factory.rb +++ b/test/support/salesforce_factory.rb @@ -10,9 +10,7 @@ module SalesforceFactory include Kernel include StripeForce::Constants include Minitest::Assertions - - sig { abstract.returns(T.untyped) } - def sf; end + include Critic::CommonHelpers sig { returns(String) } def now_time_formatted_for_salesforce @@ -32,11 +30,6 @@ def create_salesforce_id(prefix: nil) (prefix || "") + SecureRandom.alphanumeric(prefix ? 15 : 18) end - sig { returns(String) } - def create_random_email - "#{sf_randomized_id}@example.com" - end - def create_mock_salesforce_order id = create_salesforce_id(prefix: "802") Restforce::SObject.new({ @@ -65,21 +58,6 @@ def create_mock_salesforce_customer }) end - def sf_randomized_id - random_id = SecureRandom.alphanumeric(29) - - if ENV['CIRCLE_NODE_INDEX'] - random_id = "#{random_id}#{ENV['CIRCLE_NODE_INDEX']}" - end - - random_id - end - - def sf_randomized_name(sf_object_name) - node_identifier = ENV['CIRCLE_NODE_INDEX'] || "" - "REST #{sf_object_name} #{node_identifier} #{DateTime.now}" - end - def create_salesforce_account(additional_fields: {}) account_id = sf.create!(SF_ACCOUNT, { Name: sf_randomized_name(SF_ACCOUNT), @@ -136,15 +114,26 @@ def create_salesforce_product(additional_fields: {}) }.merge(additional_fields)) end - def salesforce_recurring_metered_produce_with_price + def salesforce_recurring_metered_produce_with_price(price_in_cents: nil) salesforce_recurring_product_with_price( + price: price_in_cents, additional_product_fields: { CPQ_PRODUCT_BILLING_TYPE => CPQProductBillingTypeOptions::ARREARS.serialize, } ) end + # TODO `price` should be `price_in_cents` def salesforce_recurring_product_with_price(price: nil, additional_product_fields: {}) + # I don't fully understand how the subscription term on the price iteracts with the billing frequency, + # but if the term is set to a value which is different than the billing frequency it seems to use the + # subscription term value. i.e. a yearly billed product + subscription_term = if additional_product_fields.key?(CPQ_QUOTE_BILLING_FREQUENCY) && additional_product_fields[CPQ_QUOTE_BILLING_FREQUENCY] != CPQBillingFrequencyOptions::MONTHLY.serialize + nil + else + 1 + end + # blanking out the subscription type ensures it is a one-time product product_id = create_salesforce_product(additional_fields: { # anything non-nil indicates subscription/recurring pricing @@ -152,8 +141,7 @@ def salesforce_recurring_product_with_price(price: nil, additional_product_field CPQ_PRODUCT_SUBSCRIPTION_TYPE => CPQProductSubscriptionTypeOptions::RENEWABLE, - # default term of one month - CPQ_QUOTE_SUBSCRIPTION_TERM => 1, + CPQ_QUOTE_SUBSCRIPTION_TERM => subscription_term, # one month CPQ_QUOTE_BILLING_FREQUENCY => CPQBillingFrequencyOptions::MONTHLY.serialize, @@ -221,6 +209,8 @@ def add_product_to_cpq_quote(quote_id, sf_product_id:, sf_pricebook_id: nil) end def calculate_and_save_cpq_quote(quote_data) + log.info 'calculate and save quote' + # https://developer.salesforce.com/docs/atlas.en-us.cpq_dev_api.meta/cpq_dev_api/cpq_quote_api_calculate_final.htm calculated_quote = JSON.parse(sf.patch('/services/apexrest/SBQQ/ServiceRouter?loader=SBQQ.QuoteAPI.QuoteCalculator', { # "context": quote_with_product.to_json diff --git a/test/support/stripe_factory.rb b/test/support/stripe_factory.rb index 54ee5ced8e..b409abf1a0 100644 --- a/test/support/stripe_factory.rb +++ b/test/support/stripe_factory.rb @@ -9,18 +9,80 @@ module StripeFactory include Kernel include StripeForce::Constants + include CommonHelpers - def create_stripe_id(prefix) - # NOTE: The number after the underscore has significance for Stripe's internal routing. - # While we don't expect these IDs to be used for real API calls, we want to ensure - # they don't lead to unexpected behavior if they are. - random_id = "_1" + SecureRandom.alphanumeric(29) + sig { params(type: String, obj: T.nilable(Stripe::StripeObject)).returns(Stripe::Event) } + def create_event(type, obj=nil) + obj ||= Stripe::Charge.construct_from( + id: stripe_create_id(:ch) + ) - if ENV['CIRCLE_NODE_INDEX'] - random_id = "#{random_id}#{ENV['CIRCLE_NODE_INDEX']}" - end + Stripe::Event.construct_from({ + "id" => stripe_create_id(:evt), + "created" => Time.now.getutc.to_i, + "livemode" => false, + "type" => type, + "data" => { + "object" => JSON.parse(obj.to_json), + }, + "object" => "event", + "pending_webhooks" => 0, + "account" => stripe_create_id(:acct), + "request" => stripe_create_id(:iar), + }) + end + + def create_customer_with_subscription + customer = create_customer_with_card + product, price = create_price + + Stripe::Subscription.create({ + customer: customer.id, + items: [ + { + price: price.id, + }, + ], + }, @user.stripe_credentials) + end + + def create_customer_with_card + customer = create_customer + payment_method = Stripe::PaymentMethod.attach( + 'pm_card_visa', + {customer: customer.id}, + @user.stripe_credentials + ) + customer.invoice_settings.default_payment_method = payment_method.id + customer.save + customer + end + + def create_customer + email = create_random_email + description = "Sample customer for Salesforce" + + customer = Stripe::Customer.create({ + description: description, + email: email, + }, @user.stripe_credentials) + end + + def create_price(additional_price_fields: {}) + stripe_product = Stripe::Product.create({ + name: sf_randomized_name("Product"), + }, @user.stripe_credentials) + + stripe_price = Stripe::Price.create({ + product: stripe_product.id, + currency: 'usd', + unit_amount: 10_00, + recurring: { + interval: 'month', + }, + }.merge(additional_price_fields), @user.stripe_credentials) - prefix.to_s + random_id + [stripe_product, stripe_price] end end end diff --git a/test/support/unit_test.rb b/test/support/unit_test.rb index 29ef3ed59e..2b24cd4eda 100644 --- a/test/support/unit_test.rb +++ b/test/support/unit_test.rb @@ -5,8 +5,9 @@ # ActiveSupport::TestCase gives us the ability to run tests by line number class Critic::UnitTest < ActiveSupport::TestCase - include CommonHelpers + include Critic::CommonHelpers include Critic::StripeFactory + include Critic::SalesforceFactory def setup common_setup diff --git a/test/test_helper.rb b/test/test_helper.rb index e44170e27b..decec471a2 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -22,6 +22,8 @@ module Critic require 'pry-rescue/minitest' unless ENV['CI'] || ENV['EXT_DIR'] || ENV['NO_RESCUE'] require_relative 'support/common_helpers' +require_relative 'support/stripe_factory' +require_relative 'support/salesforce_factory' Dir[File.join(File.dirname(__FILE__), "support/**/*.rb")].sort.each {|f| require f } Minitest::Ci.clean = false if ENV['CI'] diff --git a/test/unit/test_translator.rb b/test/unit/test_translator.rb index a99052353c..30d1b2f8e7 100644 --- a/test/unit/test_translator.rb +++ b/test/unit/test_translator.rb @@ -75,7 +75,7 @@ class TranslatorTest < Critic::UnitTest @user.connector_settings[CONNECTOR_SETTING_SALESFORCE_NAMESPACE] = SalesforceNamespaceOptions::QA.serialize customer = create_mock_salesforce_customer - stripe_customer = Stripe::Customer.construct_from(id: create_stripe_id(:cus)) + stripe_customer = Stripe::Customer.construct_from(id: stripe_create_id(:cus)) @translator.sf.expects(:update!).with do |name, params| assert_equal(SF_ACCOUNT, name) @@ -91,7 +91,7 @@ class TranslatorTest < Critic::UnitTest @user.connector_settings[CONNECTOR_SETTING_SALESFORCE_NAMESPACE] = SalesforceNamespaceOptions::PRODUCTION.serialize customer = create_mock_salesforce_customer - stripe_customer = Stripe::Customer.construct_from(id: create_stripe_id(:cus)) + stripe_customer = Stripe::Customer.construct_from(id: stripe_create_id(:cus)) @translator.sf.expects(:update!).with do |name, params| assert_equal(SF_ACCOUNT, name) @@ -107,7 +107,7 @@ class TranslatorTest < Critic::UnitTest @user.connector_settings[CONNECTOR_SETTING_SALESFORCE_NAMESPACE] = SalesforceNamespaceOptions::NONE.serialize customer = create_mock_salesforce_customer - stripe_customer = Stripe::Customer.construct_from(id: create_stripe_id(:cus)) + stripe_customer = Stripe::Customer.construct_from(id: stripe_create_id(:cus)) @translator.sf.expects(:update!).with do |name, params| assert_equal(SF_ACCOUNT, name)