GioBonvi / GoogleContactsEventsNotifier

Receive automatic email notifications before your Google Contacts birthday and other events!
MIT License
219 stars 50 forks source link

reminder eMail is not using photos from Google Contacts #162

Open stephankn opened 4 years ago

stephankn commented 4 years ago

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

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.

GioBonvi commented 4 years ago

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.

stephankn commented 4 years ago

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.

GioBonvi commented 4 years ago

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:

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.