Merge pull request #403 from roomorama/release/0.11.6

Release/0.11.6

CHANGELOG.md

@@ -4,6 +4,14 @@ 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.
 
+## [0.11.6] - 2016-10-03
+### Fixed
+- Proper order of table columns for sync_process/index page
+- Poplidays: metadata sync is safe for `nil` availabilities
+
+### Changed
+- PropertySynchronisation#start and CalendarSynchronisation#start returns result instead of boolean
+
 ## [0.11.5] - 2016-09-28
 ### Changed
 - Workers marked as `queued` to avoid duplicated messages on SQS.

apps/web/templates/sync_processes/index.html.erb

@@ -18,8 +18,8 @@
         <th>Started</th>
         <th>Finished</th>
         <th>Properties Created</th>
-        <th>Properties Deleted</th>
         <th>Properties Updated</th>
+        <th>Properties Deleted</th>
         <th>Properties Skipped</th>
         <th>Stats</th>
       </tr>

apps/workers/calendar_synchronisation.rb

@@ -53,6 +53,8 @@ def initialize(host)
     # the +Result+ instance should wrap a +Roomorama::Calendar+ instance. If the returned
     # result is not successful, an external error is persisted to the database and the method
     # terminates with no API calls being performed to Roomorama.
+    #
+    # Returns the result from Workers::OperationRunner#perform if all is well
     def start(identifier)
       new_context(identifier) do
         result = yield(self)
@@ -63,7 +65,7 @@ def start(identifier)
           process(calendar)
         else
           announce_failure(result)
-          false
+          result
         end
       end
     end
@@ -110,6 +112,7 @@ def new_context(identifier=nil)
 
     private
 
+    # Returns the result from Workers::OperationRunner#perform, or a Result.error(:missing_data)
     def process(calendar)
       # if the property trying to have its calendar synchronised was not
       # synchronised by Concierge, then do not attempt to update its calendar,
@@ -126,8 +129,9 @@ def process(calendar)
       run_operation(operation)
     rescue Roomorama::Error => err
       missing_data(err.message, calendar.to_h)
-      announce_failure(Result.error(:missing_data))
-      false
+      Result.error(:missing_data).tap do |error|
+        announce_failure(error)
+      end
     end
 
     def update_counters(calendar)
@@ -143,8 +147,9 @@ def run_operation(operation)
       # any errors during the request can be logged.
       Concierge.context.enable!
 
-      result = Workers::OperationRunner.new(host).perform(operation, operation.calendar)
-      announce_failure(result) unless result.success?
+      Workers::OperationRunner.new(host).perform(operation, operation.calendar).tap do |result|
+        announce_failure(result) unless result.success?
+      end
     end
 
     def synchronised?(property_identifier)

apps/workers/property_synchronisation.rb

@@ -72,6 +72,8 @@ def initialize(host)
     #   the property. In this case, the external error is also persisted. The caller
     #   is encouraged to have augmented +Concierge.context+ with meaningful information
     #   to aid debugging.
+    #
+    # Returns the result from Workers::OperationRunner#perform if all is well
     def start(identifier)
       new_context(identifier) do
         result = yield(self)
@@ -82,7 +84,7 @@ def start(identifier)
         else
           failed!
           announce_failure(result)
-          false
+          result
         end
       end
     end
@@ -173,25 +175,32 @@ def new_context(identifier=nil)
     #
     # It uses +Workers::Router+ to determine which operation should be
     # performed on the property, if any, and runs it.
+    #
+    # It returns the result from Workers::OperationRunner#perform
+    # or a Result.error(:no_operation) if router couldnt' find a suitable
+    # operation to dispatch
     def push(property)
       processed << property.identifier
 
-      router.dispatch(property).tap do |operation|
-        if operation
-          result = run_operation(operation, property)
+      operation = router.dispatch(property)
+
+      if operation
+        run_operation(operation, property).tap do |result|
           update_counters(operation) if result.success?
         end
+      else
+        Result.error(:no_operation)
       end
     end
 
     def process(property)
       property.validate!
       push(property)
-      true
     rescue Roomorama::Error => err
       missing_data(err.message, property.to_h)
-      announce_failure(Result.error(:missing_data))
-      false
+      Result.error(:missing_data).tap do |error|
+        announce_failure(error)
+      end
     end
 
     def purge_properties

apps/workers/suppliers/poplidays/metadata.rb

@@ -57,7 +57,7 @@ def perform
     private
 
     def filter_availabilities(availabilities)
-      availabilities['availabilities'].select do |availability|
+      Array(availabilities['availabilities']).select do |availability|
         availability_validator(availability).valid?
       end
     end

lib/concierge/version.rb

@@ -1,3 +1,3 @@
 module Concierge
-  VERSION = "0.11.5"
+  VERSION = "0.11.6"
 end

spec/lib/concierge/suppliers/poplidays/metadata_spec.rb

@@ -9,6 +9,7 @@
   let(:properties_list) { parse_json(read_fixture('poplidays/lodgings.json')) }
   let(:property_details) { to_safe_hash(parse_json(read_fixture('poplidays/property_details.json'))) }
   let(:availabilities) { parse_json(read_fixture('poplidays/availabilities_calendar.json')) }
+  let(:empty_availabilities) { parse_json(read_fixture('poplidays/availabilities_calendar_no_availabilities.json')) }
   let(:extras) { parse_json(read_fixture('poplidays/extras.json')) }
 
   before do
@@ -158,6 +159,14 @@
         subject.perform
       }.to_not change { PropertyRepository.count }
     end
+
+    it 'skip properties with empty availabilities' do
+      allow_any_instance_of(Poplidays::Importer).to receive(:fetch_availabilities) { Result.new(empty_availabilities) }
+      allow_any_instance_of(Roomorama::Client).to receive(:perform) { Result.new('success') }
+      expect {
+        subject.perform
+      }.to_not change { PropertyRepository.count }
+    end
   end
 
   def parse_json(json_string)

spec/workers/property_synchronisation_spec.rb

@@ -38,15 +38,17 @@
         double(success?: true)
       end
 
-      subject.start("prop1") { Result.new(roomorama_property) }
+      res = subject.start("prop1") { Result.new(roomorama_property) }
+      expect(res).to be_success
       expect(operation).to be_a Roomorama::Client::Operations::Publish
       expect(operation.property).to eq roomorama_property
     end
 
     context "error handling" do
       it "announces an error if the property returned does not pass validations" do
         roomorama_property.images.first.identifier = nil
-        subject.start("prop1") { Result.new(roomorama_property) }
+        res = subject.start("prop1") { Result.new(roomorama_property) }
+        expect(res).to_not be_success
 
         error = ExternalErrorRepository.last
         expect(error.operation).to eq  "sync"
@@ -65,7 +67,8 @@
       end
 
       it "announces an error if the property failed to be processed" do
-        subject.start("prop1") { Result.error(:http_status_404) }
+        res = subject.start("prop1") { Result.error(:http_status_404) }
+        expect(res).to_not be_success
 
         error = ExternalErrorRepository.last
         expect(error.operation).to eq "sync"
@@ -87,7 +90,9 @@
         }
 
         expect {
-          subject.start("prop1") { Result.new(roomorama_property) }
+          res = subject.start("prop1") { Result.new(roomorama_property) }
+          expect(res).to be_a Result
+          expect(res).to_not be_success
         }.to change { ExternalErrorRepository.count }.by(1)
 
         error = ExternalErrorRepository.last
@@ -105,7 +110,10 @@
         }
 
         expect {
-          subject.start("prop1") { Result.new(roomorama_property) }
+          res = subject.start("prop1") { Result.new(roomorama_property) }
+          expect(res).to be_a Result
+          expect(res).to be_success
+          expect(res.value).to be_a Property
         }.not_to change { ExternalErrorRepository.count }
       end
     end
@@ -121,14 +129,16 @@
       create_property(host_id: host.id, identifier: "prop2", data: roomorama_property.to_h.merge!(identifier: "prop2"))
 
       # create
-      subject.start("prop1") { Result.new(roomorama_property) }
+      res = subject.start("prop1") { Result.new(roomorama_property) }
+      expect(res).to_not be_success
 
       # update
-      subject.start("prop2") {
+      res = subject.start("prop2") {
         roomorama_property.identifier = "prop2"
         roomorama_property.title = "Changed Title"
         Result.new(roomorama_property)
       }
+      expect(res).to_not be_success
 
       expect {
         subject.finish!