User V2 Part II - Frontend changes #3866

paolodamico · 2021-04-02T18:12:25Z

Changes

This PR builds up on #3855 & https://github.com/PostHog/posthog-cloud/pull/100, both requiring before merging this one.

PR updates relevant types & endpoints following User V2 API endpoint #3855 to rely on /_preflight & /api/users/@me/ for render-blocking components.
Refactors across components to use the appropriate teamLogic or organizationLogic when updating attributes of the team/project or organization, respectively, instead of relying on user endpoint for everything.
Fixes a bunch of typescript errors & migrates some minor settings-related components to Typescript (subsequent PR coming updating the errors blacklist).
Refactors all billing components to use the /api/billing/ endpoint instead of relying on user.
Starts using the user's uuid as identifier instead of relying on the integer primary key.
Improves the experience when updating the current user or team to show a clear and consistent success message.

Checklist

All querysets/queries filter by Organization, by Team, and by User
Django backend tests
Jest frontend tests
Cypress end-to-end tests

mariusandra

So again there are so many changes in this PR that it's impossible to fully verify everything without just testing everything :). However things seem to work! I found two things to remark on though (and 2 nits)

The settings page is pretty jumpy now:
When you open /insights (type in the url, click enter), everything works fine. If you are on any other page, reload, and then once the page loads click "Insights", no graph shows up. That's because the currentTeamLoaded event is never fired.

frontend/src/scenes/project/Settings/index.tsx

cypress/integration/dashboard.js

kpthatsme · 2021-04-08T16:38:11Z

@paolodamico I pulled the code and ran through most of the analytics functionality + dashboards (that can be reasonably done in a demo env). Didn't notice any major issues. One thing I noticed is it looks like empty state behavior has slightly changed?

See the two attached screenshots – left is localhost (running your branch), right is app.posthog – both in the demo project

.

Noticed that 1) the events weren't expanded by default on the left 2) no chart/ empty state

EDsCODE

This PR is dangerously large where there could be several missing variable changes that will cause errors. I think a different approach could've been taken where we create the new logic and then change all the references for that single logic as a concise PR that's focused. This one is proposing the changes for userlogic, billinglogic, and teamlogic which require a bunch more reference changes.

I did a run through of the code changes and did a QA clicking through most of the app and things look okay. Fairly overwhelming to go through closely. Will have to keep a close watch in prod

frontend/src/lib/components/PropertyFilters/PropertyFilters.js

paolodamico · 2021-04-08T18:20:41Z

Thanks @mariusandra and @kpthatsme for the review, all feedback has been addressed!

paolodamico · 2021-04-08T18:22:19Z

Agreed @EDsCODE! Ended up being a massive PR and a different approach would've been better, I failed to consider the extent of the changes required when first working on this (it kind of kept growing as more things were fixed). Will merge and be very attentive to any potential things breaking down.

mariusandra

I guess with so many changes, the change of some bug slipping in is always there. However things seem to work, so I think the best thing to do is to get this in cloud for a few days before it ends up in the next release.

paolodamico · 2021-04-08T19:44:23Z

100% agreed @mariusandra! I'm just making some final tweaks to the trends tests and merging. Matching cloud PR should already be live.

fuziontech

Twixes · 2021-04-08T19:52:07Z

frontend/src/lib/utils/eventUsageLogic.ts

@@ -104,7 +104,7 @@ export const eventUsageLogic = kea<
                    content_length: annotation.content.length,
                    scope: annotation.scope,
                    deleted: annotation.deleted,
-                    created_by_me: annotation.created_by && annotation.created_by?.id === userLogic.values.user?.id,
+                    created_by_me: annotation.created_by && annotation.created_by?.uuid === userLogic.values.user?.id,


This doesn't look right, right side should use user?.uuid I think?

Hmmm don't think so. The new /users/@me endpoint sends the uuid as id.

Okay, so as in the other comment, I think the spirit of this swap is good, but it's confusing as a PostHog developer

Twixes · 2021-04-08T20:12:07Z

frontend/src/types.ts

 export interface UserType {
-    anonymize_data: boolean
-    distinct_id: string
+    id: string


This needs uuid too I think?

Same as above, we actually send the uuid as id and no longer send the internal PK in the payload.

UserBasicType has both id and uuid though?
Generally I think if the DB fields are not renamed but in API they are, it's kind of confusing, though I agree with the sentiment of sending only uuid

Makes sense, this could lead to confusion. As this requires a backend change too, will merge this and then update to have the attribute named uuid and avoid confusion.

…o 3765-cohort-by-trend * '3765-cohort-by-trend' of github.com:PostHog/posthog: (39 commits) 'string, parsable as datetime' (#3942) Update plugin server to 0.16.3 (#3944) Resizable table columns in Sessions (#3927) bump cryptography==3.4.7 and add macosx_arm64 install script (#3935) 🤖: Add jeduden as a contributor 🎉 (#3938) Fix feature flags default rollout (#3745) Less dancing in dashboards (#3824) Always show event stats and add warnings (#3908) Update plugin server to 0.16.2 (#3932) Minimum PostHog version in plugins (#3916) Renames Active users to Unique users (#3930) Fix action with same name (#3909) Fix navigation to insights from dashboards (#3928) User V2 Part II - Frontend changes (#3866) update autocapture label to be more descriptive (#3925) Log to sentry when migrations are out of date (#3924) Revert "Increase Element model varchar limits (#3912)" (#3923) Update plugin server to 0.16.1 (#3920) Are migrations safe to run on cloud? (#3917) Run Automerge as posthog-bot (#3919) ...

paolodamico added 23 commits April 1, 2021 09:14

initial refactor

804a63b

simple serializer for teams

3abd40e

basic team in front-end

2e22893

use basicTeam where applicable

4f34abd

refactor UserSerializer to UserBasicSerializer

736adee

abstract UserBasicSerializer

6f8574b

add uuid to users

d081f25

move event properties methods to Team model

5c52fa9

basic V2 user endpoint

fecca05

update current user

9b7fbaf

update current team and org

8225cf4

Merge branch 'master' into user-endpoint-refactor

f4d298a

post-merge fix

4e2d383

fix load logic

4cd5fcf

change password

da57880

add some extra tests

4f1bff8

undo all frontend changes from this PR

391c58b

Merge branch 'master' into user-endpoint-refactor

50dd7fc

remove last frontend change

e040a51

update preflight tests

d024ea9

remove team basic fetch

29c7fe7

fix foss tests

66d976e

Merge branch 'master' into user-endpoint-refactor

10ead90

timgl temporarily deployed to posthog-pr-3866 April 2, 2021 18:23 Inactive

import all frontend changes

64bf932

paolodamico force-pushed the user-endpoint-refactor-II branch from 3def701 to 64bf932 Compare April 2, 2021 18:26

timgl temporarily deployed to posthog-pr-3866 April 2, 2021 18:27 Inactive

paolodamico added 3 commits April 2, 2021 18:13

adjust migration & add distinct ID to payload

d0ec0ae

Merge branch 'user-endpoint-refactor' into user-endpoint-refactor-II

b6829cc

update types and refactor userLogic

0a5980d

mariusandra reviewed Apr 8, 2021

View reviewed changes

frontend/src/scenes/project/Settings/index.tsx Outdated Show resolved Hide resolved

cypress/integration/dashboard.js Show resolved Hide resolved

Merge branch 'master' into user-endpoint-refactor-II

19437a7

paolodamico added 3 commits April 8, 2021 10:08

fix default params on insights

68b34cf

fix jumpiness on settings page

1a576ea

add test for loading graphs from a different entry point

b4286cb

timgl temporarily deployed to posthog-pr-3866 April 8, 2021 17:25 Inactive

EDsCODE approved these changes Apr 8, 2021

View reviewed changes

frontend/src/lib/components/PropertyFilters/PropertyFilters.js Outdated Show resolved Hide resolved

paolodamico added 2 commits April 8, 2021 11:18

minor fix

7d43295

Merge branch 'master' into user-endpoint-refactor-II

6a0dce2

timgl temporarily deployed to posthog-pr-3866 April 8, 2021 18:20 Inactive

add timeout to wait for graph

eae64bc

timgl temporarily deployed to posthog-pr-3866 April 8, 2021 18:45 Inactive

mariusandra approved these changes Apr 8, 2021

View reviewed changes

fuziontech approved these changes Apr 8, 2021

View reviewed changes

adjust trends tests & improve propertyfilters callback

c9e43e3

timgl temporarily deployed to posthog-pr-3866 April 8, 2021 19:49 Inactive

paolodamico removed the request for review from Twixes April 8, 2021 19:51

Twixes reviewed Apr 8, 2021

View reviewed changes

paolodamico merged commit ae6b8ee into master Apr 8, 2021

paolodamico deleted the user-endpoint-refactor-II branch April 8, 2021 20:40

This was referenced Apr 8, 2021

Fix navigation to insights from dashboards #3928

Merged

User ID convention naming #3943

Merged

Remove fixed typescript errors #3956

Merged

mariusandra mentioned this pull request Apr 14, 2021

In projects with no events yet, there's awkwardly nothing at all in place of the Insights graph #3981

Closed

2 tasks

yakkomajuri mentioned this pull request Apr 27, 2021

fix toolbar links #4127

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

User V2 Part II - Frontend changes #3866

User V2 Part II - Frontend changes #3866

paolodamico commented Apr 2, 2021 •

edited

Loading

mariusandra left a comment

kpthatsme commented Apr 8, 2021

EDsCODE left a comment

paolodamico commented Apr 8, 2021

paolodamico commented Apr 8, 2021

mariusandra left a comment

paolodamico commented Apr 8, 2021

fuziontech left a comment

Twixes Apr 8, 2021

paolodamico Apr 8, 2021

Twixes Apr 8, 2021

Twixes Apr 8, 2021

paolodamico Apr 8, 2021

Twixes Apr 8, 2021 •

edited

Loading

paolodamico Apr 8, 2021

User V2 Part II - Frontend changes #3866

User V2 Part II - Frontend changes #3866

Conversation

paolodamico commented Apr 2, 2021 • edited Loading

Changes

Checklist

mariusandra left a comment

Choose a reason for hiding this comment

kpthatsme commented Apr 8, 2021

EDsCODE left a comment

Choose a reason for hiding this comment

paolodamico commented Apr 8, 2021

paolodamico commented Apr 8, 2021

mariusandra left a comment

Choose a reason for hiding this comment

paolodamico commented Apr 8, 2021

fuziontech left a comment

Choose a reason for hiding this comment

Twixes Apr 8, 2021

Choose a reason for hiding this comment

paolodamico Apr 8, 2021

Choose a reason for hiding this comment

Twixes Apr 8, 2021

Choose a reason for hiding this comment

Twixes Apr 8, 2021

Choose a reason for hiding this comment

paolodamico Apr 8, 2021

Choose a reason for hiding this comment

Twixes Apr 8, 2021 • edited Loading

Choose a reason for hiding this comment

paolodamico Apr 8, 2021

Choose a reason for hiding this comment

paolodamico commented Apr 2, 2021 •

edited

Loading

Twixes Apr 8, 2021 •

edited

Loading