Event blacklisting

[rowanthorpe]

As mentioned in #103 I have based this PR on the HEAD of that PR rather than the present HEAD of development, as it makes sense to merge that first, which will then leave this PR with just the one commit on top of it. If reviewing this PR before merging the other one, only look at the last commit - all the others are just the commits from #103.

This commit ended up being more complex than I expected - mainly because it requires:

1. getting the rawEvent data
2. getting the data from the Google Contact (including the blacklist field if it exists)
3. getting events from the Google Contact that aren't in the blacklist
4. getting the data and events from the Google+ profile if it exists
5. trimming any events that had already been added from the rawEvent if they turned out to be in the blacklist subsequently retrieved from the Google Contact
6. trimming any contacts who end up having no remaining events (because they turned out to all be blacklisted)

...a bit like being locked inside a trunk, holding the key to open the padlock on the outside of the trunk...

[rowanthorpe]

As much for my own future reference as anything else, one of the non-obvious decisions I made was to order it in such a way that if it is (or becomes) possible to also add a custom notificationBlacklist field to the Contact in Google+ (?), then all that would be needed is a copy-paste of the code for getting the blacklist from the Google Contact (& a tweak for differently-named fetch-command, & to do a concatenate-to-or-create rather than just create), in order for blacklisting to work additively from both Google Contacts & Google+.

[GioBonvi]

Additionally I'd like to hear your opinion on making the notificationBlacklist field case insensitive.

Having it case sensitive lets a user have various custom events with different capitalization in the same contact while blacklisting only some of them, but I can imagine it could also cause lots of issues like "I blacklisted the 'birthday' [first letter lowercase] event in one of my contacts, why do I still receive a birthday notification for that contact?"

We could try dealing with this by inserting a warning in the documentation of this option, but:

• given the notorious ability users have to ignore warnings
• considering the extremely small number of users that actually
  1. will have a contact with multiple events only differing in capitalization
  2. want to blacklist just some of them

I'd opt for the case-insensitive comparison.

[GioBonvi]

I think this conditional should be formatted differently: as it is now it's quite difficult to grasp operators precedences and meanings.

It could use some more spacing (not so pleasing to look at, but easy to understand at first glance):

if (
  (
    (!eventDay && !eventMonth) ||
    (thisEventMonth === eventMonth && thisEventDay === eventDay)
  ) && (
    !self.blacklist || !self.blacklist.length || !isIn(beautifyLabel(thisEventLabel), self.blacklist)
  )
) {

or extracting partial conditions (prettier in my opinion, but still easily understandable):

dateMatches = (!eventDay && !eventMonth) || (thisEventMonth === eventMonth && thisEventDay === eventDay));
blacklistMatches = self.blacklist && self.blacklist.length && isIn(beautifyLabel(thisEventLabel), self.blacklist)
if (dateMatches && !blacklistMatches) {

[rowanthorpe]

OK, in a while I will change it to use the partially extracted conditions you showed (or if you prefer not to wait feel free to add that change as a commit to the PR branch).

[rowanthorpe]

I had considered the case-insensitivity option, and was kind of on the fence about it because the problem you explain about users not heeding warnings and getting confused by "under-matching" due to case-sensitivity may also apply a bit with "over-matching" due to case-insensitivity. The decider for me was that accidentally sending a reminder-email which shouldn't be sent is less of a disaster than accidentally not sending one which is expected (and possibly missing a birthday, and ending up sleeping on the sofa).

On the one hand - considering the notificationBlacklist functionality is opt-in and advanced - users of it should be able to heed warning logs, but on the other hand - I guess if someone has both a BIRTHDAY field and and a CUSTOM field called Birthday (or two CUSTOM fields called SillyName and SILLYname) they are probably asking for trouble, wittingly or unwittingly. For that matter someone might add a CUSTOM entry called BIRTHDAY in which case it will be a problem even with case-sensitivity.

I suppose the option which scores lower on the Principle of least astonishment scale would be case-insensitivity. In a while I will change the PR to be case-insensitive unless you change your mind and let me know before then.

[rowanthorpe]

While addressing the case-insensitivity issue, because the whole state of event-labels, coercions thereof, and fuzzy-matching of stringified labels had already been nagging at me for a long time, I rewrote my commit to make event-label handling less "fuzzy" in general (not just in my new code, but throughout the script):

• .toLocaleLowerCase() is used instead of .toLowerCase() throughout
• as the blacklist now only does case-insensitive matching the event-labels are stored in it in lower-case, but inserting/leaving CUSTOM: prefix in place for custom labels
• event-labels are stringified and custom event-labels are CUSTOM:-prefixed at the earliest opportunity during fetch, but lower-casing is only done at the last minute for comparison against the blacklist, and beautification is only done at the last minute for outputting to the final email text

That should hopefully make event-label handling more straightforward and predictable from now on.

I also split the monolithic conditional into partially extracted conditions as you requested.

Because these changes impacted your subject-fix commit a lot, I rebased it on top of my new commits and then did semistandard-check and jsdoc-compile-check, and then ran some tests (of blacklisting fields, and of the eventTypes exclusion settings) with various dates and fields. It seemed to work OK, so I pushed your commit to this branch too. If you want to put it in a separate PR, you can cherry-pick it from there and tell me to remove it from my branch, or whatever you prefer.