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

fix(data-warehouse): Use distinct_id field if present #26635

Merged
merged 4 commits into from
Dec 4, 2024

Conversation

lcswillems
Copy link
Contributor

@lcswillems lcswillems commented Dec 4, 2024

Problem

I have a distinct_id field in my data warehouse table but this field is not used in order to prefill "Distinct ID field".

image

"Distinct ID field" is prefilled with "id" field. However, "id" doesn't corrspond to a user unique identifier, it corresponds to a row unique identifier, and so I really don't want "id" to be used for "Distinct ID field".

So each time I need to change "Distinct ID field" to use the field "distinct_id" of my table. This is very cumbersome and error prone. I have maybe 20 different insights where I need to change each time the "Distinct ID field".

Closes #26631

Changes

Use "distinct_id" field as a default for "Distinct ID field". And if it doesn't exist, falls back to "id".

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

Yes

How did you test this code?

I didn't test.

@lcswillems
Copy link
Contributor Author

lcswillems commented Dec 4, 2024

@Twixes I've tried to be way more precise in the issue and description of the PR. Is there anything more I should do?

Also, do you have an idea how fast this PR could be reviewed? This is very cumbersome and error-prone, and the PR should be very straightforward.

@Gilbert09 Gilbert09 changed the title Use distinct_id field if present fix(data-warehouse): Use distinct_id field if present Dec 4, 2024
@Gilbert09
Copy link
Member

@Twixes I've tried to be way more precise in the issue and description of the PR. Is there anything more I should do?

Also, do you have an idea how fast this PR could be reviewed? This is very cumbersome and error-prone, and the PR should be very straightforward.

Thanks for adding this in! I've approved the CI checks, once they all pass I'll get this merged in

@lcswillems
Copy link
Contributor Author

@Gilbert09 Thanks for the review! I've fixed the formatting. However, there is a Cypress test failing but I've no clue how to reproduce it locally and the CI only runs when you trigger it. How could I help fixing the last failing job?

@Gilbert09
Copy link
Member

@Gilbert09 Thanks for the review! I've fixed the formatting. However, there is a Cypress test failing but I've no clue how to reproduce it locally and the CI only runs when you trigger it. How could I help fixing the last failing job?

Thanks for fixing it. I'll keep an eye on the cypress tests - 99% of the time the failures are just flakes, and so I'll just restart them

@lcswillems
Copy link
Contributor Author

@Gilbert09 It seems that all the checks have passed! 🥳

@Gilbert09 Gilbert09 merged commit f0d9c49 into PostHog:master Dec 4, 2024
95 checks passed
@lcswillems lcswillems deleted the patch-1 branch December 4, 2024 16:01
@lcswillems
Copy link
Contributor Author

@Gilbert09 Thank you very much for the fast review and the merge! Do you have an idea when this fix should go in production?

@Gilbert09
Copy link
Member

@Gilbert09 Thank you very much for the fast review and the merge! Do you have an idea when this fix should go in production?

@lcswillems It went live about 10 mins ago 🥳

@lcswillems
Copy link
Contributor Author

@Gilbert09 Indeed! ❤️ Thank you so much for the responsiveness!!

image

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.

"Distinct ID field" prefilled with "id" field
2 participants