Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UIP-36 use the anonymous user id in Google Analytics calls #3345

Merged
merged 6 commits into from
Aug 28, 2023

Conversation

briehl
Copy link
Member

@briehl briehl commented Aug 11, 2023

Description of PR purpose/changes

This uses the previously introduced get_user_info call to set up the anonymous user id and send it off to Google Analytics. There's a few other minor tweaks here, too - the anon id gets added to the user's environment on session startup, and some bits of linting cleanup.

TODO:

  • full integration test

List any dependencies that are required for this change.

Jira Ticket / Issue

Related Jira ticket: https://kbase-jira.atlassian.net/browse/UIP-36

  • Added the Jira Ticket to the title of the PR (e.g. DATAUP-69 Adds a PR template)

Testing Instructions

  • Details for how to test the PR:
  • Tests pass locally and in GitHub Actions
  • Changes available by spinning up a local narrative and navigating to X to see Y

Dev Checklist:

  • My code follows the guidelines at https://sites.google.com/lbl.gov/trussresources/home?authuser=0
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • [n/a] (JavaScript) I have run Prettier and ESLint on changed code manually or with a git precommit hook
  • (Python) I have run Black and Flake8 on changed Python code manually or with a git precommit hook
  • Any dependent changes have been merged and published in downstream modules

Updating Version and Release Notes (if applicable)

@@ -99,7 +105,10 @@ def password_from_settings(cls, settings):

@classmethod
def login_available(cls, settings):
"""Whether this LoginHandler is needed - and therefore whether the login page should be displayed."""
"""
Whether this LoginHandler is needed - and therefore whether the login page should be
Copy link
Member Author

Choose a reason for hiding this comment

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

Make flake8 happy.

@@ -83,7 +83,7 @@ def get(self, path):
kill_kernel=False,
mathjax_url=self.mathjax_url,
google_analytics_id=URLS.google_analytics_id,
userName=kbase_env.user,
anon_user_id=kbase_env.anon_user_id,
Copy link
Member Author

Choose a reason for hiding this comment

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

This line is what this whole chain of PRs is about.

Copy link
Collaborator

Choose a reason for hiding this comment

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

One small edit for a programmer, one GA4 leap for programmer kind!

@codecov
Copy link

codecov bot commented Aug 11, 2023

Codecov Report

Merging #3345 (1ef8ecd) into develop (d869e99) will increase coverage by 0.09%.
Report is 15 commits behind head on develop.
The diff coverage is 87.71%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3345      +/-   ##
===========================================
+ Coverage    29.25%   29.35%   +0.09%     
===========================================
  Files          496      497       +1     
  Lines        50512    50579      +67     
===========================================
+ Hits         14779    14845      +66     
- Misses       35733    35734       +1     
Files Changed Coverage Δ
src/biokbase/narrative/handlers/authhandlers.py 0.00% <0.00%> (ø)
...rc/biokbase/narrative/handlers/narrativehandler.py 0.00% <0.00%> (ø)
src/biokbase/narrative/system.py 94.59% <75.00%> (ø)
src/biokbase/auth.py 100.00% <100.00%> (ø)
src/biokbase/narrative/common/util.py 71.42% <100.00%> (+2.07%) ⬆️
src/biokbase/narrative/jobs/appmanager.py 91.94% <100.00%> (ø)

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2688da...1ef8ecd. Read the comment docs.

ialarmedalien
ialarmedalien previously approved these changes Aug 11, 2023
@ialarmedalien ialarmedalien self-requested a review August 11, 2023 15:33
@ialarmedalien ialarmedalien dismissed their stale review August 11, 2023 15:34

PR still in progress

Base automatically changed from update-ga4-user-2 to develop August 24, 2023 14:49
@ialarmedalien
Copy link
Collaborator

@briehl I think you just need to merge in the current develop branch

@sonarcloud
Copy link

sonarcloud bot commented Aug 25, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@briehl briehl merged commit 0cc1432 into develop Aug 28, 2023
8 of 10 checks passed
@briehl briehl deleted the update-ga4-user-3 branch August 28, 2023 15:25
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.

2 participants