Merge records instead of overwrite and updates to metric portfolio#137
Merge records instead of overwrite and updates to metric portfolio#137
Conversation
…fined at instantiation
…metrics Essentially harmonize the approach to session-state-level and aggregate-level metrics recording
|
Updated original PR description to encompass a bug I discovered with session-level stats that was easy enough to fix in this fairly small PR as well. |
|
Hey @444B just checking in. Do you have everything you need to review? Would love to get this pushed through (if you don't see any issues ofc) so I can install in my parent project and reap the benefits. |
|
Hi @emigre459 ! |
…2 instances in firestore
|
Sorry to keep packing stuff in, but just realized that we need per_day widget states for our analytics use case so added those too. If it's too much in a single PR, no worries, can split it up. |
… vars already present for today
|
Hi
|
|
We actually discussed some of this earlier when I pushed individual session tracking, but for our use case we provide typical cookie accept/reject dialogue. Similarly a user management screen in the parent app can handle data deletion requests that could remove firestore records. I guess the question is: do you want SA2 to be the piece of the tech stack that manages that? We could probably build a data deletion function in SA2 if you like, with the caveat of course that it would still be up to individual devs to use it. |
…ssuming it's there)
I went ahead and just pushed a Do you think it makes sense to add a data privacy section to the README or as its own document or some such? We could provide some code snippets there for people to leverage to do things like a cookie banner for opt-in and pointing out the session data deletion functionality. |
|
@444B just checking to see if you think the data deletion option provided may satisfy your concerns? Happy to push a mod if you think there's a better way. |
Description
Update firestore.save() to a merge, instead of overwrite, action to preserve custom data elements (e.g. adding a username to track individual user session analytics for a registered user). Also discovered a bug with session-level tracking that was incorrectly resetting pageviews and total time passed that I was able to fix via refactor.
As it was quick to do, I also added a "session_time_seconds" metric that is recorded with the other daily metrics, so devs could see how long the typical session for a user at the day level (and thus aggregate it up to longer time periods as needed).
Related Issue(s)
Issue exists in forked repo but not parent.
Changes Made
merge=Truekwarg to the.set()operations infirestore.save()session_datainst.session_statebut use it as a quasi-global variable similar to what is done withdatato harmonize tracking approach.session_time_secondsto theper_daygroup in analytics records to expand analysisper_daylist and update both new per_day fields to be backwards compatible with existing firestore recordsTesting
Testing Details:
Tested
examples/minimal.pyby modifying thefirebase_test.pypage to use my google firebase credentials and project.streamlit-analyticscollection withcountsrecordcountsdocument ("username": "johnsmith")usernameentry did not disappear (which was the previous behavior)Screenshots (if applicable)
N/A
Checklist
Additional Notes
Had to run
uv pip install types-tomlto avoid failures fromrun_checks.shbut no changes were observed in git as a result.