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

feat: adding capture events for created and deleting org domains under authentication domains and sso #26654

Merged
merged 3 commits into from
Dec 4, 2024

Conversation

surbhi-posthog
Copy link
Contributor

@surbhi-posthog surbhi-posthog commented Dec 4, 2024

Problem

We need capture events for add and deleting domains under Authentication Domain and SSO

Changes

No UX changes, just added logs in the backend

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

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

Cloud: Yes

How did you test this code?

Tested locally on: http://localhost:8000/project/1/settings/project

Added unit test and verified that they pass:


posthog/api/test/test_organization_domain.py ......................                                                    [100%]
====================================================== inline snapshot =======================================================

============================== 22 passed in 2.44s ==============================

Verified capture events showed up under the activity tab
image

@surbhi-posthog surbhi-posthog marked this pull request as ready for review December 4, 2024 20:00
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.

Awesome, this looks good. And for this one your should be able to test it here: https://github.com/PostHog/posthog/blob/master/posthog/api/test/test_organization_domain.py#L87. Basically adding the mock (like the example I sent you). You can test locally with pytest

@surbhi-posthog
Copy link
Contributor Author

Awesome, this looks good. And for this one your should be able to test it here: https://github.com/PostHog/posthog/blob/master/posthog/api/test/test_organization_domain.py#L87. Basically adding the mock (like the example I sent you). You can test locally with pytest

Yes of course, I just updated the tests

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.

Awesome, looks great!

@zlwaterfield
Copy link
Contributor

Good to merge once tests pass.

@surbhi-posthog surbhi-posthog merged commit 407947f into master Dec 4, 2024
89 checks passed
@surbhi-posthog surbhi-posthog deleted the domain_capture branch December 4, 2024 21:48
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