Skip to content

Conversation

mrtopsyt
Copy link
Contributor

Ticket

Closes #4305.

Reviewers

@hunterckx .
@NoopDog

Changes

  • Added Python functions to the analytics package to save a DF to a spreadsheet
  • Added functions to create a DF of outbound links

Known Issues

  • Currently this uses both the custom "outbound_link_clicked" event and the builtin "Click" event. This ensures that all clicks get tracked, but leads to minor inaccuracies since they sometimes have different (although usually very close) values for the same path / link

Copy link
Contributor

@hunterckx hunterckx left a comment

Choose a reason for hiding this comment

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

I haven't looked at this fully yet, but one thing that comes to mind is that the version in setup.py should be updated -- specifically I intended it to be semantic versioning, so I would increase the version to 3.1.0, since I think this a non-breaking feature change

@mrtopsyt mrtopsyt force-pushed the jonah/4305-gspread-analytics-package branch from f7a8e93 to 04d03af Compare December 18, 2024 07:13
@mrtopsyt
Copy link
Contributor Author

@hunterckx whoops, thanks for letting me know, I wasn't familiar with setupTools. I bumped the version and added the new dependencies in setup.py

@mrtopsyt mrtopsyt requested a review from hunterckx December 18, 2024 07:14
# Build Drive API
drive_credentials = extract_credentials(authentication_response)
gc = gspread.authorize(drive_credentials)
drive_api = build('drive', 'v3', credentials=drive_credentials)
Copy link
Contributor

Choose a reason for hiding this comment

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

I imagine that, since authenticate_gspread and authenticate_drive_api exist, they should be used here instead of initializing the API clients directly


def authenticate_drive_api(authentication_response):
"""Authenticates the Drive API using the credentials in the tuple from api.authenticate"""
return build('drive', 'v3', credentials=extract_credentials(authentication_response))
Copy link
Contributor

Choose a reason for hiding this comment

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

The tuple returned from calling api.authenticate with drive_service_params should have the Drive API as its first item, so I don't think calling build here is necessary

@mrtopsyt
Copy link
Contributor Author

mrtopsyt commented Dec 20, 2024

Thanks for those reviews @hunterckx! Let me know if it looks good now

@mrtopsyt mrtopsyt requested a review from hunterckx December 20, 2024 20:33
Copy link
Contributor

@hunterckx hunterckx left a comment

Choose a reason for hiding this comment

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

Looks good!

@hunterckx hunterckx merged commit e8c34c6 into main Dec 21, 2024
4 checks passed
@MillenniumFalconMechanic MillenniumFalconMechanic deleted the jonah/4305-gspread-analytics-package branch February 28, 2025 21:42
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.

Update outbound links report to create a google sheet in the the cloud instead of a local excel file

3 participants