Skip to content

feat(propdefs): override team_id & project_id if inbound raw Event includes root_team_id #31032

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

eli-r-ph
Copy link
Contributor

@eli-r-ph eli-r-ph commented Apr 9, 2025

Problem

New environments handling has replaced project_id with a notion of parent ("root") team ID which controls the global property def type classifications and other settings for all child teams.

We need the property-defs-rs service to become root_team_id aware as part of the transition.
⚠️ DO NOT MERGE until environments prep work is completed ⚠️

Changes

  • Write path: ensure incoming Events override team_id and project_id when decomposed into Updates whenever Event#root_team_id is present
  • Read path: rely on team_id in the property defs API where project_id was needed

TODO: remove uses of COALESCE on project_id, falling back to team_id, relying on (overridden) team_id in all property-defs-rs SQL

Does this work well for both Cloud and self-hosted?

Yes

How did you test this code?

Locally and in CI; coordinated w/ @benjackwhite 's ongoing environments work

@eli-r-ph eli-r-ph requested review from benjackwhite, zlwaterfield and a team April 9, 2025 21:18
@eli-r-ph eli-r-ph self-assigned this Apr 9, 2025
@eli-r-ph eli-r-ph marked this pull request as ready for review April 9, 2025 21:32
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Refactored the property definitions service to account for the new root_team_id concept by overriding team_id and project_id when root_team_id is present. The changes update both the read and write paths across query construction, routing logic, and type conversion.

  • Updated rust/property-defs-rs/src/api/v1/query.rs to use team_id in place of project_id, ensuring consistency in query binding.
  • Modified rust/property-defs-rs/src/api/v1/routing.rs to extract and route using team_id rather than project_id.
  • Enhanced rust/property-defs-rs/src/types.rs by adding an optional root_team_id and adjusting conversion logic for event updates.
  • Noted that SQL still uses COALESCE on project_id; this will be addressed in future updates.

3 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

@zlwaterfield zlwaterfield left a comment

Choose a reason for hiding this comment

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

I don't know rust well but from what I gather this looks good.

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