From 7e18b1d42f7f94c55545b4af65a315126471bf5e Mon Sep 17 00:00:00 2001 From: Weston Triemstra Date: Wed, 28 Feb 2018 22:50:56 -0800 Subject: [PATCH 01/12] [LO-970] Get spec passing, time based query that takes deleted_at into account is already in place --- lib/orders/unit_price_logic.rb | 4 ++-- spec/lib/orders/unit_price_logic_spec.rb | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/orders/unit_price_logic.rb b/lib/orders/unit_price_logic.rb index 1a40aab974..e24da3b760 100644 --- a/lib/orders/unit_price_logic.rb +++ b/lib/orders/unit_price_logic.rb @@ -2,9 +2,9 @@ module Orders class UnitPriceLogic class << self def prices(product, market, organization, time) - price = Price.for_product_and_market_and_org_at_time(product, market, organization, time).visible.order("min_quantity, organization_id") + price = Price.for_product_and_market_and_org_at_time(product, market, organization, time).order("min_quantity, organization_id") if price.empty? - price = Price.for_product_and_market_and_org_at_time(product, market, organization, Time.current).visible.order(:organization_id, :min_quantity) + price = Price.for_product_and_market_and_org_at_time(product, market, organization, Time.current).order(:organization_id, :min_quantity) end temp_min = nil dups = [] diff --git a/spec/lib/orders/unit_price_logic_spec.rb b/spec/lib/orders/unit_price_logic_spec.rb index 3f8c0e3caf..5f08087c08 100644 --- a/spec/lib/orders/unit_price_logic_spec.rb +++ b/spec/lib/orders/unit_price_logic_spec.rb @@ -8,10 +8,10 @@ let(:organization) { order.organization } let(:prices) do - [ + [ create(:price, min_quantity: 1, sale_price: 3), create(:price, min_quantity: 5, sale_price: 2), - create(:price, min_quantity: 8, sale_price: 1) + create(:price, min_quantity: 8, sale_price: 1) ] end @@ -70,7 +70,7 @@ it "returns generic pricing when org doesn't have special pricing" do other_org = double "other org", id: organization.id+1 - + [ 0,1,4 ].each do |quantity| price = logic.unit_price(product, market, other_org, order_time, quantity) expect(price.sale_price).to eql(3) From 004e0dddb2e1c39e2c90dc395aad2bfea5440216 Mon Sep 17 00:00:00 2001 From: Weston Triemstra Date: Thu, 8 Mar 2018 22:17:48 -0800 Subject: [PATCH 02/12] [LO-970] update poltergeist config to get rid of Stripe warnings that caused a bunch of failing tests --- spec/support/capybara.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb index 56acf89e5e..11ff72d1b7 100644 --- a/spec/support/capybara.rb +++ b/spec/support/capybara.rb @@ -9,7 +9,7 @@ inspector: false, debug: false, js_errors: false, - phantomjs_options: ['--debug=false', '--ssl-protocol=TLSv1']) + phantomjs_options: ['--debug=false', '--ssl-protocol=TLSv1.2']) end Capybara.javascript_driver = :poltergeist From 5236880d95b7ccb155c384efd628a166ebf4f980 Mon Sep 17 00:00:00 2001 From: Weston Triemstra Date: Thu, 8 Mar 2018 22:41:09 -0800 Subject: [PATCH 03/12] [LO-970] Get rid of some 'already initialized constant' warnings by using proper constants --- app/interactors/send_fresh_sheet.rb | 8 ++++---- app/interactors/send_newsletter.rb | 8 ++++---- app/models/newsletter.rb | 2 +- app/models/subscription.rb | 4 ++-- app/models/subscription_type.rb | 4 ++-- spec/factories.rb | 4 ++-- .../market_manager/fresh_sheet_spec.rb | 6 +++--- .../market_manager/newsletters_spec.rb | 2 +- spec/interactors/send_fresh_sheet_spec.rb | 8 ++++---- spec/interactors/send_newsletter_spec.rb | 20 +++++++++---------- 10 files changed, 33 insertions(+), 33 deletions(-) diff --git a/app/interactors/send_fresh_sheet.rb b/app/interactors/send_fresh_sheet.rb index 649303dacd..e1a98f43df 100644 --- a/app/interactors/send_fresh_sheet.rb +++ b/app/interactors/send_fresh_sheet.rb @@ -22,15 +22,15 @@ def send_test_email end def send_fresh_sheets_to_subscribed_members - fresh_sheet_type = SubscriptionType.find_by(keyword: SubscriptionType::Keywords::FreshSheet) + fresh_sheet_type = SubscriptionType.find_by(keyword: SubscriptionType::Keywords::FRESHSHEET) User.in_market(market). subscribed_to(fresh_sheet_type). - uniq. + uniq. includes(:subscriptions). each do |user| token = user.unsubscribe_token(subscription_type: fresh_sheet_type) - MarketMailer.delay.fresh_sheet(market: market, - to: user.pretty_email, + MarketMailer.delay.fresh_sheet(market: market, + to: user.pretty_email, note: CGI::unescapeHTML(note), unsubscribe_token: token, port: get_port) diff --git a/app/interactors/send_newsletter.rb b/app/interactors/send_newsletter.rb index 5e9380bada..0435571cef 100644 --- a/app/interactors/send_newsletter.rb +++ b/app/interactors/send_newsletter.rb @@ -7,13 +7,13 @@ def perform context[:notice] = "Successfully sent a test to #{email}" elsif commit == "Send Now" - newsletter_type = SubscriptionType.find_by(keyword: SubscriptionType::Keywords::Newsletter) + newsletter_type = SubscriptionType.find_by(keyword: SubscriptionType::Keywords::NEWSLETTER) newsletter.recipients.each do |user| token = user.unsubscribe_token(subscription_type: newsletter_type) MarketMailer.delay.newsletter( - newsletter: newsletter, - market: market, - to: user.pretty_email, + newsletter: newsletter, + market: market, + to: user.pretty_email, unsubscribe_token: token, port: get_port) end diff --git a/app/models/newsletter.rb b/app/models/newsletter.rb index 908a731e07..6a050b3fa5 100644 --- a/app/models/newsletter.rb +++ b/app/models/newsletter.rb @@ -9,7 +9,7 @@ class Newsletter < ActiveRecord::Base validates_property :format, of: :image, in: %w(jpg jpeg png gif) def recipients - newsletter_type = SubscriptionType::Keywords::Newsletter + newsletter_type = SubscriptionType::Keywords::NEWSLETTER subscribers = User.in_market(market).subscribed_to(newsletter_type).includes(:subscriptions) recipients = Set.new diff --git a/app/models/subscription.rb b/app/models/subscription.rb index 23afac1977..a2f500e26a 100644 --- a/app/models/subscription.rb +++ b/app/models/subscription.rb @@ -26,8 +26,8 @@ def self.ensure_user_has_subscription_link_to(user, subscription_type_keyword:) end def self.ensure_user_has_subscription_links_to_fresh_sheet_and_newsletter(user) - [ SubscriptionType::Keywords::FreshSheet, - SubscriptionType::Keywords::Newsletter + [ SubscriptionType::Keywords::FRESHSHEET, + SubscriptionType::Keywords::NEWSLETTER ].each do |keyword| Subscription.ensure_user_has_subscription_link_to(user, subscription_type_keyword: keyword) end diff --git a/app/models/subscription_type.rb b/app/models/subscription_type.rb index d1b6bf9254..96c9c1becf 100644 --- a/app/models/subscription_type.rb +++ b/app/models/subscription_type.rb @@ -1,7 +1,7 @@ class SubscriptionType < ActiveRecord::Base module Keywords - FreshSheet = "fresh_sheet" - Newsletter = "newsletter" + FRESHSHEET = 'fresh_sheet' + NEWSLETTER = 'newsletter' end has_many :subscriptions diff --git a/spec/factories.rb b/spec/factories.rb index 12b2fef9f3..3c1269b0df 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -737,12 +737,12 @@ trait :fresh_sheet do name "Fresh Sheet (testing)" - keyword SubscriptionType::Keywords::FreshSheet + keyword SubscriptionType::Keywords::FRESHSHEET end trait :newsletter do name "Newsletter (testing)" - keyword SubscriptionType::Keywords::Newsletter + keyword SubscriptionType::Keywords::NEWSLETTER end end diff --git a/spec/features/market_manager/fresh_sheet_spec.rb b/spec/features/market_manager/fresh_sheet_spec.rb index 53c8861abd..4864bd729a 100644 --- a/spec/features/market_manager/fresh_sheet_spec.rb +++ b/spec/features/market_manager/fresh_sheet_spec.rb @@ -12,10 +12,10 @@ # Intentionally not let! changing that will break tests let(:buyer_org) { create(:organization, :buyer, markets: [market]) } - let(:buyer_user) { + let(:buyer_user) { jack = create(:user, :buyer, organizations: [buyer_org], name: "Jack Stevens") - jack.subscribe_to(SubscriptionType::Keywords::FreshSheet) - jack + jack.subscribe_to(SubscriptionType::Keywords::FRESHSHEET) + jack } scenario "navigating to the page" do diff --git a/spec/features/market_manager/newsletters_spec.rb b/spec/features/market_manager/newsletters_spec.rb index 24e9c3b07d..7e1a3922b9 100644 --- a/spec/features/market_manager/newsletters_spec.rb +++ b/spec/features/market_manager/newsletters_spec.rb @@ -126,7 +126,7 @@ describe "to groups" do it "sends to Manager" do newsletter.market_managers=true - mmtoken = market_manager.unsubscribe_token(subscription_type: SubscriptionType.find_by(keyword:SubscriptionType::Keywords::Newsletter)) + mmtoken = market_manager.unsubscribe_token(subscription_type: SubscriptionType.find_by(keyword:SubscriptionType::Keywords::NEWSLETTER)) expect_send_newsletter_mail(newsletter:newsletter, market:market, to:market_manager.pretty_email,unsubscribe_token:mmtoken) check "Manager" click_button "Send Now" diff --git a/spec/interactors/send_fresh_sheet_spec.rb b/spec/interactors/send_fresh_sheet_spec.rb index 5d6fba19b5..4da49f6ea3 100644 --- a/spec/interactors/send_fresh_sheet_spec.rb +++ b/spec/interactors/send_fresh_sheet_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe SendFreshSheet do - + let!(:market) { create(:market, name: "Mad Dog Farm n Fry", delivery_schedules: [create(:delivery_schedule)]) } let(:note) { "B flat" } @@ -21,8 +21,8 @@ end it "sends Fresh Sheet emails to all users in the given market who subscribe to Fresh Sheets" do - fresh = create(:subscription_type, - keyword: SubscriptionType::Keywords::FreshSheet, + fresh = create(:subscription_type, + keyword: SubscriptionType::Keywords::FRESHSHEET, name: "Test Fresh!") user1 = create(:user, :buyer) @@ -67,7 +67,7 @@ # # HELPERS # - + def assert_fresh_sheet_sent_to(mail,market,sent_to,note) expect(mail).to be expect(mail.to.first).to eq(sent_to) diff --git a/spec/interactors/send_newsletter_spec.rb b/spec/interactors/send_newsletter_spec.rb index b0203eb7a7..4a33e49e0f 100644 --- a/spec/interactors/send_newsletter_spec.rb +++ b/spec/interactors/send_newsletter_spec.rb @@ -27,10 +27,10 @@ end describe "sending to groups" do - let!(:newsletter_type) { create(:subscription_type, - keyword: SubscriptionType::Keywords::Newsletter, + let!(:newsletter_type) { create(:subscription_type, + keyword: SubscriptionType::Keywords::NEWSLETTER, name: "Test News!") } - before do + before do [mary, bill, basil, steve, sol, clarence, craig].each do |user| user.subscribe_to(newsletter_type) end @@ -70,8 +70,8 @@ it "do nothing on unknown action" do context = SendNewsletter.perform( commit: "lol wat", - market: fresh_market, - newsletter: news_for_all, + market: fresh_market, + newsletter: news_for_all, email:"hossnfeffer@example.com", port:80) expect(context.success?).to eq(true) @@ -82,16 +82,16 @@ # HELPERS # def send_newsletter(newsletter) - context = SendNewsletter.perform(newsletter: newsletter, - market: fresh_market, + context = SendNewsletter.perform(newsletter: newsletter, + market: fresh_market, commit: "Send Now",port:80) - + expect(context.success?).to eq(true) expect(context.notice).to eq("Successfully sent this Newsletter") - + return ActionMailer::Base.deliveries end - + def assert_newsletter_sent_to(mail,market,sent_to,newsletter) expect(mail).to be expect(mail.to.first).to eq(sent_to) From 949bbe9ee4cf041796655835ba1332f962500e4a Mon Sep 17 00:00:00 2001 From: Weston Triemstra Date: Mon, 9 Apr 2018 08:39:05 -0700 Subject: [PATCH 04/12] Add 'require spec_helper' to .rspec, store which specs failed on last run so we can use --only-failures and --next-failure --- .gitignore | 7 ++++--- .rspec | 2 ++ spec/spec_helper.rb | 4 ++++ 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index e9a18987ec..c329b4f035 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,9 @@ /config/database.yml +# rspec failures +examples.txt + # Ignore the default SQLite database. /db/*.sqlite3 /db/*.sqlite3-journal @@ -27,20 +30,18 @@ dragonfly.log /dragonfly public/system/dragonfly/development -.DS_Store - tags *.swp *.swo latest.prod.dump cleansed.prod.dump -.DS_Store ~*.xlsx # IntelliJ /.idea /localorbit.iml + node_modules/ *.sublime-project diff --git a/.rspec b/.rspec index 9d52c56327..f494de58d3 100644 --- a/.rspec +++ b/.rspec @@ -1,2 +1,4 @@ --color --format doc +--require spec_helper + diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b4d6f81fb1..39fdea202d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -58,6 +58,10 @@ def app; Capybara.app; end # --seed 1234 config.order = "random" + # Store status of last run so we can use --only-failures and --next-failure + config.example_status_persistence_file_path = "examples.txt" + config.run_all_when_everything_filtered = true + # RSpec Rails can automatically mix in different behaviours to your tests # based on their file location, for example enabling you to call `get` and # `post` in specs under `spec/controllers`. From b3af37ff026a5e127822a3b7f6e05bf7528f152f Mon Sep 17 00:00:00 2001 From: Weston Triemstra Date: Mon, 9 Apr 2018 08:39:39 -0700 Subject: [PATCH 05/12] Update to match stripe API changes --- spec/lib/error_reporting_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/lib/error_reporting_spec.rb b/spec/lib/error_reporting_spec.rb index 2a8166711c..594f05e52f 100644 --- a/spec/lib/error_reporting_spec.rb +++ b/spec/lib/error_reporting_spec.rb @@ -86,7 +86,9 @@ error_data: { error: { type: "invalid_request_error", - message: "Must provide source or customer." + message: "Must provide source or customer.", + code: "parameter_missing", + doc_url: "https://stripe.com/docs/error-codes#parameter-missing" } } ) From 2920cf868eed6618e2f53adc863ab2592e2ebecd Mon Sep 17 00:00:00 2001 From: Weston Triemstra Date: Mon, 9 Apr 2018 08:42:55 -0700 Subject: [PATCH 06/12] Update stripe data with new plans, rake task to populate test and development stripe account(s) --- lib/tasks/stripe.rake | 12 +++++++++++- spec/interactors/roll_your_own_market_spec.rb | 6 +++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/tasks/stripe.rake b/lib/tasks/stripe.rake index a2ac37a963..eb44e74be6 100644 --- a/lib/tasks/stripe.rake +++ b/lib/tasks/stripe.rake @@ -1,5 +1,15 @@ namespace :stripe do + desc 'Create plans at Stripe' + task create_plans: :environment do + stripe_plans = [ + {id: 'PRODUCER_2017', product: {name: 'Producer Plan - Annual'}, amount: 6900, interval: 'year', currency: 'usd'}, + {id: 'START_M_2017', product: {name: 'Start Plan - Monthly'}, amount: 7000, interval: 'month', currency: 'usd'}, + {id: 'GROW_M_2017', product: {name: 'Grow Plan - Monthly'}, amount: 12500, interval: 'month', currency: 'usd'}, + {id: 'ACCELERATE_M_2017', product: {name: 'Accelerate Plan - Monthly'}, amount: 45000, interval: 'month', currency: 'usd'} + ].map {|plan| Stripe::Plan.create(plan) } + end + namespace :dev do desc "Convert Apple Ridge Farm and Judith Coleman over to Stripe in the local dev database" task switch_apple_ridge_farm: :environment do @@ -77,7 +87,7 @@ namespace :stripe do # puts "\n\n" # end - desc "Update stripe*_ids for orgs and bank accounts in a given Market" + desc "Update stripe*_ids for orgs and bank accounts in a given Market" task :update_stripe_ids_on_market do env = { 'RAILS_ENV' => 'production' } market_id = ENV['market_id'] || ENV['market'] || raise("Set market id, eg, market=18") diff --git a/spec/interactors/roll_your_own_market_spec.rb b/spec/interactors/roll_your_own_market_spec.rb index f7ec29cd7c..c9bbfbb3ac 100644 --- a/spec/interactors/roll_your_own_market_spec.rb +++ b/spec/interactors/roll_your_own_market_spec.rb @@ -8,7 +8,7 @@ # after(:all) { VCR.turn_on! } let(:mkt) { create(:market) } - + let!(:stripe_customer) { create_stripe_customer(organization: mkt) } let!(:stripe_token) { create_stripe_token } @@ -37,7 +37,7 @@ let(:subscription_params) { HashWithIndifferentAccess.new( - "plan"=>"GROW", + "plan"=>"GROW_M_2017", "plan_price"=>"700.00" ) } @@ -53,7 +53,7 @@ def perform subject.perform( market_params: market_params, - billing_params: billing_params, + billing_params: billing_params, subscription_params: subscription_params, bank_account_params: bank_account_params, amount: subscription_params[:plan_price] From cff3719348e47e4e004e719c185424a83b0ea4b0 Mon Sep 17 00:00:00 2001 From: Weston Triemstra Date: Mon, 9 Apr 2018 18:36:38 -0700 Subject: [PATCH 07/12] [LO-970] flag issues related to LO-1074, postponing for now --- spec/features/webhooks/stripe_market_transfer_paid_spec.rb | 4 +++- spec/lib/payment_provider/stripe_spec.rb | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/spec/features/webhooks/stripe_market_transfer_paid_spec.rb b/spec/features/webhooks/stripe_market_transfer_paid_spec.rb index 909784f971..f8fe1487b6 100644 --- a/spec/features/webhooks/stripe_market_transfer_paid_spec.rb +++ b/spec/features/webhooks/stripe_market_transfer_paid_spec.rb @@ -8,7 +8,7 @@ let!(:orders) { create_list(:order, 3, market: market) } # We need to brow-beat the test orders in the db to have IDs that match the hand-configured metadatain transfer.paid.json: - let(:replace_order_ids) { [ 1234, 187, 1337 ] } + let(:replace_order_ids) { [ 1234, 187, 1337 ] } before do orders.zip(replace_order_ids).each do |order, new_id| @@ -16,6 +16,8 @@ end end + # FIXME see LO-1074: when legacy cassette is deleted this fails, event has changed to transfer.created, + # and lo.order_id metadata is missing it "creates a payment and emails the market's managers" do expect(find_payments.count).to eq 0 post '/webhooks/stripe', JSON.parse(File.read('spec/features/webhooks/transfer.paid.json')) diff --git a/spec/lib/payment_provider/stripe_spec.rb b/spec/lib/payment_provider/stripe_spec.rb index bb76acdfb6..cdb62441ba 100644 --- a/spec/lib/payment_provider/stripe_spec.rb +++ b/spec/lib/payment_provider/stripe_spec.rb @@ -150,7 +150,7 @@ context "when Stripe charge fails" do it "recreates and raises the exception without a root cause (to dance around Honeybadger's unwrap_exception which occludes the cause." do - err = Stripe::InvalidRequestError.new("The message", "the_param", 123, "the http body", {the:'json body'}) + err = Stripe::InvalidRequestError.new("The message", "the_param", 123, "the http body", {the: 'json body'}) expect(Stripe::Charge).to receive(:create).and_raise(err) begin @@ -627,6 +627,8 @@ def create_refund_payment subject { described_class } describe '.order_ids_for_market_payout_transfer' do + # FIXME: see LO-1047, `metadata: {lo.order_id: NNN}` is now missing on responses from Stripe + # currently passes because we have an ancient VCR cassette it "returns lo order ids from a transaction's payments" do order_ids = subject.order_ids_for_market_payout_transfer( transfer_id: 'tr_15xxwkHouQbaP1MV8O0tEg2b', stripe_account_id: 'acct_15xJY9HouQbaP1MV') From 68d72f822ef895447deacc86b622c71c44081068 Mon Sep 17 00:00:00 2001 From: Weston Triemstra Date: Mon, 9 Apr 2018 18:51:21 -0700 Subject: [PATCH 08/12] [LO-970] Add USE_UPLOAD_QUEUE to application.yml.example for documentation, postpone removal until LO-1075 --- config/application.yml.example | 2 ++ spec/features/admin/reports_spec.rb | 6 ++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/config/application.yml.example b/config/application.yml.example index 0c9353d103..f939d96c47 100644 --- a/config/application.yml.example +++ b/config/application.yml.example @@ -39,3 +39,5 @@ development: STRIPE_DEV_MARKET_ACCOUNT_ID: MAILTRAP_USERNAME: MAILTRAP_PASSWORD: +test: + USE_UPLOAD_QUEUE: 'false' diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb index ec355fe1d1..9861d83fe6 100644 --- a/spec/features/admin/reports_spec.rb +++ b/spec/features/admin/reports_spec.rb @@ -276,7 +276,8 @@ def item_rows_for_order(order) end - scenario "can download a CSV of report" do + # FIXME: behavior was changed to a background job instead of rendered inline, fails without USE_UPLOAD_QUEUE = false + scenario "FIXME: can download a CSV of report" do items = Dom::Report::ItemRow.all html_headers = page.all(".report-table th").map(&:text) @@ -314,7 +315,8 @@ def item_rows_for_order(order) end end - scenario "can download a CSV of all records irrespective of pagniation" do + # FIXME: behavior was changed to a background job instead of rendered inline, fails without USE_UPLOAD_QUEUE = false + scenario "FIXME: can download a CSV of all records irrespective of pagniation" do category = create(:category) product = create(:product, :sellable, From 44f4e90e5dd416d3513b2fce338df465b1e34023 Mon Sep 17 00:00:00 2001 From: Weston Triemstra Date: Mon, 9 Apr 2018 18:55:48 -0700 Subject: [PATCH 09/12] [LO-970] Disable logging in test env by default to help speed up tests --- config/environments/test.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/config/environments/test.rb b/config/environments/test.rb index 6592c3690f..5158e6c3bd 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -44,4 +44,11 @@ config.assets.configure do |env| env.cache = ThreadSafe::Cache.new end + + # Disable logging by default + # from: https://jtway.co/speed-up-your-rails-test-suite-by-6-in-1-line-13fedb869ec4 + unless ENV['RAILS_ENABLE_TEST_LOG'] + config.logger = Logger.new(nil) + config.log_level = :fatal + end end From 88c5048f217d0b46f70c48d8f281a6cce51a4e2e Mon Sep 17 00:00:00 2001 From: Weston Triemstra Date: Mon, 9 Apr 2018 19:00:37 -0700 Subject: [PATCH 10/12] [LO-970] Tests should run faster without Threadsafe::Cache, try it out on CircleCI --- config/environments/test.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/config/environments/test.rb b/config/environments/test.rb index 5158e6c3bd..34ad953398 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -41,10 +41,6 @@ Delayed::Worker.delay_jobs = false - config.assets.configure do |env| - env.cache = ThreadSafe::Cache.new - end - # Disable logging by default # from: https://jtway.co/speed-up-your-rails-test-suite-by-6-in-1-line-13fedb869ec4 unless ENV['RAILS_ENABLE_TEST_LOG'] From e524a56eb4bfaed6e4a9eb3a58ba2ba6b2ae0c2a Mon Sep 17 00:00:00 2001 From: Weston Triemstra Date: Mon, 9 Apr 2018 20:39:11 -0700 Subject: [PATCH 11/12] [LO-970] Rob's fix for failing spec --- spec/features/admin/manage_organization_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/admin/manage_organization_spec.rb b/spec/features/admin/manage_organization_spec.rb index 723574d9be..5edb52c367 100644 --- a/spec/features/admin/manage_organization_spec.rb +++ b/spec/features/admin/manage_organization_spec.rb @@ -258,7 +258,7 @@ click_button("Delete Selected") expect(Dom::Admin::OrganizationLocation.count).to eq(0) - expect(page).to have_content("Successfully removed the address(es) #{location_1.name} and #{location_2.name}") + expect(page).to have_content("Successfully removed the address(es) #{location_2.name} and #{location_1.name}") end it "updates default address settings", js: true do From 85a5514c4305f582632f1a03469ba7f96ce0c290 Mon Sep 17 00:00:00 2001 From: Weston Triemstra Date: Tue, 1 May 2018 14:58:36 -0700 Subject: [PATCH 12/12] WIP on pulling api/v1/products apart from gfc queries --- Gemfile | 1 + Gemfile.lock | 2 + .../components/product_catalog.js.jsx | 5 +- .../product_catalog/product_store.js | 10 +- app/controllers/api/v1/products_controller.rb | 201 +++++++++--------- .../v1/purchaseable_products_controller.rb | 45 ++++ .../api/v1/saleable_products_controller.rb | 54 +++++ app/controllers/products_controller.rb | 14 +- .../products/alternative_order_page.html.erb | 1 + app/views/shared/_navigation.html.erb | 6 +- .../initializers/active_record_query_trace.rb | 3 + config/initializers/bullet.rb | 2 +- config/routes.rb | 23 +- .../api/v1/products_controller_spec.rb | 136 +----------- .../purchaseable_products_controller_spec.rb | 8 + .../v1/saleable_products_controller_spec.rb | 8 + .../api/v1/shared_contexts/products_search.rb | 59 +++++ .../api/v1/shared_examples/products.rb | 97 +++++++++ 18 files changed, 422 insertions(+), 253 deletions(-) create mode 100644 app/controllers/api/v1/purchaseable_products_controller.rb create mode 100644 app/controllers/api/v1/saleable_products_controller.rb create mode 100644 config/initializers/active_record_query_trace.rb create mode 100644 spec/controllers/api/v1/purchaseable_products_controller_spec.rb create mode 100644 spec/controllers/api/v1/saleable_products_controller_spec.rb create mode 100644 spec/controllers/api/v1/shared_contexts/products_search.rb create mode 100644 spec/controllers/api/v1/shared_examples/products.rb diff --git a/Gemfile b/Gemfile index 3292f04ee2..076df25aec 100644 --- a/Gemfile +++ b/Gemfile @@ -124,6 +124,7 @@ group :development do end group :development, :test do + gem 'active_record_query_trace' gem 'rspec-rails', '~> 3.0' gem 'rspec-collection_matchers' gem 'rspec_junit_formatter', :git => 'https://github.com/circleci/rspec_junit_formatter.git' diff --git a/Gemfile.lock b/Gemfile.lock index e746b2bec3..8fc881b184 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -74,6 +74,7 @@ GEM erubis (~> 2.7.0) active_model_serializers (0.9.0) activemodel (>= 3.2) + active_record_query_trace (1.5.4) active_record_union (1.0.1) activerecord (>= 4.0) activemodel (4.1.16) @@ -759,6 +760,7 @@ PLATFORMS DEPENDENCIES accountingjs-rails active_model_serializers + active_record_query_trace active_record_union activerecord-import acts_as_geocodable diff --git a/app/assets/javascripts/product_catalog/components/product_catalog.js.jsx b/app/assets/javascripts/product_catalog/components/product_catalog.js.jsx index a716953395..827cdc1f74 100644 --- a/app/assets/javascripts/product_catalog/components/product_catalog.js.jsx +++ b/app/assets/javascripts/product_catalog/components/product_catalog.js.jsx @@ -9,6 +9,7 @@ propTypes: { cartUrl: React.PropTypes.string, baseUrl: React.PropTypes.string.isRequired, + resourcesPath: React.PropTypes.string.isRequired, deliveryDate: React.PropTypes.string.isRequired, selectedType: React.PropTypes.string, orderCutoff: React.PropTypes.string.isRequired, @@ -84,7 +85,7 @@ limit={30} filter={null} cartUrl={this.props.cartUrl} - url={this.props.baseUrl + '/products'} + url={this.props.baseUrl + this.props.resourcesPath} supplierOnly={this.props.supplierId > 0} orderId={this.props.orderId} purchaseOrder={this.props.purchaseOrder} @@ -92,7 +93,7 @@ consignmentMarket={this.props.consignmentMarket} supplierView={this.props.supplierView} />); - + return (
{orderTemplates} diff --git a/app/assets/javascripts/product_catalog/product_store.js b/app/assets/javascripts/product_catalog/product_store.js index ffb9f4de66..fbd900ff19 100644 --- a/app/assets/javascripts/product_catalog/product_store.js +++ b/app/assets/javascripts/product_catalog/product_store.js @@ -19,7 +19,7 @@ products: [], hasMore: true }; - this.url = window.location.protocol + "//" + window.location.host + "/api/v1/products"; + // this.url = window.location.protocol + "//" + window.location.host + "/api/v1/products"; this.orderId = null; this.parameters = { offset: 0, @@ -45,11 +45,11 @@ ProductActions.loadProducts(); }, - loadProducts: function() { + loadProducts: function(url) { this.loading = true; this.parameters.offset = 0; this.parameters.order_id = this.orderId; - $.getJSON(this.url, this.parameters, this.onLoad, this.onLoadError); + $.getJSON(url, this.parameters, this.onLoad, this.onLoadError); }, onLoad: function(res) { @@ -57,12 +57,12 @@ this.onLoadMore(res); }, - loadMoreProducts: function() { + loadMoreProducts: function(url) { if(this.loading || !this.catalog.hasMore) return; this.loading = true; this.parameters.offset = this.catalog.products.length; this.parameters.order_id = this.orderId; - $.getJSON(this.url, this.parameters, this.onLoadMore, this.onLoadError); + $.getJSON(url, this.parameters, this.onLoadMore, this.onLoadError); }, onLoadMore: function(res) { diff --git a/app/controllers/api/v1/products_controller.rb b/app/controllers/api/v1/products_controller.rb index 5f09278870..8cdc6ceaee 100644 --- a/app/controllers/api/v1/products_controller.rb +++ b/app/controllers/api/v1/products_controller.rb @@ -13,7 +13,7 @@ class ProductsController < ApplicationController def index @offset = (params[:offset] || 0).to_i @limit = (params[:limit] || 30).to_i - @query = (params[:query] || '').gsub(/\W+/, '+') || '' + @query = (params[:query] || '').gsub(/\W+/, '+') @category_ids = (params[:category_ids] || []) @seller_ids = (params[:seller_ids] || []) @sort_by = (params[:sort_by] || "top_level_category.lft, second_level_category.lft, general_products.name") @@ -26,28 +26,31 @@ def index featured_promotion = current_market.featured_promotion(current_organization, current_delivery) end - if current_market.try(:is_consignment_market?) && (order_type == 'purchase' || (!order.nil? && order.purchase_order?)) - products = filtered_available_po_consignment_products(@query, @category_ids, @seller_ids, @order) - elsif current_market.try(:is_consignment_market?) && (order_type == 'sales' || (!order.nil? && order.sales_order?)) - products = filtered_available_so_consignment_products(@query, @category_ids, @seller_ids, @order) - else - products = filtered_available_products(@query, @category_ids, @seller_ids, @order) - end + # products = if current_market.try(:is_consignment_market?) && (order_type == 'purchase' || (!order.nil? && order.purchase_order?)) + # filtered_available_po_consignment_products(@query, @category_ids, @seller_ids, @order) + # elsif current_market.try(:is_consignment_market?) && (order_type == 'sales' || (!order.nil? && order.sales_order?)) + # filtered_available_so_consignment_products(@query, @category_ids, @seller_ids, @order) + # else + # filtered_available_products(@query, @category_ids, @seller_ids, @order) + # end + products = filtered_available_products(@query, @category_ids, @seller_ids, @order) sellers = {} + # page_of_products = products page_of_products = products - .offset(@offset) - .limit(@limit) - .map { |p| format_general_product_for_catalog(p, sellers, @order) } - render :json => { - product_total: products.count(:all), - featured_promotion: { - :details => featured_promotion, - :image_url => get_image_url(featured_promotion), - :product => featured_promotion ? format_general_product_for_catalog(featured_promotion.product.general_product, sellers, @order) : nil - }, - products: page_of_products, - sellers: sellers + .includes(:organization) + .offset(@offset) + .limit(@limit) + .map { |p| format_general_product_for_catalog(p, sellers, @order) } + render json: { + product_total: products.count(:all), + featured_promotion: { + :details => featured_promotion, + :image_url => get_image_url(featured_promotion), + :product => featured_promotion ? format_general_product_for_catalog(featured_promotion.product.general_product, sellers, @order) : nil + }, + products: page_of_products, + sellers: sellers } end @@ -93,85 +96,85 @@ def filtered_available_products(query, category_ids, seller_ids, order) .uniq end - def filtered_available_po_consignment_products(query, category_ids, seller_ids, order) - catalog_products = cross_sold_products = Product.connection.unprepared_statement do - Product.joins(organization: [market_organizations: [:market]]) - .where("markets.id = ?", current_market.id) - .visible - .select(:id, :general_product_id) - .to_sql - end - - # KXM GC: Disable cross selling products for now. - # Once re-enabled you can delete the double assignment in catalog_products above... - - # cross_sold_products = Product. - # cross_selling_list_items(current_market.id). - # visible. - # with_available_inventory(current_delivery.deliver_on). - # priced_for_market_and_buyer(current_market, current_organization). - # with_visible_pricing. - # select(:id, :general_product_id). - # to_sql - - gp = GeneralProduct.joins("JOIN (#{catalog_products}) p_child - ON general_products.id=p_child.general_product_id - JOIN categories top_level_category ON general_products.top_level_category_id = top_level_category.id - JOIN categories second_level_category ON general_products.second_level_category_id = second_level_category.id - JOIN organizations supplier ON general_products.organization_id=supplier.id") - .filter_by_current_order(order) - .filter_by_name_or_category_or_supplier(query) - .filter_by_categories(category_ids) - .filter_by_suppliers(seller_ids) - .select("top_level_category.lft, top_level_category.name, second_level_category.lft, second_level_category.name, general_products.*") - .order("general_products.name") - .uniq - end - - def filtered_available_so_consignment_products(query, category_ids, seller_ids, order) - catalog_products = cross_sold_products = Product.connection.unprepared_statement do - Product.joins(organization: [market_organizations: [:market]]) - .where("markets.id = ?", current_market.id) - .with_available_so_inventory(current_delivery.deliver_on) - .visible - .select(:id, :general_product_id) - .to_sql - end - - catalog_products2 = cross_sold_products2 = Product.connection.unprepared_statement do - Product.joins(organization: [market_organizations: [:market]]) - .where("markets.id = ?", current_market.id) - .with_pending_so_inventory(current_delivery.deliver_on) - .visible - .select(:id, :general_product_id) - .to_sql - end - - cp = "#{catalog_products} UNION #{catalog_products2} UNION - SELECT products.id, products.general_product_id - FROM products - INNER JOIN organizations ON organizations.id = products.organization_id - INNER JOIN market_organizations ON market_organizations.organization_id = organizations.id - INNER JOIN markets ON markets.id = market_organizations.market_id - INNER JOIN consignment_transactions ON consignment_transactions.product_id = products.id AND consignment_transactions.market_id = markets.id AND consignment_transactions.transaction_type = 'PO' AND consignment_transactions.lot_id IS NULL AND consignment_transactions.deleted_at IS NULL - INNER JOIN orders ON consignment_transactions.order_id = orders.id AND orders.delivery_status in ('pending','partially delivered') - WHERE markets.id = #{current_market.id} AND products.deleted_at IS NULL" - - gp = GeneralProduct.joins("JOIN (#{cp}) p_child - ON general_products.id=p_child.general_product_id - JOIN categories top_level_category ON general_products.top_level_category_id = top_level_category.id - JOIN categories second_level_category ON general_products.second_level_category_id = second_level_category.id - JOIN organizations supplier ON general_products.organization_id=supplier.id - LEFT JOIN market_organizations ON general_products.organization_id = market_organizations.organization_id - AND market_organizations.market_id = #{current_market.id}") - .filter_by_name_or_category_or_supplier(query) - .filter_by_categories(category_ids) - .filter_by_suppliers(seller_ids) - .filter_by_active_org - .select("top_level_category.lft, top_level_category.name, second_level_category.lft, second_level_category.name, general_products.*") - .order("general_products.name") - .uniq - end + # def filtered_available_po_consignment_products(query, category_ids, seller_ids, order) + # catalog_products = cross_sold_products = Product.connection.unprepared_statement do + # Product.joins(organization: [market_organizations: [:market]]) + # .where("markets.id = ?", current_market.id) + # .visible + # .select(:id, :general_product_id) + # .to_sql + # end + + # # KXM GC: Disable cross selling products for now. + # # Once re-enabled you can delete the double assignment in catalog_products above... + + # # cross_sold_products = Product. + # # cross_selling_list_items(current_market.id). + # # visible. + # # with_available_inventory(current_delivery.deliver_on). + # # priced_for_market_and_buyer(current_market, current_organization). + # # with_visible_pricing. + # # select(:id, :general_product_id). + # # to_sql + + # gp = GeneralProduct.joins("JOIN (#{catalog_products}) p_child + # ON general_products.id=p_child.general_product_id + # JOIN categories top_level_category ON general_products.top_level_category_id = top_level_category.id + # JOIN categories second_level_category ON general_products.second_level_category_id = second_level_category.id + # JOIN organizations supplier ON general_products.organization_id=supplier.id") + # .filter_by_current_order(order) + # .filter_by_name_or_category_or_supplier(query) + # .filter_by_categories(category_ids) + # .filter_by_suppliers(seller_ids) + # .select("top_level_category.lft, top_level_category.name, second_level_category.lft, second_level_category.name, general_products.*") + # .order("general_products.name") + # .uniq + # end + + # def filtered_available_so_consignment_products(query, category_ids, seller_ids, order) + # catalog_products = cross_sold_products = Product.connection.unprepared_statement do + # Product.joins(organization: [market_organizations: [:market]]) + # .where("markets.id = ?", current_market.id) + # .with_available_so_inventory(current_delivery.deliver_on) + # .visible + # .select(:id, :general_product_id) + # .to_sql + # end + + # catalog_products2 = cross_sold_products2 = Product.connection.unprepared_statement do + # Product.joins(organization: [market_organizations: [:market]]) + # .where("markets.id = ?", current_market.id) + # .with_pending_so_inventory(current_delivery.deliver_on) + # .visible + # .select(:id, :general_product_id) + # .to_sql + # end + + # cp = "#{catalog_products} UNION #{catalog_products2} UNION + # SELECT products.id, products.general_product_id + # FROM products + # INNER JOIN organizations ON organizations.id = products.organization_id + # INNER JOIN market_organizations ON market_organizations.organization_id = organizations.id + # INNER JOIN markets ON markets.id = market_organizations.market_id + # INNER JOIN consignment_transactions ON consignment_transactions.product_id = products.id AND consignment_transactions.market_id = markets.id AND consignment_transactions.transaction_type = 'PO' AND consignment_transactions.lot_id IS NULL AND consignment_transactions.deleted_at IS NULL + # INNER JOIN orders ON consignment_transactions.order_id = orders.id AND orders.delivery_status in ('pending','partially delivered') + # WHERE markets.id = #{current_market.id} AND products.deleted_at IS NULL" + + # gp = GeneralProduct.joins("JOIN (#{cp}) p_child + # ON general_products.id=p_child.general_product_id + # JOIN categories top_level_category ON general_products.top_level_category_id = top_level_category.id + # JOIN categories second_level_category ON general_products.second_level_category_id = second_level_category.id + # JOIN organizations supplier ON general_products.organization_id=supplier.id + # LEFT JOIN market_organizations ON general_products.organization_id = market_organizations.organization_id + # AND market_organizations.market_id = #{current_market.id}") + # .filter_by_name_or_category_or_supplier(query) + # .filter_by_categories(category_ids) + # .filter_by_suppliers(seller_ids) + # .filter_by_active_org + # .select("top_level_category.lft, top_level_category.name, second_level_category.lft, second_level_category.name, general_products.*") + # .order("general_products.name") + # .uniq + # end def format_general_product_for_catalog(general_product, sellers, order) general_product = general_product.decorate diff --git a/app/controllers/api/v1/purchaseable_products_controller.rb b/app/controllers/api/v1/purchaseable_products_controller.rb new file mode 100644 index 0000000000..9d2506eaa2 --- /dev/null +++ b/app/controllers/api/v1/purchaseable_products_controller.rb @@ -0,0 +1,45 @@ +module Api + module V1 + class PurchaseableProductsController < ProductsController + + private + + def filtered_available_products(query, category_ids, seller_ids, order) + catalog_products = cross_sold_products = Product.connection.unprepared_statement do + Product.joins(organization: [market_organizations: [:market]]) + .where("markets.id = ?", current_market.id) + .visible + .select(:id, :general_product_id) + .to_sql + end + + # KXM GC: Disable cross selling products for now. + # Once re-enabled you can delete the double assignment in catalog_products above... + + # cross_sold_products = Product. + # cross_selling_list_items(current_market.id). + # visible. + # with_available_inventory(current_delivery.deliver_on). + # priced_for_market_and_buyer(current_market, current_organization). + # with_visible_pricing. + # select(:id, :general_product_id). + # to_sql + + gp = GeneralProduct.joins("JOIN (#{catalog_products}) p_child + ON general_products.id=p_child.general_product_id + JOIN categories top_level_category ON general_products.top_level_category_id = top_level_category.id + JOIN categories second_level_category ON general_products.second_level_category_id = second_level_category.id + JOIN organizations supplier ON general_products.organization_id=supplier.id") + .filter_by_current_order(order) + .filter_by_name_or_category_or_supplier(query) + .filter_by_categories(category_ids) + .filter_by_suppliers(seller_ids) + .select("top_level_category.lft, top_level_category.name, second_level_category.lft, second_level_category.name, general_products.*") + .order("general_products.name") + .uniq + end + + + end + end +end diff --git a/app/controllers/api/v1/saleable_products_controller.rb b/app/controllers/api/v1/saleable_products_controller.rb new file mode 100644 index 0000000000..4520a525a8 --- /dev/null +++ b/app/controllers/api/v1/saleable_products_controller.rb @@ -0,0 +1,54 @@ +module Api + module V1 + class SaleableProductsController < ProductsController + + private + + def filtered_available_products(query, category_ids, seller_ids, order) + catalog_products = cross_sold_products = Product.connection.unprepared_statement do + Product.joins(organization: [market_organizations: [:market]]) + .where("markets.id = ?", current_market.id) + .with_available_so_inventory(current_delivery.deliver_on) + .visible + .select(:id, :general_product_id) + .to_sql + end + + catalog_products2 = cross_sold_products2 = Product.connection.unprepared_statement do + Product.joins(organization: [market_organizations: [:market]]) + .where("markets.id = ?", current_market.id) + .with_pending_so_inventory(current_delivery.deliver_on) + .visible + .select(:id, :general_product_id) + .to_sql + end + + cp = "#{catalog_products} UNION #{catalog_products2} UNION + SELECT products.id, products.general_product_id + FROM products + INNER JOIN organizations ON organizations.id = products.organization_id + INNER JOIN market_organizations ON market_organizations.organization_id = organizations.id + INNER JOIN markets ON markets.id = market_organizations.market_id + INNER JOIN consignment_transactions ON consignment_transactions.product_id = products.id AND consignment_transactions.market_id = markets.id AND consignment_transactions.transaction_type = 'PO' AND consignment_transactions.lot_id IS NULL AND consignment_transactions.deleted_at IS NULL + INNER JOIN orders ON consignment_transactions.order_id = orders.id AND orders.delivery_status in ('pending','partially delivered') + WHERE markets.id = #{current_market.id} AND products.deleted_at IS NULL" + + gp = GeneralProduct.joins("JOIN (#{cp}) p_child + ON general_products.id=p_child.general_product_id + JOIN categories top_level_category ON general_products.top_level_category_id = top_level_category.id + JOIN categories second_level_category ON general_products.second_level_category_id = second_level_category.id + JOIN organizations supplier ON general_products.organization_id=supplier.id + LEFT JOIN market_organizations ON general_products.organization_id = market_organizations.organization_id + AND market_organizations.market_id = #{current_market.id}") + .filter_by_name_or_category_or_supplier(query) + .filter_by_categories(category_ids) + .filter_by_suppliers(seller_ids) + .filter_by_active_org + .select("top_level_category.lft, top_level_category.name, second_level_category.lft, second_level_category.name, general_products.*") + .order("general_products.name") + .uniq + end + + end + end +end diff --git a/app/controllers/products_controller.rb b/app/controllers/products_controller.rb index 7a4d482105..0f981e57c0 100644 --- a/app/controllers/products_controller.rb +++ b/app/controllers/products_controller.rb @@ -14,16 +14,23 @@ class ProductsController < ApplicationController before_action :load_sellers, only: [:search] def index + @resources_path = 'products' if current_market.alternative_order_page render 'alternative_order_page' - return end end def purchase + @resources_path = 'purchaseable_products' + if current_market.alternative_order_page + render 'alternative_order_page' + end + end + + def sell + @resources_path = 'saleable_products' if current_market.alternative_order_page render 'alternative_order_page' - return end end @@ -96,9 +103,10 @@ def load_products def set_order_type @order_type = case params[:action] when 'index' then 'sales' + when 'sell' then 'sales' else params[:action] end - session[:order_type] = @order_type unless session[:order_type] == @order_type + session[:order_type] = @order_type end end diff --git a/app/views/products/alternative_order_page.html.erb b/app/views/products/alternative_order_page.html.erb index 3b0c923d9d..f678db98e0 100644 --- a/app/views/products/alternative_order_page.html.erb +++ b/app/views/products/alternative_order_page.html.erb @@ -6,6 +6,7 @@ %> <%= react_component('lo.ProductCatalog', { baseUrl: "#{request.base_url}/api/v1/", + resourcesPath: @resources_path, cartUrl: cart_path, deliveryDate: current_delivery.buyer_deliver_on, selectedType: current_delivery.selected_type.capitalize, diff --git a/app/views/shared/_navigation.html.erb b/app/views/shared/_navigation.html.erb index d1aaea0a2c..eb68739aa6 100644 --- a/app/views/shared/_navigation.html.erb +++ b/app/views/shared/_navigation.html.erb @@ -5,7 +5,7 @@ else logo = image_tag(asset_path('logo.png'), alt: "Local Orbit logo", class: "vertical-centerer logo") end -on_catalog = current_page?(products_path) || current_page?(products_purchase_path) +on_catalog = current_page?(products_path) || current_page?(purchase_products_path) || current_page?(sell_products_path) %> <% if current_market %> @@ -18,8 +18,8 @@ on_catalog = current_page?(products_path) || current_page?(products_purchase_pat
<% if current_user %>