fix/ciirus skip monthly rates

[kkolotyuk]

FIx for #279

[kkolotyuk]

@keang please

[keang]

Related to #335, I think this and other ignored errors properties would be forgotten silently 2 months from now 💭

[keang]

It would be worthwhile have a log of the ids that were skipped, grouped by reasons? So when we have time to revisit, or just when we communicate with suppliers we can reproduce the ids quickly.

For now we have to shift through ExternalErrors; with this patch (and similar ones) we completely lose the data. For this particular case, for example, we have the option of contacting the host and let them know which setup is missing, etc. What do you think @kkolotyuk @rmascarenhas ?

[kkolotyuk]

This is good point. I thought about improvement of skip_property method and not only count such properties, but also store additional info for them. But this is difficult task - to analyze and group errors.

What I see now: a lot of soap_errors are exactly about this fix and they make it difficult to analyze Ciirus errors in Concierge UI.

[keang]

But since we know a few cases that we expect to occur, maybe what we can do is just build a hash like:

ignored: [
  {
    reason: "(soap:Server) Server was unable to process request. ---> GetPropertyRates: Error - No Rate Assigned by user. Please contact the user and request they populate this data."
    ids: [a, b, c]
  }, {
    reason: "(soap:Server) Server was unable to process request. ---> GetPropertyRates: Error - No Rate Rows Returned",
    ids: [x, y]
  }
]

Or something like that and just throw the hash as json into the SynchronisationRepository?

[kkolotyuk]

Yes this is exactly I was talking about.
For now stats looks like:

{"properties_created":0,"properties_updated":0,"properties_deleted":0,"properties_skipped":1}

but can be like this:

{
  properties_created: 0,
  properties_updated: 0,
  properties_deleted: 0,
  properties_skipped: [
    {
      reason: "(soap:Server) Server was unable to process request. ---> GetPropertyRates: Error - No Rate Assigned by user. Please contact the user and request they populate this data."
      ids: [a, b, c]
    }, {
      reason: "(soap:Server) Server was unable to process request. ---> GetPropertyRates: Error - No Rate Rows Returned",
      ids: [x, y]
    }
  ]
}

[kkolotyuk]

If we will change stats structure we need new place (may be new page) in Concierge UI about skipped properties. This is not small task and it should be done in separate PR I think.

[keang]

Yes true, let's merge this first, it looks good for its issue
👍

[keang]

Moving some of discussions here into an enhancement issue: #359

[kkolotyuk]

👍