to_h for invalid calendars

[kkolotyuk]

Current Calendar#parse_entries method:

    def parse_entries
      return empty_response if entries.empty?

      sorted_entries = entries.select(&:valid?).sort_by(&:date)
      start_date     = sorted_entries.first.date
      end_date       = sorted_entries.last.date

      rates          = Rates.new([], [], [])
      parsed_entries = ParsedEntries.new(start_date, "", "", "", [], rates, [])

      # index all entries by date, to make the lookup for a given date faster.
      # Index once, and then all lookups can be performed in constant time,
      # as opposed to scanning the list of entries on every iteration.
      #
      # Implementation of what Rails provide as the +index_by+ method.
      index = {}.tap do |i|
        sorted_entries.each do |entry|
          i[entry.date] = entry
        end
      end

      (start_date..end_date).each do |date|
        entry = index[date] || default_entry(date)
        ...
      end

As well as calendar.to_h is used in case when it is invalid this method should be resistant to invalid calendar. And, as I understand, that is why we build index by all the entries (valid and invalid). But if some invalid entry affect determine of start_date or end_date we will not see this entry in the result of to_h function.

If this problem is important I suggest to discuss possible solutions. I couldn't find elegant one. I tried to remove .select(&:valid?) but if some entry doesn't contain date we have an error.

May be the best solution create separate to_h method for invalid calendar case.

@rmascarenhas please let me know what do you think.

[rmascarenhas]

I'm not sure I understood the problem. Perhaps if you could give me an example, with data, of what you are trying to explain, it would be clearer?

[kkolotyuk]

Briefly, let's say calendar consists of three entries and the first one (sorted by date) is invalid: [invalid, valid, valid]. For this example to_h method will show only two last entries. And actually it is not bad, but we use to_h in rescue section:

    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,
      # since the API call to Roomorama is going to fail (the +identifier+
      # will not be recognised.)
      return unless synchronised?(calendar.identifier)

      calendar.validate!
      update_counters(calendar)
      operation = Roomorama::Client::Operations.update_calendar(calendar)

      run_operation(operation)
    rescue Roomorama::Error => err
      missing_data(err.message, calendar.to_h)
      announce_failure(Result.error(:missing_data))
      false
    end

and, as I understand, expect that calendar.to_h helps to analyze the situation. My example shows that there is possible cases when calendar.to_h doesn't even show invalid entries.

Let me know if I'm unclear again.