From 6099d3640defb931c5fdda210b83f0f4dad364a0 Mon Sep 17 00:00:00 2001 From: Michael Kohl Date: Fri, 21 Jan 2022 09:27:10 +0700 Subject: [PATCH] ListingsToolkit refactoring (#16184) --- app/controllers/concerns/listings_toolkit.rb | 29 ++---------- app/services/listings/create.rb | 50 ++++++++++++++++++++ spec/services/listings/create_spec.rb | 17 +++++++ 3 files changed, 70 insertions(+), 26 deletions(-) create mode 100644 app/services/listings/create.rb create mode 100644 spec/services/listings/create_spec.rb diff --git a/app/controllers/concerns/listings_toolkit.rb b/app/controllers/concerns/listings_toolkit.rb index 0ef3b154d..07393c498 100644 --- a/app/controllers/concerns/listings_toolkit.rb +++ b/app/controllers/concerns/listings_toolkit.rb @@ -16,10 +16,7 @@ module ListingsToolkit # [@forem/oss] Not entirely sure what the intention behind the # original code was, but at least this is more compact. - # [@forem/oss] Rails 6.1 adds `.compact` on ActionController::Parameters - # rubocop:disable Style/CollectionCompact - filtered_params = listing_params.reject { |_k, v| v.nil? } - # rubocop:enable Style/CollectionCompact + filtered_params = listing_params.compact @listing.update(filtered_params) end @@ -105,29 +102,9 @@ module ListingsToolkit end def create_listing(purchaser, cost) - successful_transaction = false - ActiveRecord::Base.transaction do - # subtract credits - Credits::Buyer.call( - purchaser: purchaser, - purchase: @listing, - cost: cost, - ) + create_result = Listings::Create.call(@listing, purchaser: purchaser, cost: cost) - # save the listing - @listing.bumped_at = Time.current - @listing.published = true - @listing.originally_published_at = Time.current - - # since we can't raise active record errors in this transaction - # due to the fact that we need to display them in the :new view, - # we manually rollback the transaction if there are validation errors - raise ActiveRecord::Rollback unless @listing.save - - successful_transaction = true - end - - if successful_transaction + if create_result.success? rate_limiter.track_limit_by_action(:listing_creation) clear_listings_cache process_successful_creation diff --git a/app/services/listings/create.rb b/app/services/listings/create.rb new file mode 100644 index 000000000..e6458c843 --- /dev/null +++ b/app/services/listings/create.rb @@ -0,0 +1,50 @@ +module Listings + # This service wraps listing purchses in a transaction which verifies that + # the purchaser has enough credits to complete this process. + class Create + # @param listing [Listing] an initialized but not yet persisted listing + # @param purchaser [User] the user attempting to purchase the listing + # @param cost [Integer] the number of credits the user has to spend + # @return [Listings::Create] the service itself, used for the success? check + def self.call(listing, purchaser:, cost:) + new(listing, purchaser, cost).call + end + + def initialize(listing, purchaser, cost) + @listing = listing + @purchaser = purchaser + @cost = cost + @success = false + end + + def call + ActiveRecord::Base.transaction do + # Subtract credits + enough_credits = Credits::Buyer.call( + purchaser: @purchaser, + purchase: @listing, + cost: @cost, + ) + + raise ActiveRecord::Rollback unless enough_credits + + # Update the listing + @listing.bumped_at = Time.current + @listing.published = true + @listing.originally_published_at = Time.current + + # Since we can't raise ActiveRecord errors in this transaction + # due to the fact that we need to display them in the :new view, + # we manually rollback the transaction if there are validation errors + raise ActiveRecord::Rollback unless @listing.save + + @success = true + end + self + end + + def success? + @success + end + end +end diff --git a/spec/services/listings/create_spec.rb b/spec/services/listings/create_spec.rb new file mode 100644 index 000000000..d93defafc --- /dev/null +++ b/spec/services/listings/create_spec.rb @@ -0,0 +1,17 @@ +require "rails_helper" + +RSpec.describe Listings::Create, type: :service do + let(:user) { create(:user) } + let(:listing) { build(:listing, user: user) } + + it "is successful when the purchaser has enough credits" do + create(:credit, user: user) + create_result = described_class.call(listing, purchaser: user, cost: 1) + expect(create_result.success?).to be true + end + + it "fails if the purchaser doesn't have enough credits" do + create_result = described_class.call(listing, purchaser: user, cost: 1) + expect(create_result.success?).to be false + end +end