Net rate calculation bug

[kkolotyuk]

During Quotation#net_rate calculation we tried to find property by property_id

  def property
    @property ||= PropertyRepository.identified_by(property_id).first
  end

But property_id is not unique in our DB. We need to use supplier_name or even better host_id

[kkolotyuk]

@keang what do you think if we add supplier_name field to the Quotation entity and fill it during quote webhook process for all the suppliers?

[sharipov-ru]

I don't much like the idea of adding supplier_name to Quotation entity.

Names can change, so I'm fan of adding host_id field to Quotation, but this possibly will be a big change (need to change all integrations)

[kkolotyuk]

Yes, host_id is better. But this info should be sent by Roomorama, and as I understand Roomorama doesn't know Concierge's host_id, it doesn't know even supplier_id, it knows only supplier's API URL.

[sharipov-ru]

Yeah...

I think we have lots of similar PropertyRepository.identified_by(property_id) calls in the project, so if we really can't guarantee the uniqueness of property_id we have to find a ways how to do that until it haven't got a big trouble.

[sharipov-ru]

We need to have to make a decision here. Our options are:

1. add a new field to Quotation entity so that Quotation can add additional scope for property search

OR

2. extract fetching host fee percentage on a higher level and pass fetched percentage value to Quotation

Since Quotation is a just Hanami entity for representing the object from / to database, it feels like having calls to Repository classes from the inside of Quotation makes this class complicated & violates single responsibility principle & violates the idea of repository <-> entities separation. Having calls to repositories makes our Quotation entity to look like usual rails' active record model 💩

All our other entity classes don't perform calls to repositories and Quotation is only class which does it.

@keang @kkolotyuk what about refactoring of host fee calculation and finding a better place for it?

[kkolotyuk]

Refactoring is a good thing. All developers know it =) But how we are going to solve the real problem here? Looks like from concierge side we can only try to find unique property per supplier (by supplier name).