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

Make /decide endpoint more flexible (pt. 2) #1592

Merged
merged 6 commits into from
Sep 8, 2020

Conversation

yakkomajuri
Copy link
Contributor

@yakkomajuri yakkomajuri commented Sep 7, 2020

Changes

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

#1553 keeps giving a cryptic error on Django tests that me and @Twixes cannot reproduce locally. Just trying the tests with some changes on a separate branch.

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)

@yakkomajuri yakkomajuri marked this pull request as draft September 7, 2020 14:18
@yakkomajuri yakkomajuri changed the title Made #1583's changes in a separate branch Made #1553's changes in a separate branch Sep 7, 2020
@timgl timgl temporarily deployed to posthog-improve-decide--kkjja0 September 7, 2020 14:21 Inactive
@timgl timgl temporarily deployed to posthog-improve-decide--kkjja0 September 7, 2020 14:44 Inactive
@timgl timgl temporarily deployed to posthog-improve-decide--kkjja0 September 7, 2020 14:54 Inactive
@timgl timgl temporarily deployed to posthog-improve-decide--kkjja0 September 7, 2020 16:09 Inactive
@timgl timgl temporarily deployed to posthog-improve-decide--kkjja0 September 7, 2020 17:32 Inactive
@yakkomajuri
Copy link
Contributor Author

@Twixes so this approach now works - Not sure why the other PR was giving Redis errors but the fix for the main issue was preventing us from trying to read request.body twice, which Django doesn't let you do, so I unpacked it and passed that to find_key.

Cypress tests are naturally broken and code quality (backend) I'm working on

@yakkomajuri
Copy link
Contributor Author

/capture with personal api key is being tested in the capture tests and /decide with personal api key is being tested on decide tests, so no more tests for those within the actual personal api key tests

@timgl timgl temporarily deployed to posthog-improve-decide--kkjja0 September 7, 2020 17:45 Inactive
@yakkomajuri yakkomajuri changed the title Made #1553's changes in a separate branch Make /decide endpoint more flexible (pt. 2) Sep 7, 2020
@yakkomajuri yakkomajuri marked this pull request as ready for review September 7, 2020 17:49
@Twixes Twixes temporarily deployed to posthog-improve-decide--kkjja0 September 8, 2020 04:01 Inactive
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

3 participants