GoogleContactsEventsNotifier icon indicating copy to clipboard operation
GoogleContactsEventsNotifier copied to clipboard

reminder eMail is not using photos from Google Contacts

Open stephankn opened this issue 4 years ago • 3 comments

Steps to reproduce

Set a proper test date to match a contact having a photo in Google Contacts and run test

Expected behavior

email should contain the photo from contacts.

Current behavior

email uses default profile image as contact in question has probably no public photo.

Context

  • Version of the script: 5.0.0

Possible solution

I suggest to read the photo from contacts. API is at https://developers.google.com/contacts/v3#retrieving_a_contacts_photo

I debugged a bit. Reading photo seems possible, but I do not fully understand what data sources get merged to have the current photo URL.

stephankn avatar Feb 02 '20 23:02 stephankn

Yes, I think we looked at this in the past, but could not get it to work, possibly due to differences between the Contact API you linked and the Contact API available in the Google Script environment.

If anyone wants to look further into this you are more than welcome.

GioBonvi avatar Feb 25 '20 13:02 GioBonvi

I have it working. Give me a bit as I currently travel. I will send a patch. As I reworked the photo code to reduce the number of external get calls it differs a bit more.

stephankn avatar Feb 26 '20 00:02 stephankn

I have some updates regarding this issue and PR #169.

After some tweaking and looking around I managed to get the People API to work directly through Apps Script. The main problem was that while it provided access to the contacts profile images I could not see an easy way to link a contact from ContactsApp (which is what the script used) to a contact from the People API.

I decided to try a drastic approach: reorganizing and partially rewriting the script almost from scratch.

In the last years I found myself getting back to this project less and less frequently and each time the code seemed more and more alien and complex, making it really difficult to intervene if not for small fixes.
I had this idea in the back of my mind for some time and this looked like a good opportunity. These were the things I had in mind:

  • The code was already quite modular: it was not necessary to completely rewrite everything. In fact most of the work was more along the lines of extracting each piece and reworking/streamlining it or ditching it completely in some cases.
  • This new People API offered all the important features of ContactsApp (plus access to the profile images), but from a single source, which offered great potential for simplification.
  • In fact it was easy to remove all the logic related to the merge of data from different sources (calendar, contacts, Google+), cutting hundreds of lines of code and really improving the data flow, which is now restricted to two or three basic functions.
  • Lastly I wanted to improve the modularity even more: right now it would theoretically possible and easy to implement different "outputs" for the notifications (e.g. push notifications, a web interface...) since the "parsing" of the data from the contacts is very simplified and isolated from the rest of the code. I don't know if this is actually feasible or even desirable, but I liked the idea.

In general I am quite pleased with the results, but would really appreciate some feedback: I am still not so sure this was the right approach.
Some work and checks are obviously still necessary: to me it looks like all the previously existing funcitonalities are present and working, but I might have missed something or some edge cases.

The code is available in the refactor branch.

GioBonvi avatar Aug 12 '20 11:08 GioBonvi