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 toggling access control button #26705

Closed
wants to merge 8 commits into from

Conversation

surbhi-posthog
Copy link
Contributor

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

Problem

We want to capture logs when the user toggles on/off access control feature under teams.

Remaining features include:

  • SSO enforcement which is turned on after verifying a domain.
  • Project permissions - so if someone toggles project private/public
  • Ingestion taxonomy - if someone marks events as verified
  • White label branding - surveys and sharing dashboards

Changes

Changes only add logs in the backend, no frontend changes

👉 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?

Locally verified that the activity monitor showed the events and add tests
image

@surbhi-posthog surbhi-posthog requested a review from a team December 5, 2024 23:20
@surbhi-posthog surbhi-posthog marked this pull request as ready for review December 5, 2024 23:33
@surbhi-posthog
Copy link
Contributor Author

Adding a separate function to test toggle action

@surbhi-posthog
Copy link
Contributor Author

The tests I wrote for this event keeps failing due to mismatch event name. I can keep working on this, but Id like to merge the capture at least so we can start collecting logs on this action

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.

Can you remove the cache that's being added in these new files?

@zlwaterfield
Copy link
Contributor

The tests I wrote for this event keeps failing due to mismatch event name

Hmm, not sure what you mean. We should be able to test this fairly easily. Do you want to add the test back and I can help you debug it?

@surbhi-posthog
Copy link
Contributor Author

I've added the tests back and verified that they pass locally. Previously the tests were failing, so I want to wait for these to complete before moving forward.

@surbhi-posthog
Copy link
Contributor Author

creating a new branch since this got ugly: #27031

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