Skip to content

Comments

fix(calendar): use Calendars.Get for TZ lookup to fix SA 404#325

Open
markwatson wants to merge 1 commit intosteipete:mainfrom
markwatson:fix/sa-404-calendar-list
Open

fix(calendar): use Calendars.Get for TZ lookup to fix SA 404#325
markwatson wants to merge 1 commit intosteipete:mainfrom
markwatson:fix/sa-404-calendar-list

Conversation

@markwatson
Copy link

Hi @steipete,

When using with a service account, I ran into some 404s when running: gog calendar list email@example.com (or events). This was true even when gog calendar calendars did in fact return a couple calendars.

It appears this is an issue with how service accounts manage their list of calendars. In time_helpers.go, we call CalendarList.get(calendarID):

 // getCalendarLocation fetches a calendar's timezone and returns it as a location.
func getCalendarLocation(ctx context.Context, svc *calendar.Service, calendarID string) (string, *time.Location, error) {
        //...

        cal, err := svc.CalendarList.Get(calendarID).Context(ctx).Do()
        //...

But svc.CalendarList doesn't contain the calendar for the service account, so it fails. Instead this change will use svc.Calendars.Get in order to always be able to lookup the calendars. I'm unsure what exactly controls this list for service accounts, but in my case I somehow had the calendars in the list, and then suddenly didn't. The documentation I found for the differences is here. There's also a similar change for the time zone helper.

I also manually retested this using the existing OAuth / standard flow documented in the README, and it appears to still
work. I would expect something similar to happen with the CalendarCalendarsCmd usage of svc.CalendarList.List(), but in my testing that did not seem to be the case.

Please let me know if I can change / improve any of this. I know it's a slight change, so if it's not something you want to do I won't mind. I needed the code for a project I was working on, so figured I would share it in case it's useful to others.

Thanks!

Mark

@markwatson
Copy link
Author

Note: I accidentally opened this PR previously, but it wasn't tested enough. Now that I've tested it, I opened a new PR to keep the branch naming consistent (fix/...), instead of reopening the existing one. Apologies for the noise.

Comment on lines +34 to +35
// Uses Calendars.Get (not CalendarList.Get) so it works for service accounts
// whose "primary" calendar may not appear in their CalendarList.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These comments are probably unnecessary - I can remove them if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant