Bugfix for kigo quote price

[sharipov-ru]

Previously PR #411 added condition to select the maximum of two possibly different minimum stay values:

property.minimum_stay = [
  property.minimum_stay.to_i,
  pricing_mapper.minimum_stay
].max

Fix was applied to property mapper, however, calendar builder was still using only value from pricing hash without any knowledge that property's minimum stay is stricter. This was the reason of problems when we had minimum_stay = 28 in database, but calendar still allowed period selections of less than 28 days, causing us to get lots of

"The property pricing information is unavailable due to a constraint. This Property has a minimum stay of 28 nights.No Rates AvailableUnfortunately, this unit is not available for your selected period. Please try another unit."

errors while quoting prices. Example: https://concierge-web.roomorama.com/errors/631156

This PR changes Kigo::Calendar to use the same [val1, val2].max logic as property mapper does. Also minimum stays selection extracted to its own class.

Value in property database and calendar entry now synced with the same value. However, due to different intervals of workers start times, it's still possible that values be different from each other. But in that case everything will be working because calendar entry's value will be the most "fresh" one and it will still check which of two minimum stay values should be used.

[sharipov-ru]

This PR fixes #381

[keang]

We parse the Kigo period as:

module Kigo
  # +Kigo::TimeInterval+
  #
  # represents days count according to +UNIT+ type
  # possible unit types NIGHT, MONTH, YEAR
  #
  # example:
  #
  #   TimeInterval.new({"UNIT"=>"NIGHT", "NUMBER"=>3})
  TimeInterval = Struct.new(:interval) do
    def days
      return nil if interval['NUMBER'].zero?          # <---- could be nil

      multiplier = { 'MONTH' => 30, 'YEAR' => 365 }.fetch(interval['UNIT'], 1)
      interval['NUMBER'].to_i * multiplier
    end
  end

Wonder if you saw this? maybe the condition above to return nil is wrong..

[sharipov-ru]

Yes, that's why I applied to_i for both minimum_stay values before calculating max (nil.to_i evaluates to 0)

We may return 0 instead of nil btw, I still have to convert everything to integer, so there is no difference from getting 0 or nil, just one extra convertion.

[keang]

@sharipov-ru how about defaulting to 1 if it's zero, rather than returning error?
I think kigo is known to put nil minimum stay right?

[sharipov-ru]

Yes, in this PR you said:

On production currently it was counted about 1260 properties with minimum_stay = 1, so we risk disabling all of it.

So yes, let's keep being on this path. 👍

If quote issues happens again, we'll investigate why property without minimum stay started require it while quote.

[sharipov-ru]

Changed MinStay class to return nil instead of Result.error.

It's now up-to-caller to decide whether minimum_stay should be nil or we should set a default value:

• property mapper sets it to 1 to pass roomorama validations
• calendar entries still have minimum_stay value equal nil (since it's not required field)

[sharipov-ru]

@keang @kkolotyuk please review again 👀

[kkolotyuk]

LGTM, but I would wait @keang review, because he knows Kigo better then me.

[keang]

👍