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

[PP-1818] implement month active user data #2148

Merged
merged 12 commits into from
Nov 7, 2024

Conversation

dbernstein
Copy link
Contributor

Description

This update includes the following changes:

  1. The patron object now has a uuid field.
  2. Patrons can erase their activity history with a PUT to the /patrons/me/erase_activity_history endpoint.
  3. The uuid is passed to the s3 analytics provider.

Motivation and Context

https://ebce-lyrasis.atlassian.net/browse/PP-1818

How Has This Been Tested?

Checklist

  • I have updated the documentation accordingly.
  • All new and existing tests passed.

@dbernstein dbernstein changed the title Pp 1818 implement month active user data [PP-1818] implement month active user data Oct 31, 2024
@dbernstein dbernstein force-pushed the PP-1818-implement-month-active-user-data branch from dbf50ac to 939a5f3 Compare October 31, 2024 18:52
Copy link

codecov bot commented Oct 31, 2024

Codecov Report

Attention: Patch coverage is 97.14286% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.79%. Comparing base (91609a2) to head (5648441).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/palace/manager/api/routes.py 83.33% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2148   +/-   ##
=======================================
  Coverage   90.78%   90.79%           
=======================================
  Files         350      351    +1     
  Lines       40693    40718   +25     
  Branches     8822     8824    +2     
=======================================
+ Hits        36942    36968   +26     
  Misses       2441     2441           
+ Partials     1310     1309    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dbernstein dbernstein force-pushed the PP-1818-implement-month-active-user-data branch 4 times, most recently from 8146920 to 776d725 Compare October 31, 2024 20:35
@dbernstein dbernstein requested a review from a team November 1, 2024 17:17
src/palace/manager/api/routes.py Outdated Show resolved Hide resolved
src/palace/manager/api/s3_analytics_provider.py Outdated Show resolved Hide resolved
src/palace/manager/api/s3_analytics_provider.py Outdated Show resolved Hide resolved
src/palace/manager/api/s3_analytics_provider.py Outdated Show resolved Hide resolved
@dbernstein dbernstein force-pushed the PP-1818-implement-month-active-user-data branch 6 times, most recently from f85e074 to 44aefa9 Compare November 6, 2024 00:48
@tdilauro
Copy link
Contributor

tdilauro commented Nov 6, 2024

You may have considered this already, but while looking into PP-1863, I was reminded that we have a credential type of Credential.IDENTIFIER_TO_REMOTE_SERVICE that might be suitable for this purpose or as a template for associating an identifier with a patron, but without modifying the patron table.

Sorry to lob in a hand grenade 💣 at this stage. Feel free to do -- or not do ("There is no try?") -- what you will with this information.

@dbernstein dbernstein force-pushed the PP-1818-implement-month-active-user-data branch from 44aefa9 to 8d94a75 Compare November 6, 2024 20:51
@dbernstein
Copy link
Contributor Author

@tdilauro : I appreciate your suggestion. And I like it. I can't think of a good reason not to do it that way other than that I did it this way and it works and is done and I feel like I need to move on. But I suppose if we wanted to rework it in the future it would not be a heavy lift to do it.

Copy link
Member

@jonathangreen jonathangreen 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! 🎸

@dbernstein dbernstein merged commit 2e8316b into main Nov 7, 2024
21 checks passed
@dbernstein dbernstein deleted the PP-1818-implement-month-active-user-data branch November 7, 2024 19:47
@dbernstein dbernstein added feature New feature DB migration This PR contains a DB migration labels Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DB migration This PR contains a DB migration feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants