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

Updates Action without ignoring created_by #1448

Merged
merged 4 commits into from
Aug 18, 2020
Merged

Updates Action without ignoring created_by #1448

merged 4 commits into from
Aug 18, 2020

Conversation

Algogator
Copy link
Contributor

Changes

Please describe.
If this affects the front-end, include screenshots.

Checklist

  • All querysets/queries filter by Team (if this PR affects any querysets/queries)
  • Backend tests (if this PR affects the backend)
  • Cypress E2E tests (if this PR affects the front and/or backend)

@@ -91,6 +91,17 @@ class Meta:
"created_by",
]

def update(self, instance, validated_data):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm interesting, though I think this will end up creating a lot of ongoing work for us, as each time we add a field we have to remember to add it here too.

Would it be easier to just delete created_by on this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have a link to the line you sent

but a del request.data['created_by'] somewhere before the update in https://github.com/PostHog/posthog/blob/master/posthog/api/action.py#L167 works as well

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep that sounds good!

Copy link
Collaborator

@timgl timgl left a comment

Choose a reason for hiding this comment

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

Looks good! will merge once tests pass

Check for created_by in data before deleting it
@timgl timgl merged commit edca315 into PostHog:master Aug 18, 2020
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.

None yet

2 participants