Skip to content

AAI-236 Unify schemas for user data and metadata#23

Merged
marius-mather merged 10 commits into
mainfrom
unify-metadata
Jun 5, 2025
Merged

AAI-236 Unify schemas for user data and metadata#23
marius-mather merged 10 commits into
mainfrom
unify-metadata

Conversation

@marius-mather

@marius-mather marius-mather commented Jun 4, 2025

Copy link
Copy Markdown
Collaborator

Description

AAI-236: unify/combine any schemas we have for user data sent and received from Auth0, so we don't end up with incompatible schemas defined in different places

Changes

  • New biocommons schema module with schemas for our core user data
  • Remove schemas that were defined under a specific app like Galaxy/BPA + combine them into shared schemas
  • Make sure we are validating user + app metadata where possible
  • Update usages + tests

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • I have added unit / integration tests that prove my fix is effective or that my feature works
  • I have run all tests locally and they pass
  • I have updated the documentation (if applicable)

How to Test Manually (if necessary)

Run uv run pytest

amandazhuyilan
amandazhuyilan previously approved these changes Jun 4, 2025

@amandazhuyilan amandazhuyilan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall looks great except for usual nik pik on docstrings - thanks for making this change!

I was wondering what is the rationale behind the renaming User class to SessionUser?

Comment thread schemas/biocommons.py Outdated
@marius-mather

Copy link
Copy Markdown
Collaborator Author

I was wondering what is the rationale behind the renaming User class to SessionUser?

We have multiple different "User" objects so wanted to try and clarify what this one represented. Maybe there's a better name than SessionUser but I think we need something more specific than just User.

Co-authored-by: Amanda Zhu <amandazhuyilan@gmail.com>
amandazhuyilan
amandazhuyilan previously approved these changes Jun 5, 2025

@amandazhuyilan amandazhuyilan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me! Lets ship it 🚢

@marius-mather

Copy link
Copy Markdown
Collaborator Author

Not quite ready to ship sorry! Needs a bit of testing against our actual Auth0 instance (found some errors where if app or user metadata weren't defined, the data would fail validation). I'll request approval when it's good to go

@marius-mather

Copy link
Copy Markdown
Collaborator Author

OK @amandazhuyilan should be good to go now. I'll do a test registration once it's deployed to double-check everything works as expected

@amandazhuyilan amandazhuyilan self-requested a review June 5, 2025 02:12

@amandazhuyilan amandazhuyilan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approve assuming double testing passed @marius-mather

@marius-mather marius-mather merged commit 25449a0 into main Jun 5, 2025
1 check passed
@amandazhuyilan amandazhuyilan deleted the unify-metadata branch June 19, 2026 03:21
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