Changing Colt parsing to use Subject and remove ICal

[pke11y]

This PR is an update for Colt notifications to accommodate maintenance notification status updates, which do not contain .ics files. All info is in the email subject.

[glennmatthews]

Can you clarify why you've removed ICalParserColt1? Based on my limited understanding it seems like this PR should be adding a new processor/parser option, not removing/replacing the iCal support.

[pke11y]

@glennmatthews the objective was to consolidate the number of parsers. Update notifications do no contain an ICal attachment. Most of the useful information is contained in the subject for all notifications. So we're extracting the account number from the CSV file now instead of the ICal attachment and all of the rest is in the subject.

[chadell]

LGTM

[chadell]

If we are removing this parser, why do we keep the test data file?

[pke11y]

Seems to be an old version. The test has been removed. The data file is still in the repo as the colt1 is an .ics attachment included in the colt3.eml.

[glennmatthews]

@glennmatthews the objective was to consolidate the number of parsers. Update notifications do no contain an ICal attachment. Most of the useful information is contained in the subject for all notifications. So we're extracting the account number from the CSV file now instead of the ICal attachment and all of the rest is in the subject.

Hm, okay. I had thought that a goal of this library was to prefer the standard iCal format where possible since it's an emerging standard. Disregarding a (standardized, easy-to-parse) iCal attachment from this specific provider in favor of (fragile, error-prone) HTML parsing feels like a step backwards to me. I'll defer to you and @chadell on this point though.

[chadell]

@glennmatthews the objective was to consolidate the number of parsers. Update notifications do no contain an ICal attachment. Most of the useful information is contained in the subject for all notifications. So we're extracting the account number from the CSV file now instead of the ICal attachment and all of the rest is in the subject.

Hm, okay. I had thought that a goal of this library was to prefer the standard iCal format where possible since it's an emerging standard. Disregarding a (standardized, easy-to-parse) iCal attachment from this specific provider in favor of (fragile, error-prone) HTML parsing feels like a step backwards to me. I'll defer to you and @chadell on this point though.

@glennmatthews you are right about the preference for the icalendar standard format, but this one is not following the BCOP, just using icalendar and the point from the PR (if I understand correctly) was to use the email subject to get all the information that was part of the icalendar part because the Status info was not part of the icalendar one and the subject parser was also needed, so it gets rid of one parser that was providing info that was available on the other parts, and was useless alone. does it sound accurate @pke11y ?

[pke11y]

@glennmatthews the objective was to consolidate the number of parsers. Update notifications do no contain an ICal attachment. Most of the useful information is contained in the subject for all notifications. So we're extracting the account number from the CSV file now instead of the ICal attachment and all of the rest is in the subject.

Hm, okay. I had thought that a goal of this library was to prefer the standard iCal format where possible since it's an emerging standard. Disregarding a (standardized, easy-to-parse) iCal attachment from this specific provider in favor of (fragile, error-prone) HTML parsing feels like a step backwards to me. I'll defer to you and @chadell on this point though.

@glennmatthews you are right about the preference for the icalendar standard format, but this one is not following the BCOP, just using icalendar and the point from the PR (if I understand correctly) was to use the email subject to get all the information that was part of the icalendar part because the Status info was not part of the icalendar one and the subject parser was also needed, so it gets rid of one parser that was providing info that was available on the other parts, and was useless alone. does it sound accurate @pke11y ?

That's correct. We need the subject parser to get update Status and it turns out all of the other major data can be retrieved from the subject too.

[chadell]

LGTM