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.

Sign up for GitHub

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

Jump to bottom

fix: improve downgrade flow in onboarding #22436

Merged
merged 41 commits into from
Jul 24, 2024
Merged

fix: improve downgrade flow in onboarding #22436

merged 41 commits into from
Jul 24, 2024

Conversation

raquelmsmith
Copy link
Member

@raquelmsmith raquelmsmith commented May 22, 2024 •
edited
Loading

Problem

If someone upgrades in onboarding, then decides to downgrade, they get an error.

Changes

  • Fixes the downgrade link so that it doesn't point to an activation URL.
  • Handles downgrade action in the button
  • Adds loading states for both upgrade and downgrade
Screen.Recording.2024-05-22.at.11.36.53.AM.mov

After downgrade it flickers saying "success" again for a split second... I don't think that many people will use this so I'm not too concerned about it.

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

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

Not available on self-hosted

How did you test this code?

👀 🖱️

@raquelmsmith raquelmsmith requested a review from a team May 22, 2024 18:39
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 22, 2024 •
edited
Loading

Size Change: +393 B (+0.04%)

Total Size: 1.07 MB

ℹ️ View Unchanged
Filename Size Change
frontend/dist/toolbar.js 1.07 MB +393 B (+0.04%)

compressed-size-action

zlwaterfield
zlwaterfield approved these changes May 22, 2024
View reviewed changes
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.

just a few questions but looks good

frontend/src/scenes/billing/billingProductLogic.ts Outdated Show resolved Hide resolved
frontend/src/scenes/billing/billingProductLogic.ts Show resolved Hide resolved
@raquelmsmith raquelmsmith enabled auto-merge (squash) May 22, 2024 20:17
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

3 snapshot changes in total. 0 added, 3 modified, 0 deleted:

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@raquelmsmith raquelmsmith disabled auto-merge June 3, 2024 16:42
@posthog-bot
Copy link
Contributor

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

@raquelmsmith raquelmsmith removed the stale label Jun 6, 2024
@posthog-bot
Copy link
Contributor

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

@posthog-bot
Copy link
Contributor

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

@raquelmsmith
Copy link
Member Author

I'm going to pick this back up after subscribe-to-all-products is out

@posthog-bot
Copy link
Contributor

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

zlwaterfield
zlwaterfield reviewed Jul 24, 2024
View reviewed changes
frontend/__snapshots__/scenes-app-dashboards--insight-legend--light.png Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

this is flakey, pr didn't touch anything related

raquelmsmith
raquelmsmith commented Jul 24, 2024
View reviewed changes
Copy link
Member Author

@raquelmsmith raquelmsmith 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, works good! ✅

@raquelmsmith
Copy link
Member Author

Cypress test failure looks legit https://github.com/PostHog/posthog/actions/runs/10072575554/job/27844845724?pr=22436

@zlwaterfield
Copy link
Contributor

Cypress test failure looks legit PostHog/posthog/actions/runs/10072575554/job/27844845724?pr=22436

will look!

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

3 snapshot changes in total. 0 added, 3 modified, 0 deleted:

  • chromium: 0 added, 3 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

3 snapshot changes in total. 0 added, 3 modified, 0 deleted:

  • chromium: 0 added, 3 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

3 snapshot changes in total. 0 added, 3 modified, 0 deleted:

  • chromium: 0 added, 3 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@raquelmsmith raquelmsmith merged commit cbf16e8 into master Jul 24, 2024
92 checks passed
@raquelmsmith raquelmsmith deleted the fix/downgrade-button-action branch July 24, 2024 22:19
zlwaterfield added a commit that referenced this pull request Jul 25, 2024
@raquelmsmith @github-actions @zlwaterfield
fix: improve downgrade flow in onboarding (#22436)
e1bbd89 
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Zach Waterfield <[email protected]>
silentninja pushed a commit to silentninja/posthog that referenced this pull request Aug 8, 2024
@raquelmsmith @github-actions
@zlwaterfield @silentninja
fix: improve downgrade flow in onboarding (PostHog#22436)
3789f81 
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Zach Waterfield <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zlwaterfield zlwaterfield zlwaterfield approved these changes

@xrdt xrdt xrdt approved these changes

Assignees

@zlwaterfield zlwaterfield

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@raquelmsmith @posthog-bot @zlwaterfield @xrdt