From 44ef32f033f2dadeefbcd7c40f97630ec983902f Mon Sep 17 00:00:00 2001 From: Sharipov Ruslan Date: Tue, 13 Sep 2016 10:30:36 +0600 Subject: [PATCH 01/38] SAW: remove flag_free_sale from search endpoint --- lib/concierge/suppliers/saw/payload_builder.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/concierge/suppliers/saw/payload_builder.rb b/lib/concierge/suppliers/saw/payload_builder.rb index 7c207ff84..d35b68f9d 100644 --- a/lib/concierge/suppliers/saw/payload_builder.rb +++ b/lib/concierge/suppliers/saw/payload_builder.rb @@ -85,7 +85,6 @@ def propertysearch_request(country:, property_id: nil) #{build_username_and_password} #{country} -1 - Y #{property_id ? build_property_container(property_id) : nil } } From 1730f1893ffb86c3c9b80b09409f592fe510e388 Mon Sep 17 00:00:00 2001 From: Sharipov Ruslan Date: Tue, 13 Sep 2016 10:45:06 +0600 Subject: [PATCH 02/38] SAW: cleanup not-needed logic --- apps/workers/suppliers/saw.rb | 2 +- lib/concierge/suppliers/saw/importer.rb | 37 ------------------- .../concierge/suppliers/saw/importer_spec.rb | 36 ------------------ 3 files changed, 1 insertion(+), 74 deletions(-) diff --git a/apps/workers/suppliers/saw.rb b/apps/workers/suppliers/saw.rb index 8c39ba18e..8a3b6b22c 100644 --- a/apps/workers/suppliers/saw.rb +++ b/apps/workers/suppliers/saw.rb @@ -21,7 +21,7 @@ def perform return end - result = importer.fetch_available_properties_by_countries(countries) + result = importer.fetch_properties_by_countries(countries) if result.success? properties = result.value diff --git a/lib/concierge/suppliers/saw/importer.rb b/lib/concierge/suppliers/saw/importer.rb index 5c13116f0..5f29af69f 100644 --- a/lib/concierge/suppliers/saw/importer.rb +++ b/lib/concierge/suppliers/saw/importer.rb @@ -56,24 +56,6 @@ def fetch_properties_by_country(country) properties_fetcher.call(country) end - # Retrieves the list of available properties in a given country - # Available means those which doesn't have `on_request` attribute - # - # Returns a +Result+ wrapping +Array+ of +SAW::Entities::BasicProperty+ - # when operation succeeds - # Returns a +Result+ with +Result::Error+ when operation fails - def fetch_available_properties_by_country(country) - result = fetch_properties_by_country(country) - - if result.success? - all_properties = result.value - available_properties = all_properties.reject { |p| p.on_request? } - Result.new(available_properties) - else - result - end - end - # Retrieves the list of properties in given countries # In case if request to one of the countries fails, method stops its # execution and returns a result with a failure. @@ -93,25 +75,6 @@ def fetch_properties_by_countries(countries) Result.new(properties) end - # Retrieves the list of available properties in given countries - # In case if request to one of the countries fails, method stops its - # execution and returns a result with a failure. - # - # Returns a +Result+ wrapping +Array+ of +SAW::Entities::BasicProperty+ - # when operation succeeds - # Returns a +Result+ with +Result::Error+ when operation fails - def fetch_available_properties_by_countries(countries) - properties = countries.map do |country| - result = fetch_available_properties_by_country(country) - - return result unless result.success? - - result.value - end.flatten - - Result.new(properties) - end - # Retrieves property with extended information. # # Returns a +Result+ wrapping +SAW::Entities::DetailedProperty+ object diff --git a/spec/lib/concierge/suppliers/saw/importer_spec.rb b/spec/lib/concierge/suppliers/saw/importer_spec.rb index 044c29fad..578c12155 100644 --- a/spec/lib/concierge/suppliers/saw/importer_spec.rb +++ b/spec/lib/concierge/suppliers/saw/importer_spec.rb @@ -45,42 +45,6 @@ end end - describe "fetch_available_properties_by_country" do - it "returns full set if there wasn't any property with on request flag" do - mock_request(:country, :one) - mock_request(:propertysearch, :success) - - countries_result = subject.fetch_countries - current_country = countries_result.value.first - - properties_result = subject.fetch_available_properties_by_country( - current_country - ) - expect(properties_result.success?).to be true - expect(properties_result.value.size).to eq(5) - expect(properties_result.value).to all( - be_kind_of(SAW::Entities::BasicProperty) - ) - end - - it "filters out properties with on request flag" do - mock_request(:country, :one) - mock_request(:propertysearch, :success_with_on_request) - - countries_result = subject.fetch_countries - current_country = countries_result.value.first - - properties_result = subject.fetch_available_properties_by_country( - current_country - ) - expect(properties_result.success?).to be true - expect(properties_result.value.size).to eq(4) - expect(properties_result.value).to all( - be_kind_of(SAW::Entities::BasicProperty) - ) - end - end - describe "fetch_properties_by_countries" do it "returns a result with an empty array when all requests are empty" do mock_request(:country, :multiple) From a290fba3fac5ee2c22a732ced943949a1de447aa Mon Sep 17 00:00:00 2001 From: Sharipov Ruslan Date: Tue, 13 Sep 2016 11:45:08 +0600 Subject: [PATCH 03/38] SAW: documentation update --- lib/concierge/suppliers/saw/importer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/concierge/suppliers/saw/importer.rb b/lib/concierge/suppliers/saw/importer.rb index 5f29af69f..c306e2a52 100644 --- a/lib/concierge/suppliers/saw/importer.rb +++ b/lib/concierge/suppliers/saw/importer.rb @@ -95,7 +95,7 @@ def fetch_unit_rates_by_property_ids(ids) bulk_rates_fetcher.call(ids) end - # Retrieves rates for properties + # Retrieves rates for units of all given properties. # # It groups properties by currency because SAW API doesn't support # fetching property rates when multiple currencies are given. From 8e9d96a5ae9eb1863c3b3ab2539f6836b967d32c Mon Sep 17 00:00:00 2001 From: Sharipov Ruslan Date: Tue, 13 Sep 2016 11:46:38 +0600 Subject: [PATCH 04/38] SAW: update error message when fetching unit rates fails --- apps/workers/suppliers/saw.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/workers/suppliers/saw.rb b/apps/workers/suppliers/saw.rb index 8a3b6b22c..5db25721a 100644 --- a/apps/workers/suppliers/saw.rb +++ b/apps/workers/suppliers/saw.rb @@ -36,7 +36,7 @@ def perform if result.success? all_unit_rates = result.value else - message = "Failed to perform the `#fetch_rates_for_properties` operation" + message = "Failed to perform the `#fetch_all_unit_rates_for_properties` operation" announce_error(message, result) return end From e218f66811b6d31e19a672c4efaac8abb882f526 Mon Sep 17 00:00:00 2001 From: Sharipov Ruslan Date: Tue, 13 Sep 2016 11:47:52 +0600 Subject: [PATCH 05/38] SAW: better variable name for clarity --- apps/workers/suppliers/saw.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/workers/suppliers/saw.rb b/apps/workers/suppliers/saw.rb index 5db25721a..dd839f667 100644 --- a/apps/workers/suppliers/saw.rb +++ b/apps/workers/suppliers/saw.rb @@ -53,7 +53,7 @@ def perform synchronisation.finish! end - def fetch_details_and_build_property(property, rates) + def fetch_details_and_build_property(property, unit_rates) result = importer.fetch_detailed_property(property.internal_id) if result.success? @@ -62,7 +62,7 @@ def fetch_details_and_build_property(property, rates) roomorama_property = ::SAW::Mappers::RoomoramaProperty.build( property, detailed_property, - rates + unit_rates ) Result.new(roomorama_property) From d36b7b91bdcaf8ca6a6504e66fa2ca0232121e1d Mon Sep 17 00:00:00 2001 From: Sharipov Ruslan Date: Tue, 13 Sep 2016 11:57:32 +0600 Subject: [PATCH 06/38] SAW: more strict spec --- .../suppliers/saw/mappers/roomorama_property_mapper_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/concierge/suppliers/saw/mappers/roomorama_property_mapper_spec.rb b/spec/lib/concierge/suppliers/saw/mappers/roomorama_property_mapper_spec.rb index 576992528..9afca5901 100644 --- a/spec/lib/concierge/suppliers/saw/mappers/roomorama_property_mapper_spec.rb +++ b/spec/lib/concierge/suppliers/saw/mappers/roomorama_property_mapper_spec.rb @@ -208,7 +208,7 @@ rates ) - expect(property.units).not_to eq([]) + expect(property.units.size).to eq(6) expect(property.units).to all(be_kind_of(Roomorama::Unit)) end end From cf62f4e933fb320e855eef26afdd7045f1f87897 Mon Sep 17 00:00:00 2001 From: Sharipov Ruslan Date: Tue, 13 Sep 2016 14:59:58 +0600 Subject: [PATCH 07/38] SAW: return [] instead of Result.error if rates are not available --- lib/concierge/suppliers/saw/commands/base_fetcher.rb | 3 ++- .../saw/commands/bulk_rates_fetcher_spec.rb | 12 ++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/concierge/suppliers/saw/commands/base_fetcher.rb b/lib/concierge/suppliers/saw/commands/base_fetcher.rb index 092b46d38..cff3e384d 100644 --- a/lib/concierge/suppliers/saw/commands/base_fetcher.rb +++ b/lib/concierge/suppliers/saw/commands/base_fetcher.rb @@ -6,7 +6,8 @@ class BaseFetcher # # Whitelisted SAW API errors: # 1007 - No properties are available for the given search parameters - VALID_RESULT_ERROR_CODES = ['1007'] + # 3031 - Rates are not available for this property + VALID_RESULT_ERROR_CODES = ['1007', '3031'] attr_reader :credentials diff --git a/spec/lib/concierge/suppliers/saw/commands/bulk_rates_fetcher_spec.rb b/spec/lib/concierge/suppliers/saw/commands/bulk_rates_fetcher_spec.rb index 0c1e197e8..f7e83bbf3 100644 --- a/spec/lib/concierge/suppliers/saw/commands/bulk_rates_fetcher_spec.rb +++ b/spec/lib/concierge/suppliers/saw/commands/bulk_rates_fetcher_spec.rb @@ -34,6 +34,18 @@ expect(rates).to all(be_kind_of(SAW::Entities::PropertyRate)) end + it "returns a result with an empty array if all rates are unavailable" do + mock_request(:propertyrates, :rates_not_available) + + result = subject.call(property_ids) + expect(result).to be_success + + rates = result.value + + expect(rates).to be_kind_of(Array) + expect(rates.size).to eq(0) + end + it "returns result with error after error" do mock_request(:propertyrates, :error) From fafd8f4b019ad4a0883212c8fe195973e46801c5 Mon Sep 17 00:00:00 2001 From: Sharipov Ruslan Date: Tue, 13 Sep 2016 15:00:15 +0600 Subject: [PATCH 08/38] SAW: disable context --- apps/workers/suppliers/saw.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/apps/workers/suppliers/saw.rb b/apps/workers/suppliers/saw.rb index dd839f667..b8fd8ed3c 100644 --- a/apps/workers/suppliers/saw.rb +++ b/apps/workers/suppliers/saw.rb @@ -43,8 +43,6 @@ def perform properties.each do |property| synchronisation.start(property.internal_id) do - Concierge.context.disable! - unit_rates = find_rates(property.internal_id, all_unit_rates) fetch_details_and_build_property(property, unit_rates) end From 9e8282b0256ec4588355ae7837b474d00e2c3ca9 Mon Sep 17 00:00:00 2001 From: Sharipov Ruslan Date: Tue, 13 Sep 2016 15:28:42 +0600 Subject: [PATCH 09/38] SAW: adjust quote specs according latest changes --- spec/api/controllers/saw/quote_spec.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/spec/api/controllers/saw/quote_spec.rb b/spec/api/controllers/saw/quote_spec.rb index a06eb0da7..b6cb37812 100644 --- a/spec/api/controllers/saw/quote_spec.rb +++ b/spec/api/controllers/saw/quote_spec.rb @@ -104,9 +104,17 @@ def provoke_failure! result = controller.quote_price(params) - expect(result.success?).to be false + expect(result.success?).to be true expect(result).to be_kind_of(Result) - expect(result.value).to be nil + + quotation = result.value + expect(quotation).to be_kind_of(Quotation) + expect(quotation.property_id).to eq(params[:property_id]) + expect(quotation.unit_id).to eq(params[:unit_id]) + expect(quotation.check_in).to eq(params[:check_in]) + expect(quotation.check_out).to eq(params[:check_out]) + expect(quotation.guests).to eq(params[:guests]) + expect(quotation.available).to be false end end From 172d276b95635054e2e9470024881f708d976b18 Mon Sep 17 00:00:00 2001 From: Sharipov Ruslan Date: Wed, 14 Sep 2016 15:48:24 +0600 Subject: [PATCH 10/38] Update unit rates fetching strategy --- .../saw/commands/bulk_rates_fetcher.rb | 6 +++--- .../suppliers/saw/commands/price_fetcher.rb | 2 +- .../suppliers/saw/mappers/property_rate.rb | 14 +++++++++++--- .../saw/mappers/property_rate_spec.rb | 19 +++++++++++++++++++ 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/lib/concierge/suppliers/saw/commands/bulk_rates_fetcher.rb b/lib/concierge/suppliers/saw/commands/bulk_rates_fetcher.rb index 0251d2b85..57c7a64c6 100644 --- a/lib/concierge/suppliers/saw/commands/bulk_rates_fetcher.rb +++ b/lib/concierge/suppliers/saw/commands/bulk_rates_fetcher.rb @@ -46,7 +46,7 @@ def build_rates(rates_hash) Array(rates).map do |rate| safe_hash = Concierge::SafeAccessHash.new(rate) SAW::Mappers::PropertyRate.build(safe_hash) - end + end.compact end def build_payload(ids) @@ -59,11 +59,11 @@ def build_payload(ids) end def check_in - (today + 30 * one_day).strftime("%d/%m/%Y") + (today + 90 * one_day).strftime("%d/%m/%Y") end def check_out - (today + 31 * one_day).strftime("%d/%m/%Y") + (today + 92 * one_day).strftime("%d/%m/%Y") end def one_day diff --git a/lib/concierge/suppliers/saw/commands/price_fetcher.rb b/lib/concierge/suppliers/saw/commands/price_fetcher.rb index 22cfed51a..5bacfeca0 100644 --- a/lib/concierge/suppliers/saw/commands/price_fetcher.rb +++ b/lib/concierge/suppliers/saw/commands/price_fetcher.rb @@ -46,7 +46,7 @@ def call(params) if valid_result?(result_hash) property_rate = build_property_rate(result_hash) - quotation = if property_rate.has_unit?(params[:unit_id]) + quotation = if property_rate && property_rate.has_unit?(params[:unit_id]) SAW::Mappers::Quotation.build(params, property_rate) else SAW::Mappers::Quotation.build_unavailable(params) diff --git a/lib/concierge/suppliers/saw/mappers/property_rate.rb b/lib/concierge/suppliers/saw/mappers/property_rate.rb index 58e1b62e8..f038e0d1b 100644 --- a/lib/concierge/suppliers/saw/mappers/property_rate.rb +++ b/lib/concierge/suppliers/saw/mappers/property_rate.rb @@ -15,6 +15,8 @@ class << self # # Returns [SAW::Entities::PropertyRate] def build(hash) + return nil if empty_unit_rates?(hash) + Entities::PropertyRate.new( id: hash.get("@id"), units: build_units(hash), @@ -28,10 +30,16 @@ def parse_currency(hash) hash.get("currency_code") end + def empty_unit_rates?(hash) + units_hash(hash).nil? + end + + def units_hash(hash) + hash.get("apartments.accommodation_type.property_accommodation") + end + def build_units(hash) - units = hash.get( - "apartments.accommodation_type.property_accommodation" - ) + units = units_hash(hash) Array(units).map do |unit_hash| safe_hash = Concierge::SafeAccessHash.new(unit_hash) diff --git a/spec/lib/concierge/suppliers/saw/mappers/property_rate_spec.rb b/spec/lib/concierge/suppliers/saw/mappers/property_rate_spec.rb index 2c6559bde..f20e2ce71 100644 --- a/spec/lib/concierge/suppliers/saw/mappers/property_rate_spec.rb +++ b/spec/lib/concierge/suppliers/saw/mappers/property_rate_spec.rb @@ -76,5 +76,24 @@ module SAW expect(unit.available).to eq(true) expect(unit.max_guests).to eq(4) end + + describe "when rates are not available" do + let(:attributes) do + { + "name"=>"Adagio Access Avignon", + "check_in"=>"01/09/2016", + "check_out"=>"02/09/2016", + "currency_code"=>"EUR", + "default_currency_code"=>"EUR", + "apartments"=> {} + } + end + + it "returns nil object" do + property_rate = described_class.build(hash) + + expect(property_rate).to be_nil + end + end end end From 2a6089e3f641a600db6decfcff1092ac9f1552bc Mon Sep 17 00:00:00 2001 From: Keang Date: Mon, 19 Sep 2016 17:16:21 +0800 Subject: [PATCH 11/38] Return result instead of nil --- .../suppliers/saw/commands/price_fetcher.rb | 12 +++++----- .../suppliers/saw/mappers/property_rate.rb | 8 +++---- .../suppliers/saw/mappers/quotation.rb | 24 +++++++++++-------- .../saw/commands/bulk_rates_fetcher_spec.rb | 10 ++++++-- .../saw/mappers/property_rate_spec.rb | 7 +++--- 5 files changed, 36 insertions(+), 25 deletions(-) diff --git a/lib/concierge/suppliers/saw/commands/price_fetcher.rb b/lib/concierge/suppliers/saw/commands/price_fetcher.rb index 5bacfeca0..f42b09b66 100644 --- a/lib/concierge/suppliers/saw/commands/price_fetcher.rb +++ b/lib/concierge/suppliers/saw/commands/price_fetcher.rb @@ -44,15 +44,15 @@ def call(params) result_hash = response_parser.to_hash(result.value.body) if valid_result?(result_hash) - property_rate = build_property_rate(result_hash) + property_rate_res = build_property_rate(result_hash) - quotation = if property_rate && property_rate.has_unit?(params[:unit_id]) - SAW::Mappers::Quotation.build(params, property_rate) + if property_rate_res.success? + Result.new SAW::Mappers::Quotation.build(params, property_rate_res.value) + elsif property_rate_res.error.code == :empty_unit_rates + Result.new SAW::Mappers::Quotation.build_unavailable(params) else - SAW::Mappers::Quotation.build_unavailable(params) + property_rate_res end - - Result.new(quotation) else error_result(result_hash) end diff --git a/lib/concierge/suppliers/saw/mappers/property_rate.rb b/lib/concierge/suppliers/saw/mappers/property_rate.rb index f038e0d1b..a04c30273 100644 --- a/lib/concierge/suppliers/saw/mappers/property_rate.rb +++ b/lib/concierge/suppliers/saw/mappers/property_rate.rb @@ -13,15 +13,15 @@ class << self # * +hash+ [Concierge::SafeAccessHash] property rate object # attributes # - # Returns [SAW::Entities::PropertyRate] + # Returns [Result] wrapping [SAW::Entities::PropertyRate] def build(hash) - return nil if empty_unit_rates?(hash) + return Result.error(:empty_unit_rates) if empty_unit_rates?(hash) - Entities::PropertyRate.new( + Result.new(Entities::PropertyRate.new( id: hash.get("@id"), units: build_units(hash), currency: parse_currency(hash) - ) + )) end private diff --git a/lib/concierge/suppliers/saw/mappers/quotation.rb b/lib/concierge/suppliers/saw/mappers/quotation.rb index e1e61420e..2d0711c5e 100644 --- a/lib/concierge/suppliers/saw/mappers/quotation.rb +++ b/lib/concierge/suppliers/saw/mappers/quotation.rb @@ -15,16 +15,20 @@ class Quotation def self.build(params, property_rate) requested_unit = property_rate.find_unit(params[:unit_id]) - ::Quotation.new( - property_id: params[:property_id], - unit_id: params[:unit_id], - check_in: params[:check_in].to_s, - check_out: params[:check_out].to_s, - guests: params[:guests], - currency: property_rate.currency, - total: requested_unit.price, - available: requested_unit.available - ) + if requested_unit + ::Quotation.new( + property_id: params[:property_id], + unit_id: params[:unit_id], + check_in: params[:check_in].to_s, + check_out: params[:check_out].to_s, + guests: params[:guests], + currency: property_rate.currency, + total: requested_unit.price, + available: requested_unit.available + ) + else + self.build_unavailable(params) + end end # Builds unavailable quotation. diff --git a/spec/lib/concierge/suppliers/saw/commands/bulk_rates_fetcher_spec.rb b/spec/lib/concierge/suppliers/saw/commands/bulk_rates_fetcher_spec.rb index f7e83bbf3..899a9bc3c 100644 --- a/spec/lib/concierge/suppliers/saw/commands/bulk_rates_fetcher_spec.rb +++ b/spec/lib/concierge/suppliers/saw/commands/bulk_rates_fetcher_spec.rb @@ -19,7 +19,10 @@ rates = result.value expect(rates).to be_kind_of(Array) expect(rates.size).to eq(2) - expect(rates).to all(be_kind_of(SAW::Entities::PropertyRate)) + expect(rates).to all(be_kind_of(Result)) + rates.each do |r| + expect(r).to be_success + end end it "returns rates for single property id" do @@ -31,7 +34,10 @@ rates = result.value expect(rates).to be_kind_of(Array) expect(rates.size).to eq(1) - expect(rates).to all(be_kind_of(SAW::Entities::PropertyRate)) + expect(rates).to all(be_kind_of(Result)) + rates.each do |r| + expect(r).to be_success + end end it "returns a result with an empty array if all rates are unavailable" do diff --git a/spec/lib/concierge/suppliers/saw/mappers/property_rate_spec.rb b/spec/lib/concierge/suppliers/saw/mappers/property_rate_spec.rb index f20e2ce71..898b25b32 100644 --- a/spec/lib/concierge/suppliers/saw/mappers/property_rate_spec.rb +++ b/spec/lib/concierge/suppliers/saw/mappers/property_rate_spec.rb @@ -63,7 +63,7 @@ module SAW let(:hash) { Concierge::SafeAccessHash.new(attributes) } it "builds available property rate object" do - property_rate = described_class.build(hash) + property_rate = described_class.build(hash).value expect(property_rate).to be_kind_of(SAW::Entities::PropertyRate) expect(property_rate.id).to eq("2596") expect(property_rate.currency).to eq("EUR") @@ -89,10 +89,11 @@ module SAW } end - it "returns nil object" do + it "returns Result with error object" do property_rate = described_class.build(hash) - expect(property_rate).to be_nil + expect(property_rate).to be_a Result + expect(property_rate.error.code).to eq :empty_unit_rates end end end From 1be93c56a1f1626a13e4dc8ae01b5b8d5d9746cb Mon Sep 17 00:00:00 2001 From: Sharipov Ruslan Date: Tue, 20 Sep 2016 12:25:18 +0600 Subject: [PATCH 12/38] SAW: separate logic for quote price and bulk price fetcher --- .../saw/commands/bulk_rates_fetcher.rb | 19 ++-- .../suppliers/saw/commands/price_fetcher.rb | 26 ++--- .../suppliers/saw/entities/property_rate.rb | 45 -------- .../suppliers/saw/entities/unit_rate.rb | 2 +- .../suppliers/saw/entities/units_pricing.rb | 45 ++++++++ lib/concierge/suppliers/saw/importer.rb | 4 +- .../suppliers/saw/mappers/quotation.rb | 100 ++++++++++++------ .../{property_rate.rb => units_pricing.rb} | 43 +++++--- .../saw/commands/bulk_rates_fetcher_spec.rb | 10 +- .../saw/mappers/roomorama_unit_set_spec.rb | 4 +- ...rty_rate_spec.rb => units_pricing_spec.rb} | 50 +++++++-- 11 files changed, 205 insertions(+), 143 deletions(-) delete mode 100644 lib/concierge/suppliers/saw/entities/property_rate.rb create mode 100644 lib/concierge/suppliers/saw/entities/units_pricing.rb rename lib/concierge/suppliers/saw/mappers/{property_rate.rb => units_pricing.rb} (52%) rename spec/lib/concierge/suppliers/saw/mappers/{property_rate_spec.rb => units_pricing_spec.rb} (62%) diff --git a/lib/concierge/suppliers/saw/commands/bulk_rates_fetcher.rb b/lib/concierge/suppliers/saw/commands/bulk_rates_fetcher.rb index 57c7a64c6..e3cd7e57c 100644 --- a/lib/concierge/suppliers/saw/commands/bulk_rates_fetcher.rb +++ b/lib/concierge/suppliers/saw/commands/bulk_rates_fetcher.rb @@ -10,6 +10,12 @@ module Commands # command = SAW::Commands::BulkRatesFetcher.new(credentials) # result = command.call(property_id) class BulkRatesFetcher < BaseFetcher + # How many days in future skip to fetch rates. + STAY_OFFSET = 90 + + # SAW returns rates for more properties if chosen STAY_LENGTH is 2 days + STAY_LENGTH = 2 + # Calls the SAW API method using the HTTP client. # # Arguments @@ -39,14 +45,11 @@ def call(ids) private def build_rates(rates_hash) - rates = rates_hash.get("response.property") + rates_array = Array(rates_hash.get("response.property")) - return [] unless rates + return rates_array unless rates_array.any? - Array(rates).map do |rate| - safe_hash = Concierge::SafeAccessHash.new(rate) - SAW::Mappers::PropertyRate.build(safe_hash) - end.compact + SAW::Mappers::UnitsPricing.build(rates_array, STAY_LENGTH) end def build_payload(ids) @@ -59,11 +62,11 @@ def build_payload(ids) end def check_in - (today + 90 * one_day).strftime("%d/%m/%Y") + (today + STAY_OFFSET * one_day).strftime("%d/%m/%Y") end def check_out - (today + 92 * one_day).strftime("%d/%m/%Y") + (today + (STAY_OFFSET+STAY_LENGTH) * one_day).strftime("%d/%m/%Y") end def one_day diff --git a/lib/concierge/suppliers/saw/commands/price_fetcher.rb b/lib/concierge/suppliers/saw/commands/price_fetcher.rb index f42b09b66..ea343f330 100644 --- a/lib/concierge/suppliers/saw/commands/price_fetcher.rb +++ b/lib/concierge/suppliers/saw/commands/price_fetcher.rb @@ -44,15 +44,11 @@ def call(params) result_hash = response_parser.to_hash(result.value.body) if valid_result?(result_hash) - property_rate_res = build_property_rate(result_hash) + quotation = build_quotation(params, result_hash) - if property_rate_res.success? - Result.new SAW::Mappers::Quotation.build(params, property_rate_res.value) - elsif property_rate_res.error.code == :empty_unit_rates - Result.new SAW::Mappers::Quotation.build_unavailable(params) - else - property_rate_res - end + return Result.error(:empty_unit_rates) unless quotation + + Result.new(quotation) else error_result(result_hash) end @@ -62,13 +58,6 @@ def call(params) end private - def build_property_rate(hash) - rates_hash = hash.get("response.property") - safe_hash = Concierge::SafeAccessHash.new(rates_hash) - - SAW::Mappers::PropertyRate.build(safe_hash) - end - def build_payload(params) payload_builder.build_compute_pricing( property_id: params[:property_id], @@ -78,6 +67,13 @@ def build_payload(params) num_guests: params[:guests] ) end + + def build_quotation(params, result_hash) + rates_hash = result_hash.get("response.property") + safe_hash = Concierge::SafeAccessHash.new(rates_hash) + + SAW::Mappers::Quotation.build(params, safe_hash) + end end end end diff --git a/lib/concierge/suppliers/saw/entities/property_rate.rb b/lib/concierge/suppliers/saw/entities/property_rate.rb deleted file mode 100644 index 0a526c89a..000000000 --- a/lib/concierge/suppliers/saw/entities/property_rate.rb +++ /dev/null @@ -1,45 +0,0 @@ -module SAW - module Entities - # +SAW::Entities::PropertyRate+ - # - # This entity represents an object with available rates for property - # units: entity includes +units+ array which has unit rates and the - # currency used for the current property - # - # Attributes - # - # +id+ - property id - # +units+ - array of SAW::Entities::UnitRate objects - # +current+ - currency code - class PropertyRate - attr_reader :id, :units, :currency - - def initialize(id:, units:, currency:) - @id = id - @units = units - @currency = currency - end - - # Find unit by +id+ - # - # Arguments - # * +id+ - # - # Returns [SAW::Entities::UnitRate] - def find_unit(id) - units.detect { |u| u.id == id } - end - - # Determines whether unit with given +id+ is present in property rate - # object or not. - # - # Arguments - # * +id+ - # - # Returns [Boolean] flag indicating the presence of rates for unit - def has_unit?(id) - !!find_unit(id) - end - end - end -end diff --git a/lib/concierge/suppliers/saw/entities/unit_rate.rb b/lib/concierge/suppliers/saw/entities/unit_rate.rb index aab25c72c..5e92f8e46 100644 --- a/lib/concierge/suppliers/saw/entities/unit_rate.rb +++ b/lib/concierge/suppliers/saw/entities/unit_rate.rb @@ -1,6 +1,6 @@ module SAW module Entities - # +SAW::Entities::PropertyRate+ + # +SAW::Entities::UnitRate+ # # This entity represents an object with available rates for units in # property. diff --git a/lib/concierge/suppliers/saw/entities/units_pricing.rb b/lib/concierge/suppliers/saw/entities/units_pricing.rb new file mode 100644 index 000000000..477c98f7e --- /dev/null +++ b/lib/concierge/suppliers/saw/entities/units_pricing.rb @@ -0,0 +1,45 @@ +module SAW + module Entities + # +SAW::Entities::UnitsPricing+ + # + # This entity represents an object with available rates for property + # units: entity includes +units+ array which has unit rates and the + # currency used for the current property + # + # Attributes + # + # +property_id+ - property id + # +currency+ - currency code + # +units+ - array of +SAW::Entities::UnitRate+ objects + class UnitsPricing + attr_reader :property_id, :units, :currency + + def initialize(property_id:, units:, currency:) + @property_id = property_id + @units = units + @currency = currency + end + + # Find unit rate by +unit_id+ + # + # Arguments + # * +unit_id+ + # + # Returns [SAW::Entities::UnitRate] + def find_unit_rate(unit_id) + units.detect { |u| u.id == unit_id } + end + + # Determines whether unit with given +uni_id+ is present in units pricing + # object or not. + # + # Arguments + # * +unit_id+ + # + # Returns [Boolean] flag indicating the presence of rates for unit + def has_rates_for_unit?(unit_id) + !!find_unit(unit_id) + end + end + end +end diff --git a/lib/concierge/suppliers/saw/importer.rb b/lib/concierge/suppliers/saw/importer.rb index c306e2a52..3decb37d0 100644 --- a/lib/concierge/suppliers/saw/importer.rb +++ b/lib/concierge/suppliers/saw/importer.rb @@ -87,7 +87,7 @@ def fetch_detailed_property(property_id) # Retrieves rates for units by given property ids # - # Returns a +Result+ wrapping an array of +SAW::Entities::PropertyRate+ + # Returns a +Result+ wrapping an array of +SAW::Entities::UnitsPricing+ # objects when operation succeeds # Returns a +Result+ with +Result::Error+ when operation fails def fetch_unit_rates_by_property_ids(ids) @@ -102,7 +102,7 @@ def fetch_unit_rates_by_property_ids(ids) # # Limits to send maximum 20 property ids (API limitation) # - # Returns a +Result+ wrapping an array of +SAW::Entities::PropertyRate+ + # Returns a +Result+ wrapping an array of +SAW::Entities::UnitsPricing+ # objects when operation succeeds # Returns a +Result+ with +Result::Error+ when operation fails def fetch_all_unit_rates_for_properties(properties) diff --git a/lib/concierge/suppliers/saw/mappers/quotation.rb b/lib/concierge/suppliers/saw/mappers/quotation.rb index 2d0711c5e..750241d6e 100644 --- a/lib/concierge/suppliers/saw/mappers/quotation.rb +++ b/lib/concierge/suppliers/saw/mappers/quotation.rb @@ -4,50 +4,80 @@ module Mappers # # This class is responsible for building a +Quotation+ object class Quotation - # Builds a quotation - # - # Arguments: - # - # * +params+ [Hash] property parameters - # * +property_rate+ [SAW::Entities::PropertyRate] property rate - # - # Returns [Quotation] - def self.build(params, property_rate) - requested_unit = property_rate.find_unit(params[:unit_id]) - - if requested_unit + class << self + # Builds a quotation + # + # Arguments: + # + # * +params+ [Hash] quotation request parameters + # * +safe_hash+ [Concierge::SafeAccessHash] result hash with price + # + # Returns [Quotation] + def build(params, safe_hash) + requested_unit = find_requested_unit(safe_hash, params[:unit_id]) + + if requested_unit + ::Quotation.new( + property_id: params[:property_id], + unit_id: params[:unit_id], + check_in: params[:check_in].to_s, + check_out: params[:check_out].to_s, + guests: params[:guests], + currency: parse_currency(safe_hash), + total: parse_price(requested_unit), + available: parse_availability(requested_unit) + ) + else + build_unavailable(params) + end + end + + private + # Builds unavailable quotation. + # Used in cases when rates information for unit is not available. + # + # Arguments: + # + # * +params+ [Hash] parameters + # + # Returns [Quotation] + def build_unavailable(params) ::Quotation.new( property_id: params[:property_id], unit_id: params[:unit_id], check_in: params[:check_in].to_s, check_out: params[:check_out].to_s, guests: params[:guests], - currency: property_rate.currency, - total: requested_unit.price, - available: requested_unit.available + available: false ) - else - self.build_unavailable(params) end - end - # Builds unavailable quotation. - # Used in cases when rates information for unit is not available. - # - # Arguments: - # - # * +params+ [Hash] parameters - # - # Returns [Quotation] - def self.build_unavailable(params) - ::Quotation.new( - property_id: params[:property_id], - unit_id: params[:unit_id], - check_in: params[:check_in].to_s, - check_out: params[:check_out].to_s, - guests: params[:guests], - available: false - ) + def units_hash(hash) + hash.get("apartments.accommodation_type.property_accommodation") + end + + def find_requested_unit(safe_hash, id) + units = units_hash(safe_hash) + requested_unit_hash = Array(units).find { |u| u["@id"] == id } + + return nil unless requested_unit_hash + Concierge::SafeAccessHash.new(requested_unit_hash) + end + + def parse_currency(hash) + hash.get("currency_code") + end + + def parse_price(hash) + price = hash.get("price_detail.net.total_price.price") + BigDecimal.new(price) + end + + def parse_availability(hash) + flag = hash.get("flag_bookable_property_accommodation") + + flag == "Y" + end end end end diff --git a/lib/concierge/suppliers/saw/mappers/property_rate.rb b/lib/concierge/suppliers/saw/mappers/units_pricing.rb similarity index 52% rename from lib/concierge/suppliers/saw/mappers/property_rate.rb rename to lib/concierge/suppliers/saw/mappers/units_pricing.rb index a04c30273..0fe62729d 100644 --- a/lib/concierge/suppliers/saw/mappers/property_rate.rb +++ b/lib/concierge/suppliers/saw/mappers/units_pricing.rb @@ -1,30 +1,37 @@ module SAW module Mappers - # +SAW::Mappers::PropertyRate+ + # +SAW::Mappers::UnitsPricing+ # - # This class is responsible for building a +SAW::Entities::PropertyRate+ + # This class is responsible for building a +SAW::Entities::UnitsPricing+ # object from the hash which was fetched from the SAW API. - class PropertyRate + class UnitsPricing class << self # Builds a property rate object # # Arguments: # - # * +hash+ [Concierge::SafeAccessHash] property rate object - # attributes + # * +rates+ [Array] array with rates hashes + # * +stay_length+ [Integer] number of days for which rates are + # fetched for # - # Returns [Result] wrapping [SAW::Entities::PropertyRate] - def build(hash) - return Result.error(:empty_unit_rates) if empty_unit_rates?(hash) - - Result.new(Entities::PropertyRate.new( - id: hash.get("@id"), - units: build_units(hash), - currency: parse_currency(hash) - )) + # Returns [Array] wrapping [SAW::Entities::UnitsPricing] objects + def build(rates, stay_length) + rates.map do |rate| + safe_hash = Concierge::SafeAccessHash.new(rate) + build_units_pricing(safe_hash, stay_length) + end.compact end private + def build_units_pricing(hash, stay_length) + return nil if empty_unit_rates?(hash) + + Entities::UnitsPricing.new( + property_id: hash.get("@id"), + units: build_units(hash, stay_length), + currency: parse_currency(hash) + ) + end def parse_currency(hash) hash.get("currency_code") @@ -38,15 +45,19 @@ def units_hash(hash) hash.get("apartments.accommodation_type.property_accommodation") end - def build_units(hash) + def build_units(hash, stay_length) units = units_hash(hash) Array(units).map do |unit_hash| safe_hash = Concierge::SafeAccessHash.new(unit_hash) + price_per_period = parse_price(safe_hash) + + # since it's average prices, we don't need high precision + price_per_night = (price_per_period / stay_length).round(2) Entities::UnitRate.new( id: safe_hash.get("@id"), - price: parse_price(safe_hash), + price: price_per_night, available: parse_availability(safe_hash), max_guests: parse_max_guests(safe_hash) ) diff --git a/spec/lib/concierge/suppliers/saw/commands/bulk_rates_fetcher_spec.rb b/spec/lib/concierge/suppliers/saw/commands/bulk_rates_fetcher_spec.rb index 899a9bc3c..87f6f4db1 100644 --- a/spec/lib/concierge/suppliers/saw/commands/bulk_rates_fetcher_spec.rb +++ b/spec/lib/concierge/suppliers/saw/commands/bulk_rates_fetcher_spec.rb @@ -19,10 +19,7 @@ rates = result.value expect(rates).to be_kind_of(Array) expect(rates.size).to eq(2) - expect(rates).to all(be_kind_of(Result)) - rates.each do |r| - expect(r).to be_success - end + expect(rates).to all(be_kind_of(SAW::Entities::UnitsPricing)) end it "returns rates for single property id" do @@ -34,10 +31,7 @@ rates = result.value expect(rates).to be_kind_of(Array) expect(rates.size).to eq(1) - expect(rates).to all(be_kind_of(Result)) - rates.each do |r| - expect(r).to be_success - end + expect(rates).to all(be_kind_of(SAW::Entities::UnitsPricing)) end it "returns a result with an empty array if all rates are unavailable" do diff --git a/spec/lib/concierge/suppliers/saw/mappers/roomorama_unit_set_spec.rb b/spec/lib/concierge/suppliers/saw/mappers/roomorama_unit_set_spec.rb index 0da28d011..aabf7cf67 100644 --- a/spec/lib/concierge/suppliers/saw/mappers/roomorama_unit_set_spec.rb +++ b/spec/lib/concierge/suppliers/saw/mappers/roomorama_unit_set_spec.rb @@ -367,8 +367,8 @@ module SAW context "when custom property rates are given" do let(:unit_rates) do - SAW::Entities::PropertyRate.new( - id: '123', + SAW::Entities::UnitsPricing.new( + property_id: '123', currency: 'USD', units: [ SAW::Entities::UnitRate.new(id: '8368', price: 200.04, available: true, max_guests: 7), diff --git a/spec/lib/concierge/suppliers/saw/mappers/property_rate_spec.rb b/spec/lib/concierge/suppliers/saw/mappers/units_pricing_spec.rb similarity index 62% rename from spec/lib/concierge/suppliers/saw/mappers/property_rate_spec.rb rename to spec/lib/concierge/suppliers/saw/mappers/units_pricing_spec.rb index 898b25b32..72bf64a97 100644 --- a/spec/lib/concierge/suppliers/saw/mappers/property_rate_spec.rb +++ b/spec/lib/concierge/suppliers/saw/mappers/units_pricing_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' module SAW - RSpec.describe Mappers::PropertyRate do + RSpec.describe Mappers::UnitsPricing do let(:attributes) do { "name"=>"Adagio Access Avignon", @@ -61,15 +61,21 @@ module SAW end let(:hash) { Concierge::SafeAccessHash.new(attributes) } + let(:rates_array) { Array(hash) } + let(:stay_length) { 1 } it "builds available property rate object" do - property_rate = described_class.build(hash).value - expect(property_rate).to be_kind_of(SAW::Entities::PropertyRate) - expect(property_rate.id).to eq("2596") - expect(property_rate.currency).to eq("EUR") - expect(property_rate.units.size).to eq(1) + units_pricings = described_class.build(rates_array, stay_length) + expect(units_pricings).to be_kind_of(Array) + expect(units_pricings.size).to eq(1) + expect(units_pricings).to all(be_kind_of(SAW::Entities::UnitsPricing)) - unit = property_rate.units.first + units_pricing = units_pricings.first + expect(units_pricing.property_id).to eq("2596") + expect(units_pricing.currency).to eq("EUR") + expect(units_pricing.units.size).to eq(1) + + unit = units_pricing.units.first expect(unit.id).to eq("9863") expect(unit.price).to eq(104.00) @@ -89,11 +95,33 @@ module SAW } end - it "returns Result with error object" do - property_rate = described_class.build(hash) + it "returns an empty array" do + units_pricing = described_class.build(rates_array, stay_length) + + expect(units_pricing).to eq([]) + end + end + + describe "when rates are fetched for a few days" do + let(:stay_length) { 3 } + + it "divide total price to stay_length value" do + units_pricings = described_class.build(rates_array, stay_length) + expect(units_pricings).to be_kind_of(Array) + expect(units_pricings.size).to eq(1) + expect(units_pricings).to all(be_kind_of(SAW::Entities::UnitsPricing)) + + units_pricing = units_pricings.first + expect(units_pricing.property_id).to eq("2596") + expect(units_pricing.currency).to eq("EUR") + expect(units_pricing.units.size).to eq(1) + + unit = units_pricing.units.first - expect(property_rate).to be_a Result - expect(property_rate.error.code).to eq :empty_unit_rates + expect(unit.id).to eq("9863") + expect(unit.price).to eq(34.67) + expect(unit.available).to eq(true) + expect(unit.max_guests).to eq(4) end end end From 604a2a9d9cb092373ccb47fd4e8fdb248f0adba2 Mon Sep 17 00:00:00 2001 From: Sharipov Ruslan Date: Tue, 20 Sep 2016 13:54:45 +0600 Subject: [PATCH 13/38] SAW: worker update --- apps/workers/suppliers/saw.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/apps/workers/suppliers/saw.rb b/apps/workers/suppliers/saw.rb index b8fd8ed3c..e5a341c06 100644 --- a/apps/workers/suppliers/saw.rb +++ b/apps/workers/suppliers/saw.rb @@ -11,7 +11,7 @@ def initialize(host) end def perform - result = importer.fetch_countries + result = synchronisation.new_context { importer.fetch_countries } if result.success? countries = result.value @@ -21,7 +21,9 @@ def perform return end - result = importer.fetch_properties_by_countries(countries) + result = synchronisation.new_context do + importer.fetch_properties_by_countries(countries) + end if result.success? properties = result.value @@ -31,7 +33,9 @@ def perform return end - result = importer.fetch_all_unit_rates_for_properties(properties) + result = synchronisation.new_context do + importer.fetch_all_unit_rates_for_properties(properties) + end if result.success? all_unit_rates = result.value @@ -99,7 +103,7 @@ def announce_error(message, result) end def find_rates(property_id, all_unit_rates) - all_unit_rates.find { |rate| rate.id == property_id.to_s } + all_unit_rates.find { |rate| rate.property_id == property_id.to_s } end end end From e101471bcfaf6a8138d19177530ef892eb295c5e Mon Sep 17 00:00:00 2001 From: Sharipov Ruslan Date: Tue, 20 Sep 2016 13:55:13 +0600 Subject: [PATCH 14/38] SAW: remove not-needed methods --- .../suppliers/saw/entities/units_pricing.rb | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/lib/concierge/suppliers/saw/entities/units_pricing.rb b/lib/concierge/suppliers/saw/entities/units_pricing.rb index 477c98f7e..53baf9df5 100644 --- a/lib/concierge/suppliers/saw/entities/units_pricing.rb +++ b/lib/concierge/suppliers/saw/entities/units_pricing.rb @@ -19,27 +19,6 @@ def initialize(property_id:, units:, currency:) @units = units @currency = currency end - - # Find unit rate by +unit_id+ - # - # Arguments - # * +unit_id+ - # - # Returns [SAW::Entities::UnitRate] - def find_unit_rate(unit_id) - units.detect { |u| u.id == unit_id } - end - - # Determines whether unit with given +uni_id+ is present in units pricing - # object or not. - # - # Arguments - # * +unit_id+ - # - # Returns [Boolean] flag indicating the presence of rates for unit - def has_rates_for_unit?(unit_id) - !!find_unit(unit_id) - end end end end From cc81f1509d09866008bd55fda13e6dda10da5efd Mon Sep 17 00:00:00 2001 From: Sharipov Ruslan Date: Tue, 20 Sep 2016 13:55:52 +0600 Subject: [PATCH 15/38] SAW: update class documentation --- lib/concierge/suppliers/saw/mappers/roomorama_unit_set.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/concierge/suppliers/saw/mappers/roomorama_unit_set.rb b/lib/concierge/suppliers/saw/mappers/roomorama_unit_set.rb index aae09ca15..257b010a0 100644 --- a/lib/concierge/suppliers/saw/mappers/roomorama_unit_set.rb +++ b/lib/concierge/suppliers/saw/mappers/roomorama_unit_set.rb @@ -13,6 +13,7 @@ class RoomoramaUnitSet # # * +basic_property+ [SAW::Entities::BasicProperty] # * +detailed_property+ [SAW::Entities::DetailedProperty] + # * +rates+ [SAW::Entities::UnitsPricing] # # +detailed_property+ includes two attributes-hashes: # From 78336bb67cac520e2658dd6d3528e640338c8645 Mon Sep 17 00:00:00 2001 From: Sharipov Ruslan Date: Tue, 20 Sep 2016 14:17:08 +0600 Subject: [PATCH 16/38] SAW: nil quotation does not make any sense --- lib/concierge/suppliers/saw/commands/price_fetcher.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/concierge/suppliers/saw/commands/price_fetcher.rb b/lib/concierge/suppliers/saw/commands/price_fetcher.rb index ea343f330..e871cfefa 100644 --- a/lib/concierge/suppliers/saw/commands/price_fetcher.rb +++ b/lib/concierge/suppliers/saw/commands/price_fetcher.rb @@ -46,8 +46,6 @@ def call(params) if valid_result?(result_hash) quotation = build_quotation(params, result_hash) - return Result.error(:empty_unit_rates) unless quotation - Result.new(quotation) else error_result(result_hash) From 0323ab66cd4a82ea2cf15d4566a1794e5f786c8b Mon Sep 17 00:00:00 2001 From: Sharipov Ruslan Date: Wed, 21 Sep 2016 12:42:22 +0600 Subject: [PATCH 17/38] Update CHANGELOG.md for SAW changes (#370) --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f95bd1ac..0b0f905ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ how this file is formatted and how the process works. ### Fixed - Ciirus:: skip properties without images +- SAW: return unavailable quotation instead of Result.error while quote price +- SAW: pass stay length = 2 days instead 1 so that API returns prices for a bit more properties +- SAW: add synchronisation.new_context for metadata worker +- SAW: renamed PropertyRate entity to UnitsPricing ## [0.9.2] - 2016-09-19 ### Fixed From 106c60e4b58ef56d995c74b09a8a521df819c6e1 Mon Sep 17 00:00:00 2001 From: "K.Kolotyuk" Date: Wed, 21 Sep 2016 14:10:30 +0600 Subject: [PATCH 18/38] Ignore MC disabled clone properties --- CHANGELOG.md | 4 ++++ .../commands/property_permissions_fetcher.rb | 5 ++++- ...ed_clone_property_permissions_response.xml | 20 +++++++++++++++++++ .../property_permissions_fetcher_spec.rb | 11 ++++++++++ 4 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 spec/fixtures/ciirus/responses/mc_disabled_clone_property_permissions_response.xml diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ddefdf2d..f1c21b350 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ This file summarises the most important changes that went live on each release of Concierge. Please check the Wiki entry on the release process to understand how this file is formatted and how the process works. +## Unreleased +### Fixed +- Ciirus:: ignore permissions error about MC disabled clone properties + ## [0.10.0] - 2016-09-20 ### Added - Atleisure:: calendar sync worker diff --git a/lib/concierge/suppliers/ciirus/commands/property_permissions_fetcher.rb b/lib/concierge/suppliers/ciirus/commands/property_permissions_fetcher.rb index 455feef5b..401075679 100644 --- a/lib/concierge/suppliers/ciirus/commands/property_permissions_fetcher.rb +++ b/lib/concierge/suppliers/ciirus/commands/property_permissions_fetcher.rb @@ -22,7 +22,10 @@ class PropertyPermissionsFetcher < BaseCommand MC_PROPERTY_DISABLED_MESSAGE = 'Error (8000) The MC may have disabled the property from the feed. The MC user for '\ 'this property has not enabled this property for your feed, but you have accepted this property'\ ' in the SuperSites area of the CiiRUS windows application. ' - IGNORABLE_ERROR_MESSAGES = [PROPERTY_DELETED_MESSAGE, MC_PROPERTY_DISABLED_MESSAGE] + MC_DISABLED_CLONE_PROPERTY_MESSAGE = 'Error (15000) The MC may have disabled the clone property from the feed. The MC '\ + 'user for this property has not enabled this property for your feed, but you have accepted this property in '\ + 'the SuperSites area of the CiiRUS windows application. ' + IGNORABLE_ERROR_MESSAGES = [PROPERTY_DELETED_MESSAGE, MC_PROPERTY_DISABLED_MESSAGE, MC_DISABLED_CLONE_PROPERTY_MESSAGE] def call(property_id) diff --git a/spec/fixtures/ciirus/responses/mc_disabled_clone_property_permissions_response.xml b/spec/fixtures/ciirus/responses/mc_disabled_clone_property_permissions_response.xml new file mode 100644 index 000000000..ba6cb08a2 --- /dev/null +++ b/spec/fixtures/ciirus/responses/mc_disabled_clone_property_permissions_response.xml @@ -0,0 +1,20 @@ + + + + + + 63921 + false + false + 26888 + 33322 + false + true + false + Error (15000) The MC may have disabled the clone property from the feed. The MC user for this property has not enabled this property for your feed, but you have accepted this property in the SuperSites area of the CiiRUS windows application. + false + false + + + + \ No newline at end of file diff --git a/spec/lib/concierge/suppliers/ciirus/commands/property_permissions_fetcher_spec.rb b/spec/lib/concierge/suppliers/ciirus/commands/property_permissions_fetcher_spec.rb index bcf93686e..581478eb8 100644 --- a/spec/lib/concierge/suppliers/ciirus/commands/property_permissions_fetcher_spec.rb +++ b/spec/lib/concierge/suppliers/ciirus/commands/property_permissions_fetcher_spec.rb @@ -15,6 +15,7 @@ let(:success_response) { read_fixture('ciirus/responses/property_permissions_response.xml') } let(:deleted_property_response) { read_fixture('ciirus/responses/deleted_property_permissions_response.xml') } let(:mc_disabled_property_response) { read_fixture('ciirus/responses/mc_disabled_property_permissions_response.xml') } + let(:mc_disabled_clone_property_response) { read_fixture('ciirus/responses/mc_disabled_clone_property_permissions_response.xml') } let(:error_response) { read_fixture('ciirus/responses/error_property_permissions_response.xml') } let(:wsdl) { read_fixture('ciirus/wsdl.xml') } @@ -80,6 +81,16 @@ permissions = result.value expect(permissions.mc_enable_property).to be_falsey end + + it 'ignores mc disabled clone error message and returns permissions' do + stub_call(method: described_class::OPERATION_NAME, response: mc_disabled_clone_property_response) + + result = subject.call(property_id) + + expect(result).to be_success + permissions = result.value + expect(permissions.mc_enable_property).to be_falsey + end end context 'when xml contains error message' do From 900586acc72098b55098ff7ebbcf710d27dc0cf1 Mon Sep 17 00:00:00 2001 From: Keang Date: Wed, 21 Sep 2016 15:47:47 +0800 Subject: [PATCH 19/38] Skip property if postal code is a dot --- apps/workers/suppliers/saw.rb | 52 ++-- .../propertydetail/invalid_postal_code.xml | 267 ++++++++++++++++++ spec/workers/suppliers/saw_worker_spec.rb | 59 ++-- 3 files changed, 333 insertions(+), 45 deletions(-) create mode 100644 spec/fixtures/saw/propertydetail/invalid_postal_code.xml diff --git a/apps/workers/suppliers/saw.rb b/apps/workers/suppliers/saw.rb index e5a341c06..1990c559a 100644 --- a/apps/workers/suppliers/saw.rb +++ b/apps/workers/suppliers/saw.rb @@ -46,39 +46,47 @@ def perform end properties.each do |property| - synchronisation.start(property.internal_id) do - unit_rates = find_rates(property.internal_id, all_unit_rates) - fetch_details_and_build_property(property, unit_rates) + result = importer.fetch_detailed_property(property.internal_id) + if result.success? + detailed_property = result.value + + next if skip(detailed_property) + + synchronisation.start(property.internal_id) do + unit_rates = find_rates(property.internal_id, all_unit_rates) + Result.new ::SAW::Mappers::RoomoramaProperty.build( + property, + detailed_property, + unit_rates + ) + end + else + synchronisation.failed! + # potentially a more meaningful result can be passed from HTTPClient, into result.error.data + announce_error("Failed to perform the `#fetch_detailed_property` operation", result) end end synchronisation.finish! end - def fetch_details_and_build_property(property, unit_rates) - result = importer.fetch_detailed_property(property.internal_id) - - if result.success? - detailed_property = result.value - - roomorama_property = ::SAW::Mappers::RoomoramaProperty.build( - property, - detailed_property, - unit_rates - ) + private - Result.new(roomorama_property) - else - message = "Failed to perform the `#fetch_detailed_property` operation" - announce_error(message, result) - result + # Check if we can skip(and not publish) property, because of the following cases + # - Postal code is "." + # + # Returns true to caller if skipped + # + def skip(detailed_property) + if detailed_property.postal_code == "." + synchronisation.skip_property(detailed_property.internal_id, "Invalid postal_code: .") + return true end + return false end - private - def importer - @properties ||= ::SAW::Importer.new(credentials) + @importer ||= ::SAW::Importer.new(credentials) end def credentials diff --git a/spec/fixtures/saw/propertydetail/invalid_postal_code.xml b/spec/fixtures/saw/propertydetail/invalid_postal_code.xml new file mode 100644 index 000000000..2f835ab50 --- /dev/null +++ b/spec/fixtures/saw/propertydetail/invalid_postal_code.xml @@ -0,0 +1,267 @@ + + + + Outrigger Laguna Phuket Resort & Villas + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39050 + Thailand + Golf Course + Phuket + 3:00PM + 11:00AM + 111111111 + 2222222 + reservations@mansleyapartments.com + . + + N + A cancellation period of 25 days applies + 25 + + 10 + SUPERIOR PLUS + + + Villas provide private homes with kitchen, dining and living areas on lower floor and bedrooms on the upper floor. Pool villas feature lap pools and Thai sala pavilions. + +Suites provide contemporary single-level and dual-level suites adjacent to the main reception, swimming pool, Kids Club, and Panache restaurant. Suites do not include a washing machine in the apartment. + +Partake of all that Laguna Phuket provides, superb beaches, exquisite spas, exceptional dining, and simply great golf. + +If staying the night of 31st December there is a compulsory NYE Gala Dinner at THB 5,500.00 per adult and THB 2,750.00 per child up to the age of 12. + +Extra beds can be added to each apartment at an additional cost. + + + All + + + 1-Bedroom + + + 2 Bedroom - 1 Bathroom + + + 3 Bedroom - 2 Bathroom + + + 4 Bedroom - 3 Bathroom + + + + + 1 Bedroom Suite + + + Double Bed + + + + + 2 Bedroom Suite + + + Double & Twin + + + + + 2 Bedroom Villa + + + Double & Twin + + + + + 3 Bedroom Pool Suite + + + Double & Double & Twin + + + + + 3 Bedroom Pool Villa + + + Double & Double & Twin + + + + + 4 Bedroom Pool Villa + + + Double & Double & Double & Twin + + + + + + + 1-Bedroom + + 1 Bedroom Suite + + + + 2 Bedroom - 1 Bathroom + + 2 Bedroom Suite + + + 2 Bedroom Villa + + + + 3 Bedroom - 2 Bathroom + + 3 Bedroom Pool Suite + + + 3 Bedroom Pool Villa + + + + 4 Bedroom - 3 Bathroom + + 4 Bedroom Pool Villa + + + + + + 24 Hour Emergency Contact + 24 Hour Reception/Concierge + Air-Conditioning/Cooling + Alarm Clock + Answer Phones + Baby Amenities + Babysitting (On Request) + Bar + Bathroom Welcome Pack + BBQ + Breakfast Room + Burners / Hobs + Chauffeur Service (Extra Cost) + Childrens' Playground + Coffee Maker + Coffee Shop Nearby + Conference Facilities + Deposit Box + Digital TV + Direct Dial Telephone + Dry Cleaning (Extra Cost) + DVD Player + Fitness Centre On-Site + Freeview TV + Fridge / Freezer + Garden + Hairdryer + Ipod Docking Station + Iron and Ironing Board + Kettle + Laundry Facilities On-Site + Lift + Maid Service Daily + No Smoking Apartments/Rooms + Oven + Parking On-Site + Restaurant Nearby + Restaurant On-Site + Room Safe + Room Service + Shuttle Services + Sports Facilities + Supermarket On-Site + Swimming Pool Outdoors + Tea and Coffee + Toaster + Washing Machine / Dryer + WIFI Free of Charge + WIFI Reception Area Free of Charge + Work Place + + + Laguna Phuket Golf Club + + + + 1 Bedroom Suite + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=38990 + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=38992 + + + 1 Bedroom Suite + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=38995 + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=38997 + + + 1 Bedroom Suite + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39000 + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39002 + + + 1 Bedroom Suite + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39005 + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39007 + + + 2 Bedroom Suite + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39010 + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39012 + + + 2 Bedroom Suite + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39015 + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39017 + + + 2 Bedroom Suite + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39020 + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39022 + + + 3 Bedroom Pool Suite + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39025 + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39027 + + + 3 Bedroom Pool Suite + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39030 + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39032 + + + 3 Bedroom Pool Suite + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39035 + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39037 + + + 3 Bedroom Pool Suite + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39045 + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39047 + + + 3 Bedroom Pool Suite + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39050 + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39052 + + + Outrigger Laguna Phuket Resort & Villas + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39055 + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39057 + + + Outrigger Laguna Phuket Resort & Villas + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39060 + http://staging.servicedapartmentsworldwide.net/ImageHandler.jpg?ImageInstanceId=39062 + + + + 8.00382 + 98.30736 + Address1 Address2 Phuket Thailand + + + + diff --git a/spec/workers/suppliers/saw_worker_spec.rb b/spec/workers/suppliers/saw_worker_spec.rb index 7fdfe37a7..098bac498 100644 --- a/spec/workers/suppliers/saw_worker_spec.rb +++ b/spec/workers/suppliers/saw_worker_spec.rb @@ -63,34 +63,47 @@ expect(result.to_h[:successful]).to be true end - it "fails when there is an error while fetching property details" do - mock_request(:propertydetail, :error) + describe "handling property detail" do + before do + mock_request(:country, :one) + mock_request(:propertysearch, :success) + mock_request(:propertyrates, :success) + end + it "fails when there is an error while fetching property details" do + mock_request(:propertydetail, :error) + + result = worker.perform + + expect(result).to be_a(SyncProcess) + expect(result.successful).to eq false + expect(last_context_event[:label]).to eq( + "Synchronisation Failure" + ) + expect(last_context_event[:message]).to eq( + "Failed to perform the `#fetch_detailed_property` operation" + ) + expect(last_context_event[:backtrace]).to be_kind_of(Array) + expect(last_context_event[:backtrace].any?).to be true + end - property = SAW::Entities::BasicProperty.new - result = worker.fetch_details_and_build_property(property, nil) + it "returns built property" do + mock_request(:propertydetail, :success) - expect(result).to be_kind_of(Result) - expect(result).not_to be_success - expect(result.error.code).to eq("0000") - expect(last_context_event[:label]).to eq( - "Synchronisation Failure" - ) - expect(last_context_event[:message]).to eq( - "Failed to perform the `#fetch_detailed_property` operation" - ) - expect(last_context_event[:backtrace]).to be_kind_of(Array) - expect(last_context_event[:backtrace].any?).to be true - end + expect_any_instance_of(Workers::PropertySynchronisation).to receive(:process).exactly(5).times + result = worker.perform - it "returns built property" do - mock_request(:propertydetail, :success) + expect(result).to be_a(SyncProcess) + expect(result.successful).to eq true + end - property = SAW::Entities::BasicProperty.new - result = worker.fetch_details_and_build_property(property, nil) + it "returns a built property with invalid postal_code" do + mock_request(:propertydetail, :invalid_postal_code) + expect_any_instance_of(Workers::PropertySynchronisation).to receive(:skip_property).exactly(5).times + result = worker.perform - expect(result).to be_kind_of(Result) - expect(result).to be_success - expect(result.value).to be_kind_of(Roomorama::Property) + expect(result).to be_a(SyncProcess) + expect(result.successful).to eq true + end end end end From f523081d7cc79a6e8a255eda33e745f7e60720f2 Mon Sep 17 00:00:00 2001 From: Keang Date: Wed, 21 Sep 2016 15:50:22 +0800 Subject: [PATCH 20/38] Update changelog --- CHANGELOG.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1c21b350..c80498028 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ how this file is formatted and how the process works. ## Unreleased ### Fixed - Ciirus:: ignore permissions error about MC disabled clone properties +- SAW: return unavailable quotation instead of Result.error while quote price +- SAW: pass stay length = 2 days instead 1 so that API returns prices for a bit more properties +- SAW: add synchronisation.new_context for metadata worker +- SAW: renamed PropertyRate entity to UnitsPricing +- SAW: Skip invalid postal code "." #286 ## [0.10.0] - 2016-09-20 ### Added @@ -16,10 +21,6 @@ how this file is formatted and how the process works. ### Fixed - Ciirus:: skip properties without images -- SAW: return unavailable quotation instead of Result.error while quote price -- SAW: pass stay length = 2 days instead 1 so that API returns prices for a bit more properties -- SAW: add synchronisation.new_context for metadata worker -- SAW: renamed PropertyRate entity to UnitsPricing ## [0.9.2] - 2016-09-19 ### Fixed From c427102875a1aaf3dea8c94d455b6aec4cbefb40 Mon Sep 17 00:00:00 2001 From: Keang Date: Wed, 21 Sep 2016 15:54:52 +0800 Subject: [PATCH 21/38] Fix specs --- spec/workers/suppliers/saw_worker_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/workers/suppliers/saw_worker_spec.rb b/spec/workers/suppliers/saw_worker_spec.rb index 098bac498..18a73e22f 100644 --- a/spec/workers/suppliers/saw_worker_spec.rb +++ b/spec/workers/suppliers/saw_worker_spec.rb @@ -51,6 +51,7 @@ mock_request(:country, :one) mock_request(:propertysearch, :success) mock_request(:propertyrates, :success) + mock_request(:propertydetail, :success) expected_property_ids = [1787, 1757, 2721, 2893, 1766] From 1fa40cbaa000ad543e2d6b7e78a31049d92726ae Mon Sep 17 00:00:00 2001 From: Keang Date: Wed, 21 Sep 2016 16:10:03 +0800 Subject: [PATCH 22/38] Rename --- apps/workers/suppliers/saw.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/workers/suppliers/saw.rb b/apps/workers/suppliers/saw.rb index 1990c559a..cde444f16 100644 --- a/apps/workers/suppliers/saw.rb +++ b/apps/workers/suppliers/saw.rb @@ -50,7 +50,7 @@ def perform if result.success? detailed_property = result.value - next if skip(detailed_property) + next if skip?(detailed_property) synchronisation.start(property.internal_id) do unit_rates = find_rates(property.internal_id, all_unit_rates) @@ -77,7 +77,7 @@ def perform # # Returns true to caller if skipped # - def skip(detailed_property) + def skip?(detailed_property) if detailed_property.postal_code == "." synchronisation.skip_property(detailed_property.internal_id, "Invalid postal_code: .") return true From 2faa0d582694e8142c897655a2cef51bbafce31d Mon Sep 17 00:00:00 2001 From: Keang Date: Wed, 21 Sep 2016 16:12:45 +0800 Subject: [PATCH 23/38] Wrap new context --- apps/workers/suppliers/saw.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/workers/suppliers/saw.rb b/apps/workers/suppliers/saw.rb index cde444f16..52b8e8468 100644 --- a/apps/workers/suppliers/saw.rb +++ b/apps/workers/suppliers/saw.rb @@ -46,7 +46,9 @@ def perform end properties.each do |property| - result = importer.fetch_detailed_property(property.internal_id) + result = synchronisation.new_context do + importer.fetch_detailed_property(property.internal_id) + end if result.success? detailed_property = result.value From 71d6b62361b2dfca5b887b7f236efc52fd23f110 Mon Sep 17 00:00:00 2001 From: Renato Mascarenhas Date: Wed, 21 Sep 2016 11:19:42 +0800 Subject: [PATCH 24/38] Include ZendeskNotify client. --- apps/api/application.rb | 3 +- apps/api/support/zendesk_notify.rb | 86 +++++++++++++++++++++++++ spec/api/support/zendesk_notify_spec.rb | 84 ++++++++++++++++++++++++ 3 files changed, 172 insertions(+), 1 deletion(-) create mode 100644 apps/api/support/zendesk_notify.rb create mode 100644 spec/api/support/zendesk_notify_spec.rb diff --git a/apps/api/application.rb b/apps/api/application.rb index bb6750dc6..309e5ec1e 100644 --- a/apps/api/application.rb +++ b/apps/api/application.rb @@ -13,7 +13,8 @@ class Application < Hanami::Application load_paths << [ 'controllers', - 'views' + 'views', + 'support' ] routes 'config/routes' diff --git a/apps/api/support/zendesk_notify.rb b/apps/api/support/zendesk_notify.rb new file mode 100644 index 000000000..0281f4005 --- /dev/null +++ b/apps/api/support/zendesk_notify.rb @@ -0,0 +1,86 @@ +module API::Support + + # +API::Support::ZendeskNotify+ + # + # This is a client class to the +ZendeskNotify+ service, a simple API to + # send tickets on Roomorama/BridgeRentals Zendesk account. + # + # Usage + # + # client = API::Support::ZendeskNotify.new + # ticket_id = "cancellation" + # attributes = { + # supplier: "Supplier X", + # supplier_id: "212", + # bridge_id: "291" + # } + # + # client.notify # => <#Result value=true> + # + # The +attributes+ passed when calling the +notify+ method are dependent on the + # type of ticket being sent. + class ZendeskNotify + include Concierge::JSON + + SUPPORTED_TICKETS = %w(cancellation) + + attr_reader :http, :endpoint + + # initializes internal state. The +ZendeskNotify+ service URL must be properly + # configured on the +ZENDESK_NOTIFY_URL+ environment variable. + def initialize + uri = URI.parse(zendesk_notify_url) + host = [uri.scheme, "://", uri.host].join + + @http = Concierge::HTTPClient.new(host) + @endpoint = uri.request_uri + end + + def notify(ticket_id, attributes) + return invalid_ticket unless SUPPORTED_TICKETS.include?(ticket_id.to_s) + + params = { + ticketId: ticket_id, + attributes: attributes + } + + result = http.post(endpoint, params) + return result unless result.success? + + parse_response(result.value.body) + end + + private + + # expected response (examples) + # + # Success + # { "status": "ok", "message": "Ticket sent successfully" } + # + # Failure + # { "status": "error", "message": "Failure to deliver ticket" } + def parse_response(body) + decoded_body = json_decode(body) + return decoded_body unless decoded_body.success? + + successful = (decoded_body.value["status"] == "ok") + + if successful + Result.new(true) + else + error_message = decoded_body.value["message"] + Result.error(:zendesk_notify_failure, error_message) + end + end + + def invalid_ticket + Result.error(:zendesk_invalid_ticket) + end + + def zendesk_notify_url + ENV["ZENDESK_NOTIFY_URL"] + end + + end + +end diff --git a/spec/api/support/zendesk_notify_spec.rb b/spec/api/support/zendesk_notify_spec.rb new file mode 100644 index 000000000..7ff427a12 --- /dev/null +++ b/spec/api/support/zendesk_notify_spec.rb @@ -0,0 +1,84 @@ +require "spec_helper" + +RSpec.describe API::Support::ZendeskNotify do + include Support::HTTPStubbing + + let(:zendesk_notify_url) { "https://www.zendesk-notify-test.com/v1/production" } + let(:ticket_id) { "cancellation" } + let(:attributes) { + { + supplier: "Supplier Y", + supplier_id: "123", + bridge_id: "321" + } + } + + before do + ENV["ZENDESK_NOTIFY_URL"] = zendesk_notify_url + end + + after do + ENV.delete("ZENDESK_NOTIFY_URL") + end + + describe "#notify" do + it "forwards an error at the HTTP level if it occurs" do + stub_call(:post, zendesk_notify_url) { [500, {}, ""] } + + result = subject.notify(ticket_id, attributes) + expect(result).to be_a Result + expect(result).not_to be_success + expect(result.error.code).to eq :http_status_500 + end + + it "is unsuccessful when the requested ticket ID is not valid" do + ticket_id = "invalid_ticket_id" + + result = subject.notify(ticket_id, attributes) + expect(result).to be_a Result + expect(result).not_to be_success + expect(result.error.code).to eq :zendesk_invalid_ticket + end + + it "returns an unsuccessful result when the response contains garbage" do + stub_call(:post, zendesk_notify_url) { [200, {}, "*#&"] } + + result = subject.notify(ticket_id, attributes) + expect(result).to be_a Result + expect(result).not_to be_success + expect(result.error.code).to eq :invalid_json_representation + end + + it "returns an unsuccessful result in case the ticket cannot be sent" do + stub_call(:post, zendesk_notify_url) { + body = { + status: "error", + message: "Failure to connect to the Zendesk API" + } + + [200, {}, body.to_json] + } + + result = subject.notify(ticket_id, attributes) + expect(result).to be_a Result + expect(result).not_to be_success + expect(result.error.code).to eq :zendesk_notify_failure + expect(result.error.data).to eq "Failure to connect to the Zendesk API" + end + + it "returns a successful result in case the ticket is properly sent" do + stub_call(:post, zendesk_notify_url) { + body = { + status: "ok", + message: "Ticket sent successfully" + } + + [200, {}, body.to_json] + } + + result = subject.notify(ticket_id, attributes) + expect(result).to be_a Result + expect(result).to be_success + end + end +end From b1f606655c23c8b6910b774f0588b858476e53d3 Mon Sep 17 00:00:00 2001 From: Renato Mascarenhas Date: Wed, 21 Sep 2016 11:20:19 +0800 Subject: [PATCH 25/38] Require ZENDESK_NOTIFY_URL on the api app. --- apps/api/config/environment_variables.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/api/config/environment_variables.yml b/apps/api/config/environment_variables.yml index bba12cb4b..a5dc050f4 100644 --- a/apps/api/config/environment_variables.yml +++ b/apps/api/config/environment_variables.yml @@ -6,3 +6,4 @@ - ROOMORAMA_SECRET_WAYTOSTAY - ROOMORAMA_SECRET_CIIRUS - ROOMORAMA_SECRET_SAW +- ZENDESK_NOTIFY_URL From bd0a428959048cc7f9cda6d1d1cd8f1d04ba0430 Mon Sep 17 00:00:00 2001 From: Renato Mascarenhas Date: Wed, 21 Sep 2016 11:42:09 +0800 Subject: [PATCH 26/38] Pass BridgeRentals inquiry ID to the cancellation endpoint. --- apps/api/controllers/params/cancel.rb | 1 + apps/api/middlewares/roomorama_webhook.rb | 5 ++++- spec/api/controllers/ciirus/cancel_spec.rb | 10 +++++----- spec/api/controllers/kigo/cancel_spec.rb | 10 +++++----- spec/api/controllers/kigo/legacy/cancel_spec.rb | 10 +++++----- spec/api/controllers/saw/cancel_spec.rb | 8 ++++---- spec/api/controllers/waytostay/cancel_spec.rb | 10 +++++----- spec/api/middlewares/roomorama_webhook_spec.rb | 1 + 8 files changed, 30 insertions(+), 25 deletions(-) diff --git a/apps/api/controllers/params/cancel.rb b/apps/api/controllers/params/cancel.rb index b410c564b..e1d03ca49 100644 --- a/apps/api/controllers/params/cancel.rb +++ b/apps/api/controllers/params/cancel.rb @@ -13,6 +13,7 @@ module API::Controllers::Params class Cancel < API::Action::Params param :reference_number, presence: true, type: String + param :inquiry_id, presence: true, type: String # Constructs a map of errors for the request. # diff --git a/apps/api/middlewares/roomorama_webhook.rb b/apps/api/middlewares/roomorama_webhook.rb index 78c374b69..72412fd16 100644 --- a/apps/api/middlewares/roomorama_webhook.rb +++ b/apps/api/middlewares/roomorama_webhook.rb @@ -108,7 +108,10 @@ def booking(data) def cancelled(data) payload = safe_access(data) - params = { reference_number: payload.get("inquiry.reference_number") } + params = { + reference_number: payload.get("inquiry.reference_number"), + inquiry_id: payload.get("inquiry.id") + } env["rack.input"] = StringIO.new(json_encode(params)) true diff --git a/spec/api/controllers/ciirus/cancel_spec.rb b/spec/api/controllers/ciirus/cancel_spec.rb index fe5b95786..906a95549 100644 --- a/spec/api/controllers/ciirus/cancel_spec.rb +++ b/spec/api/controllers/ciirus/cancel_spec.rb @@ -6,15 +6,15 @@ it_behaves_like 'cancel action' do let(:success_cases) do [ - { params: {reference_number: '5486789'}, cancelled_reference_number: '5486789' }, - { params: {reference_number: '2154254'}, cancelled_reference_number: '2154254' }, + { params: {reference_number: '5486789', inquiry_id: '125'}, cancelled_reference_number: '5486789' }, + { params: {reference_number: '2154254', inquiry_id: '128'}, cancelled_reference_number: '2154254' }, ] end let(:error_cases) do [ - { params: {reference_number: '658794'}, error: {'cancellation' => 'Could not cancel with remote supplier'} }, - { params: {reference_number: '245784'}, error: {'cancellation' => 'Already cancelled'} } + { params: {reference_number: '658794', inquiry_id: '392'}, error: {'cancellation' => 'Could not cancel with remote supplier'} }, + { params: {reference_number: '245784', inquiry_id: '399'}, error: {'cancellation' => 'Already cancelled'} } ] end end @@ -37,4 +37,4 @@ result end end -end \ No newline at end of file +end diff --git a/spec/api/controllers/kigo/cancel_spec.rb b/spec/api/controllers/kigo/cancel_spec.rb index 83bbf730d..678462516 100644 --- a/spec/api/controllers/kigo/cancel_spec.rb +++ b/spec/api/controllers/kigo/cancel_spec.rb @@ -2,19 +2,19 @@ require_relative "../shared/cancel" RSpec.describe API::Controllers::Kigo::Cancel do - let(:params) { { reference_number: "A123" } } + let(:params) { { reference_number: "A123", inquiry_id: "123" } } it_behaves_like "cancel action" do let(:success_cases) { [ - { params: {reference_number: "A023"}, cancelled_reference_number: "XYZ" }, - { params: {reference_number: "A024"}, cancelled_reference_number: "ASD" }, + { params: {reference_number: "A023", inquiry_id: "123"}, cancelled_reference_number: "XYZ" }, + { params: {reference_number: "A024", inquiry_id: "125"}, cancelled_reference_number: "ASD" }, ] } let(:error_cases) { [ - { params: {reference_number: "A123"}, error: {"cancellation" => "Could not cancel with remote supplier"} }, - { params: {reference_number: "A124"}, error: {"cancellation" => "Already cancelled"} }, + { params: {reference_number: "A123", inquiry_id: "123"}, error: {"cancellation" => "Could not cancel with remote supplier"} }, + { params: {reference_number: "A124", inquiry_id: "124"}, error: {"cancellation" => "Already cancelled"} }, ] } diff --git a/spec/api/controllers/kigo/legacy/cancel_spec.rb b/spec/api/controllers/kigo/legacy/cancel_spec.rb index 28e770d74..29f656979 100644 --- a/spec/api/controllers/kigo/legacy/cancel_spec.rb +++ b/spec/api/controllers/kigo/legacy/cancel_spec.rb @@ -2,19 +2,19 @@ require_relative "../../shared/cancel" RSpec.describe API::Controllers::Kigo::Legacy::Cancel do - let(:params) { { reference_number: "A123" } } + let(:params) { { reference_number: "A123", inquiry_id: "123" } } it_behaves_like "cancel action" do let(:success_cases) { [ - { params: {reference_number: "A023"}, cancelled_reference_number: "XYZ" }, - { params: {reference_number: "A024"}, cancelled_reference_number: "ASD" }, + { params: {reference_number: "A023", inquiry_id: "123"}, cancelled_reference_number: "XYZ" }, + { params: {reference_number: "A024", inquiry_id: "125"}, cancelled_reference_number: "ASD" }, ] } let(:error_cases) { [ - { params: {reference_number: "A123"}, error: {"cancellation" => "Could not cancel with remote supplier"} }, - { params: {reference_number: "A124"}, error: {"cancellation" => "Already cancelled"} }, + { params: {reference_number: "A123", inquiry_id: "124"}, error: {"cancellation" => "Could not cancel with remote supplier"} }, + { params: {reference_number: "A124", inquiry_id: "128"}, error: {"cancellation" => "Already cancelled"} }, ] } diff --git a/spec/api/controllers/saw/cancel_spec.rb b/spec/api/controllers/saw/cancel_spec.rb index 823a7ae8f..c9822193b 100644 --- a/spec/api/controllers/saw/cancel_spec.rb +++ b/spec/api/controllers/saw/cancel_spec.rb @@ -5,14 +5,14 @@ it_behaves_like "cancel action" do let(:success_cases) { [ - { params: {reference_number: "A023"}, cancelled_reference_number: "XYZ" }, - { params: {reference_number: "A024"}, cancelled_reference_number: "ASD" }, + { params: {reference_number: "A023", inquiry_id: "303"}, cancelled_reference_number: "XYZ" }, + { params: {reference_number: "A024", inquiry_id: "308"}, cancelled_reference_number: "ASD" }, ] } let(:error_cases) { [ - { params: {reference_number: "A123"}, error: {"cancellation" => "Could not cancel with remote supplier"} }, - { params: {reference_number: "A124"}, error: {"cancellation" => "Already cancelled"} }, + { params: {reference_number: "A123", inquiry_id: "392"}, error: {"cancellation" => "Could not cancel with remote supplier"} }, + { params: {reference_number: "A124", inquiry_id: "399"}, error: {"cancellation" => "Already cancelled"} }, ] } diff --git a/spec/api/controllers/waytostay/cancel_spec.rb b/spec/api/controllers/waytostay/cancel_spec.rb index 8f1b52bd4..1c83c093e 100644 --- a/spec/api/controllers/waytostay/cancel_spec.rb +++ b/spec/api/controllers/waytostay/cancel_spec.rb @@ -2,19 +2,19 @@ require_relative "../shared/cancel" RSpec.describe API::Controllers::Waytostay::Cancel do - let(:params) { { reference_number: "A123" } } + let(:params) { { reference_number: "A123", inquiry_id: "123" } } it_behaves_like "cancel action" do let(:success_cases) { [ - { params: {reference_number: "A023"}, cancelled_reference_number: "XYZ" }, - { params: {reference_number: "A024"}, cancelled_reference_number: "ASD" }, + { params: {reference_number: "A023", inquiry_id: "392"}, cancelled_reference_number: "XYZ" }, + { params: {reference_number: "A024", inquiry_id: "398"}, cancelled_reference_number: "ASD" }, ] } let(:error_cases) { [ - { params: {reference_number: "A123"}, error: {"cancellation" => "Could not cancel with remote supplier"} }, - { params: {reference_number: "A124"}, error: {"cancellation" => "Already cancelled"} }, + { params: {reference_number: "A123", inquiry_id: "320"}, error: {"cancellation" => "Could not cancel with remote supplier"} }, + { params: {reference_number: "A124", inquiry_id: "328"}, error: {"cancellation" => "Already cancelled"} }, ] } diff --git a/spec/api/middlewares/roomorama_webhook_spec.rb b/spec/api/middlewares/roomorama_webhook_spec.rb index 9355d78c4..6e7592a5b 100644 --- a/spec/api/middlewares/roomorama_webhook_spec.rb +++ b/spec/api/middlewares/roomorama_webhook_spec.rb @@ -121,6 +121,7 @@ roomorama_webhook["action"] = "cancelled" roomorama_webhook["event"] = "cancelled" roomorama_webhook["inquiry"]["reference_number"] = "test_code" + roomorama_webhook["inquiry"]["id"] = "3929" end it "returns 200 if upstream was success" do From ab17c362fbba314a066bf7298a69ccc788c4b7ce Mon Sep 17 00:00:00 2001 From: Renato Mascarenhas Date: Wed, 21 Sep 2016 13:33:51 +0800 Subject: [PATCH 27/38] Support Zendesk cancellation notifications for Poplidays. --- apps/api/config/routes.rb | 13 ++++----- apps/api/controllers/poplidays/cancel.rb | 17 ++++++++++++ .../zendesk_notify_cancellation.rb | 26 ++++++++++++++++++ spec/api/controllers/poplidays/cancel_spec.rb | 27 +++++++++++++++++++ 4 files changed, 77 insertions(+), 6 deletions(-) create mode 100644 apps/api/controllers/poplidays/cancel.rb create mode 100644 apps/api/controllers/zendesk_notify_cancellation.rb create mode 100644 spec/api/controllers/poplidays/cancel_spec.rb diff --git a/apps/api/config/routes.rb b/apps/api/config/routes.rb index f9f32dd55..c87d4d964 100644 --- a/apps/api/config/routes.rb +++ b/apps/api/config/routes.rb @@ -16,11 +16,12 @@ post '/saw/booking', to: 's_a_w#booking' post '/poplidays/booking', to: 'poplidays#booking' -post 'waytostay/cancel', to: 'waytostay#cancel' -post 'ciirus/cancel', to: 'ciirus#cancel' -post 'saw/cancel', to: 's_a_w#cancel' -post 'kigo/cancel', to: 'kigo#cancel' -post 'kigo/legacy/cancel', to: 'kigo/legacy#cancel' +post '/waytostay/cancel', to: 'waytostay#cancel' +post '/ciirus/cancel', to: 'ciirus#cancel' +post '/saw/cancel', to: 's_a_w#cancel' +post '/kigo/cancel', to: 'kigo#cancel' +post '/kigo/legacy/cancel', to: 'kigo/legacy#cancel' +post '/poplidays/cancel', to: 'poplidays#cancel' -post 'checkout', to: 'static#checkout' +post '/checkout', to: 'static#checkout' get '/kigo/image/:property_id/:image_id', to: 'kigo#image' diff --git a/apps/api/controllers/poplidays/cancel.rb b/apps/api/controllers/poplidays/cancel.rb new file mode 100644 index 000000000..849ce2d20 --- /dev/null +++ b/apps/api/controllers/poplidays/cancel.rb @@ -0,0 +1,17 @@ +require_relative "../cancel" + +module API::Controllers::Poplidays + + # API::Controllers::Poplidays::Cancel + # + # Poplidays does not have a cancellation API. Therefore, when a booking is + # cancelled, we notify Customer Support through Zendesk. + class Cancel + include API::Controllers::Cancel + include API::Controllers::ZendeskNotifyCancellation + + def supplier_name + Poplidays::Client::SUPPLIER_NAME + end + end +end diff --git a/apps/api/controllers/zendesk_notify_cancellation.rb b/apps/api/controllers/zendesk_notify_cancellation.rb new file mode 100644 index 000000000..54860c6cc --- /dev/null +++ b/apps/api/controllers/zendesk_notify_cancellation.rb @@ -0,0 +1,26 @@ +module API::Controllers + + # +API::Controllers::ZendeskNotifyCancellation+ + # + # This module implements the +cancel_reservation+ method expected to be present + # on cancellation controllers that include the helper +API::Controllers::Cancel+ + # module. + # + # The +cancel_reservation+ expects a +supplier_name+ method to be implemented + # (a requirement already enforced by +API::Controllers::Cancel+), and sends a request + # to the +ZendeskNotify+ service to notify Customer Support about a supplier cancellation. + # + # To be used by suppliers which do not provide a cancellation API. + module ZendeskNotifyCancellation + + def cancel_reservation(params) + API::Support::ZendeskNotify.new.notify("cancellation", { + supplier: supplier_name, + supplier_id: params[:reference_number], + bridge_id: params[:inquiry_id] + }) + end + + end + +end diff --git a/spec/api/controllers/poplidays/cancel_spec.rb b/spec/api/controllers/poplidays/cancel_spec.rb new file mode 100644 index 000000000..101febcce --- /dev/null +++ b/spec/api/controllers/poplidays/cancel_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' +require_relative "../shared/cancel" + +RSpec.describe API::Controllers::Poplidays::Cancel do + include Support::HTTPStubbing + + let(:params) { { reference_number: "123", inquiry_id: "392" } } + let(:zendesk_notify_url) { "https://www.zendesk-notify-example.org" } + + before do + ENV["ZENDESK_NOTIFY_URL"] = zendesk_notify_url + end + + after do + ENV.delete("ZENDESK_NOTIFY_URL") + end + + it "sends a ticket to Zendesk upon cancellation" do + stub_call(:post, zendesk_notify_url) { [200, {}, { "status" => "ok" }.to_json] } + + expect_any_instance_of(API::Support::ZendeskNotify).to receive(:notify). + with("cancellation", { supplier: "Poplidays", supplier_id: "123", bridge_id: "392" }). + once.and_call_original + + subject.call(params) + end +end From d4078094c8aa7eaa955cfa2ed2f6288edf62ae34 Mon Sep 17 00:00:00 2001 From: Renato Mascarenhas Date: Wed, 21 Sep 2016 13:34:35 +0800 Subject: [PATCH 28/38] Include ZENDESK_NOTIFY_URL on .env.example. --- .env.example | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.env.example b/.env.example index 6aba231f1..e5b05d155 100644 --- a/.env.example +++ b/.env.example @@ -20,3 +20,5 @@ CONCIERGE_WEB_AUTHENTICATION=admin:admin CONCIERGE_APP=all CONCIERGE_API_SECRET="f52bfb549b54f119272cb21" CONCIERGE_URL=http://localhost:2300 + +ZENDESK_NOTIFY_URL="https://example.org" From 9d5ec842971286589087b72d806fcdbc8086a289 Mon Sep 17 00:00:00 2001 From: Renato Mascarenhas Date: Wed, 21 Sep 2016 13:37:54 +0800 Subject: [PATCH 29/38] Support for Zendesk cancellation notification for AtLeisure. --- apps/api/config/routes.rb | 1 + apps/api/controllers/atleisure/cancel.rb | 17 ++++++++++++ spec/api/controllers/atleisure/cancel_spec.rb | 27 +++++++++++++++++++ 3 files changed, 45 insertions(+) create mode 100644 apps/api/controllers/atleisure/cancel.rb create mode 100644 spec/api/controllers/atleisure/cancel_spec.rb diff --git a/apps/api/config/routes.rb b/apps/api/config/routes.rb index c87d4d964..c97dd37a7 100644 --- a/apps/api/config/routes.rb +++ b/apps/api/config/routes.rb @@ -22,6 +22,7 @@ post '/kigo/cancel', to: 'kigo#cancel' post '/kigo/legacy/cancel', to: 'kigo/legacy#cancel' post '/poplidays/cancel', to: 'poplidays#cancel' +post '/atleisure/cancel', to: 'atleisure#cancel' post '/checkout', to: 'static#checkout' get '/kigo/image/:property_id/:image_id', to: 'kigo#image' diff --git a/apps/api/controllers/atleisure/cancel.rb b/apps/api/controllers/atleisure/cancel.rb new file mode 100644 index 000000000..028608400 --- /dev/null +++ b/apps/api/controllers/atleisure/cancel.rb @@ -0,0 +1,17 @@ +require_relative "../cancel" + +module API::Controllers::AtLeisure + + # API::Controllers::AtLeisure::Cancel + # + # AtLeisure does not have a cancellation API. Therefore, when a booking is + # cancelled, we notify Customer Support through Zendesk. + class Cancel + include API::Controllers::Cancel + include API::Controllers::ZendeskNotifyCancellation + + def supplier_name + AtLeisure::Client::SUPPLIER_NAME + end + end +end diff --git a/spec/api/controllers/atleisure/cancel_spec.rb b/spec/api/controllers/atleisure/cancel_spec.rb new file mode 100644 index 000000000..7ca7ac87b --- /dev/null +++ b/spec/api/controllers/atleisure/cancel_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' +require_relative "../shared/cancel" + +RSpec.describe API::Controllers::AtLeisure::Cancel do + include Support::HTTPStubbing + + let(:params) { { reference_number: "123", inquiry_id: "392" } } + let(:zendesk_notify_url) { "https://www.zendesk-notify-example.org" } + + before do + ENV["ZENDESK_NOTIFY_URL"] = zendesk_notify_url + end + + after do + ENV.delete("ZENDESK_NOTIFY_URL") + end + + it "sends a ticket to Zendesk upon cancellation" do + stub_call(:post, zendesk_notify_url) { [200, {}, { "status" => "ok" }.to_json] } + + expect_any_instance_of(API::Support::ZendeskNotify).to receive(:notify). + with("cancellation", { supplier: "AtLeisure", supplier_id: "123", bridge_id: "392" }). + once.and_call_original + + subject.call(params) + end +end From 3087c82350497a62a77e2de351d6efe7927c1b33 Mon Sep 17 00:00:00 2001 From: Renato Mascarenhas Date: Wed, 21 Sep 2016 13:42:17 +0800 Subject: [PATCH 30/38] Move cancellation specs to shared examples. --- spec/api/controllers/atleisure/cancel_spec.rb | 23 +---------------- spec/api/controllers/poplidays/cancel_spec.rb | 23 +---------------- spec/api/controllers/shared/cancel.rb | 25 +++++++++++++++++++ 3 files changed, 27 insertions(+), 44 deletions(-) diff --git a/spec/api/controllers/atleisure/cancel_spec.rb b/spec/api/controllers/atleisure/cancel_spec.rb index 7ca7ac87b..fcfb06d44 100644 --- a/spec/api/controllers/atleisure/cancel_spec.rb +++ b/spec/api/controllers/atleisure/cancel_spec.rb @@ -2,26 +2,5 @@ require_relative "../shared/cancel" RSpec.describe API::Controllers::AtLeisure::Cancel do - include Support::HTTPStubbing - - let(:params) { { reference_number: "123", inquiry_id: "392" } } - let(:zendesk_notify_url) { "https://www.zendesk-notify-example.org" } - - before do - ENV["ZENDESK_NOTIFY_URL"] = zendesk_notify_url - end - - after do - ENV.delete("ZENDESK_NOTIFY_URL") - end - - it "sends a ticket to Zendesk upon cancellation" do - stub_call(:post, zendesk_notify_url) { [200, {}, { "status" => "ok" }.to_json] } - - expect_any_instance_of(API::Support::ZendeskNotify).to receive(:notify). - with("cancellation", { supplier: "AtLeisure", supplier_id: "123", bridge_id: "392" }). - once.and_call_original - - subject.call(params) - end + it_behaves_like "Zendesk cancellation notification", supplier: "AtLeisure" end diff --git a/spec/api/controllers/poplidays/cancel_spec.rb b/spec/api/controllers/poplidays/cancel_spec.rb index 101febcce..ab61a2c84 100644 --- a/spec/api/controllers/poplidays/cancel_spec.rb +++ b/spec/api/controllers/poplidays/cancel_spec.rb @@ -2,26 +2,5 @@ require_relative "../shared/cancel" RSpec.describe API::Controllers::Poplidays::Cancel do - include Support::HTTPStubbing - - let(:params) { { reference_number: "123", inquiry_id: "392" } } - let(:zendesk_notify_url) { "https://www.zendesk-notify-example.org" } - - before do - ENV["ZENDESK_NOTIFY_URL"] = zendesk_notify_url - end - - after do - ENV.delete("ZENDESK_NOTIFY_URL") - end - - it "sends a ticket to Zendesk upon cancellation" do - stub_call(:post, zendesk_notify_url) { [200, {}, { "status" => "ok" }.to_json] } - - expect_any_instance_of(API::Support::ZendeskNotify).to receive(:notify). - with("cancellation", { supplier: "Poplidays", supplier_id: "123", bridge_id: "392" }). - once.and_call_original - - subject.call(params) - end + it_behaves_like "Zendesk cancellation notification", supplier: "Poplidays" end diff --git a/spec/api/controllers/shared/cancel.rb b/spec/api/controllers/shared/cancel.rb index 06ece6df9..e6091eb80 100644 --- a/spec/api/controllers/shared/cancel.rb +++ b/spec/api/controllers/shared/cancel.rb @@ -33,6 +33,31 @@ end end +RSpec.shared_examples "Zendesk cancellation notification" do |supplier:| + include Support::HTTPStubbing + + let(:params) { { reference_number: "123", inquiry_id: "392" } } + let(:zendesk_notify_url) { "https://www.zendesk-notify-example.org" } + + before do + ENV["ZENDESK_NOTIFY_URL"] = zendesk_notify_url + end + + after do + ENV.delete("ZENDESK_NOTIFY_URL") + end + + it "sends a ticket to Zendesk upon cancellation" do + stub_call(:post, zendesk_notify_url) { [200, {}, { "status" => "ok" }.to_json] } + + expect_any_instance_of(API::Support::ZendeskNotify).to receive(:notify). + with("cancellation", { supplier: supplier, supplier_id: "123", bridge_id: "392" }). + once.and_call_original + + subject.call(params) + end +end + private def call(controller, params) From 9324f97d519c2b00493a96674209c04016b533f3 Mon Sep 17 00:00:00 2001 From: Renato Mascarenhas Date: Wed, 21 Sep 2016 13:49:11 +0800 Subject: [PATCH 31/38] Include module on controllers. --- apps/api/controllers/atleisure/cancel.rb | 1 + apps/api/controllers/poplidays/cancel.rb | 1 + 2 files changed, 2 insertions(+) diff --git a/apps/api/controllers/atleisure/cancel.rb b/apps/api/controllers/atleisure/cancel.rb index 028608400..fa72e9ec4 100644 --- a/apps/api/controllers/atleisure/cancel.rb +++ b/apps/api/controllers/atleisure/cancel.rb @@ -1,4 +1,5 @@ require_relative "../cancel" +require_relative "../zendesk_notify_cancellation" module API::Controllers::AtLeisure diff --git a/apps/api/controllers/poplidays/cancel.rb b/apps/api/controllers/poplidays/cancel.rb index 849ce2d20..3454ebb21 100644 --- a/apps/api/controllers/poplidays/cancel.rb +++ b/apps/api/controllers/poplidays/cancel.rb @@ -1,4 +1,5 @@ require_relative "../cancel" +require_relative "../zendesk_notify_cancellation" module API::Controllers::Poplidays From 96f0dbd9a719d13bb80098b3cadbc35879f2cfe0 Mon Sep 17 00:00:00 2001 From: Renato Mascarenhas Date: Wed, 21 Sep 2016 14:10:52 +0800 Subject: [PATCH 32/38] Ensure the reference number is returned in case of success. --- apps/api/controllers/zendesk_notify_cancellation.rb | 8 +++++++- spec/api/controllers/shared/cancel.rb | 7 ++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/apps/api/controllers/zendesk_notify_cancellation.rb b/apps/api/controllers/zendesk_notify_cancellation.rb index 54860c6cc..7f4c11284 100644 --- a/apps/api/controllers/zendesk_notify_cancellation.rb +++ b/apps/api/controllers/zendesk_notify_cancellation.rb @@ -14,11 +14,17 @@ module API::Controllers module ZendeskNotifyCancellation def cancel_reservation(params) - API::Support::ZendeskNotify.new.notify("cancellation", { + zendesk_notify = API::Support::ZendeskNotify.new.notify("cancellation", { supplier: supplier_name, supplier_id: params[:reference_number], bridge_id: params[:inquiry_id] }) + + if zendesk_notify.success? + Result.new(params[:reference_number]) + else + zendesk_notify + end end end diff --git a/spec/api/controllers/shared/cancel.rb b/spec/api/controllers/shared/cancel.rb index e6091eb80..026b0e534 100644 --- a/spec/api/controllers/shared/cancel.rb +++ b/spec/api/controllers/shared/cancel.rb @@ -54,7 +54,12 @@ with("cancellation", { supplier: supplier, supplier_id: "123", bridge_id: "392" }). once.and_call_original - subject.call(params) + response = call(described_class.new, params) + + expect(response.body).to eq({ + "status" => "ok", + "cancelled_reference_number" => "123" + }) end end From 573daf9e0768e13542ae55419961a41f7600ab65 Mon Sep 17 00:00:00 2001 From: Renato Mascarenhas Date: Wed, 21 Sep 2016 15:45:58 +0800 Subject: [PATCH 33/38] Fix AtLeisure cancel route. --- apps/api/config/routes.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/api/config/routes.rb b/apps/api/config/routes.rb index c97dd37a7..a854bd0d1 100644 --- a/apps/api/config/routes.rb +++ b/apps/api/config/routes.rb @@ -22,7 +22,7 @@ post '/kigo/cancel', to: 'kigo#cancel' post '/kigo/legacy/cancel', to: 'kigo/legacy#cancel' post '/poplidays/cancel', to: 'poplidays#cancel' -post '/atleisure/cancel', to: 'atleisure#cancel' +post '/atleisure/cancel', to: 'at_leisure#cancel' post '/checkout', to: 'static#checkout' get '/kigo/image/:property_id/:image_id', to: 'kigo#image' From 36e3eee1f25de3902520da0d77c73eb588412986 Mon Sep 17 00:00:00 2001 From: Renato Mascarenhas Date: Wed, 21 Sep 2016 16:00:51 +0800 Subject: [PATCH 34/38] Content-type header when making requests. Strict specs. --- apps/api/support/zendesk_notify.rb | 2 +- spec/api/support/zendesk_notify_spec.rb | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/apps/api/support/zendesk_notify.rb b/apps/api/support/zendesk_notify.rb index 0281f4005..b5d43b424 100644 --- a/apps/api/support/zendesk_notify.rb +++ b/apps/api/support/zendesk_notify.rb @@ -44,7 +44,7 @@ def notify(ticket_id, attributes) attributes: attributes } - result = http.post(endpoint, params) + result = http.post(endpoint, params, { "Content-Type" => "application/json" }) return result unless result.success? parse_response(result.value.body) diff --git a/spec/api/support/zendesk_notify_spec.rb b/spec/api/support/zendesk_notify_spec.rb index 7ff427a12..4311483c0 100644 --- a/spec/api/support/zendesk_notify_spec.rb +++ b/spec/api/support/zendesk_notify_spec.rb @@ -67,13 +67,23 @@ end it "returns a successful result in case the ticket is properly sent" do - stub_call(:post, zendesk_notify_url) { - body = { + request_body = { + ticketId: "cancellation", + attributes: { + supplier: "Supplier Y", + supplier_id: "123", + bridge_id: "321" + } + } + headers = { "Content-Type" => "application/json" } + + stub_call(:post, zendesk_notify_url, strict: true, headers: headers, body: request_body) { + response_body = { status: "ok", message: "Ticket sent successfully" } - [200, {}, body.to_json] + [200, {}, response_body.to_json] } result = subject.notify(ticket_id, attributes) From fe66ce1e3bcae9eac378d682cd3280321d5ef9c6 Mon Sep 17 00:00:00 2001 From: Renato Mascarenhas Date: Wed, 21 Sep 2016 16:07:02 +0800 Subject: [PATCH 35/38] Properly encode request body to JSON before sending request. --- apps/api/support/zendesk_notify.rb | 2 +- spec/api/support/zendesk_notify_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/api/support/zendesk_notify.rb b/apps/api/support/zendesk_notify.rb index b5d43b424..73ff476c5 100644 --- a/apps/api/support/zendesk_notify.rb +++ b/apps/api/support/zendesk_notify.rb @@ -44,7 +44,7 @@ def notify(ticket_id, attributes) attributes: attributes } - result = http.post(endpoint, params, { "Content-Type" => "application/json" }) + result = http.post(endpoint, json_encode(params), { "Content-Type" => "application/json" }) return result unless result.success? parse_response(result.value.body) diff --git a/spec/api/support/zendesk_notify_spec.rb b/spec/api/support/zendesk_notify_spec.rb index 4311483c0..f8e35bc5a 100644 --- a/spec/api/support/zendesk_notify_spec.rb +++ b/spec/api/support/zendesk_notify_spec.rb @@ -74,7 +74,7 @@ supplier_id: "123", bridge_id: "321" } - } + }.to_json headers = { "Content-Type" => "application/json" } stub_call(:post, zendesk_notify_url, strict: true, headers: headers, body: request_body) { From 97c20921ad848c58418cbd19b0ad23ac75dcab16 Mon Sep 17 00:00:00 2001 From: Renato Mascarenhas Date: Wed, 21 Sep 2016 16:18:36 +0800 Subject: [PATCH 36/38] Increase timeout for Zendesk API calls. --- apps/api/support/zendesk_notify.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/apps/api/support/zendesk_notify.rb b/apps/api/support/zendesk_notify.rb index 73ff476c5..718742970 100644 --- a/apps/api/support/zendesk_notify.rb +++ b/apps/api/support/zendesk_notify.rb @@ -24,6 +24,11 @@ class ZendeskNotify SUPPORTED_TICKETS = %w(cancellation) + # Zendesk's API is very slow when sending tickets. Especially on the sandbox + # environment. As the cancellation webhook is sent on the background, without + # blocking the user interface, we can afford a higher timeout. + CONNECTION_TIMEOUT = 20 + attr_reader :http, :endpoint # initializes internal state. The +ZendeskNotify+ service URL must be properly @@ -32,7 +37,7 @@ def initialize uri = URI.parse(zendesk_notify_url) host = [uri.scheme, "://", uri.host].join - @http = Concierge::HTTPClient.new(host) + @http = Concierge::HTTPClient.new(host, timeout: CONNECTION_TIMEOUT) @endpoint = uri.request_uri end From 7fc6f39e46d6db18358e285f88e963d5383e639f Mon Sep 17 00:00:00 2001 From: Renato Mascarenhas Date: Wed, 21 Sep 2016 16:28:50 +0800 Subject: [PATCH 37/38] Update change log. --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1c21b350..3054c48bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ of Concierge. Please check the Wiki entry on the release process to understand how this file is formatted and how the process works. ## Unreleased +### Added +- `ZendeskNotify` client with ticket creation on cancellation of bookings from Poplidays and AtLeisure + ### Fixed - Ciirus:: ignore permissions error about MC disabled clone properties From 160bfae7c7042f9a260f5b95ba9c063331d6356d Mon Sep 17 00:00:00 2001 From: Keang Date: Wed, 21 Sep 2016 16:49:15 +0800 Subject: [PATCH 38/38] Release cut 0.11.0 :knife: --- CHANGELOG.md | 2 +- lib/concierge/version.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d7826dfa..7311ab850 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ This file summarises the most important changes that went live on each release of Concierge. Please check the Wiki entry on the release process to understand how this file is formatted and how the process works. -## Unreleased +## [0.11.0] - 2016-09-21 ### Added - `ZendeskNotify` client with ticket creation on cancellation of bookings from Poplidays and AtLeisure diff --git a/lib/concierge/version.rb b/lib/concierge/version.rb index c7350382e..a58fd3d33 100644 --- a/lib/concierge/version.rb +++ b/lib/concierge/version.rb @@ -1,3 +1,3 @@ module Concierge - VERSION = "0.10.0" + VERSION = "0.11.0" end