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

Expose Hosting Trial: Allow starting free trial from themes upload page #86878

Merged
merged 7 commits into from
Jan 29, 2024

Conversation

vykes-mac
Copy link
Contributor

@vykes-mac vykes-mac commented Jan 25, 2024

Related to https://github.com/Automattic/dotcom-forge/issues/5097

Proposed Changes

This PR allow the user to start a free trial from the themes upload page. There is an issue where we gets redirected to crowdsignal after the atomic transfer. what should happen is that we should be redirected to /wp-admin/theme-install.php

  • Show free trial banner when user is eligible
  • Shows the trial acknowledge modal and start the trial

Testing Instructions

  • go to /themes/upload/simple_site
  • Start your free trial and verify the site transitions to atomic successfully

image

Pre-merge Checklist

  • Has the general commit checklist been followed? (PCYsg-hS-p2)
  • https://wpcalypso.wordpress.com/devdocs/docs/testing/index.md for your changes?
  • Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
  • Have you checked for TypeScript, React or other console errors?
  • Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
  • Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
  • For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-ajp-p2)?

Copy link

github-actions bot commented Jan 25, 2024

@vykes-mac vykes-mac self-assigned this Jan 26, 2024
@vykes-mac vykes-mac added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. Expose Hosting Trial labels Jan 26, 2024
@zaguiini
Copy link
Contributor

zaguiini commented Jan 26, 2024

what should happen is that we should be redirected to /wp-admin/theme-install.php

Should we? What's wrong with keeping the user in that same page? They want to upload a theme, and they are in the right place in the sense that all they need to do is to upload the ZIP

After the transfer, it opened a ton of tabs:

image

@vykes-mac
Copy link
Contributor Author

vykes-mac commented Jan 27, 2024

Should we? What's wrong with keeping the user in that same page? They want to upload a theme, and they are in the right place in the sense that all they need to do is to upload the ZIP

Yeah it should redirect to wp-admin, it's a design decision that was made because the calypso upload doesn't allow updating a plugin so they are forcing the user to use the wp-admin one instead. See conversation p1706055415216519-slack-C029FM1EH, If you go to /themes/upload on an AT site it will redirect you.

@vykes-mac
Copy link
Contributor Author

Should we? What's wrong with keeping the user in that same page? They want to upload a theme, and they are in the right place in the sense that all they need to do is to upload the ZIP

Let's no redirect If we start the trial on this page. There's a bunch of redirect happening as it seems the atomic platform is not fully setup so /theme-install.php returns 302 which intern redirect to another location and it continues until a valid /wp-admin/ url is returned.

Copy link
Contributor

@paulopmt1 paulopmt1 left a comment

Choose a reason for hiding this comment

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

The trial started successfully! After my site became Atomic, I still saw this screen and can't click on that area or drop zip files, it seems it is still disabled:

image

After I refreshed it, it redirected me to the native theme page. Is that intentional?

image

@vykes-mac
Copy link
Contributor Author

By design from previous comment it should redirect to wp-admin theme if I refresh. You should be able to upload a theme though without refreshing the page, I think what happened in this case the request site runs before the site is fully atomic on the backend so the isAtomic flag is still false and so the drop zone is disabled. I think I need to keep running requestSite until it returns isAtomic is true. I wait for 2 seconds before requesting site but sometimes it still seem to be too early

Comment on lines 206 to 207
isUserEligibleForFreeHostingTrial( state ) &&
site.plan.is_free &&
config.isEnabled( 'hosting-trial' );
isUserEligibleForFreeHostingTrial( state ) && site && site.plan?.is_free;
config.isEnabled( 'hosting-trial' );
Copy link
Contributor

Choose a reason for hiding this comment

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

What's happening here? The trial check went over to a new line :) Can we also replace the site check with site?.plan?.is_free?

Copy link
Contributor

@zaguiini zaguiini left a comment

Choose a reason for hiding this comment

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

Works well!

Copy link
Contributor

@paulopmt1 paulopmt1 left a comment

Choose a reason for hiding this comment

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

It's working as expected, nice work @vykes-mac! I was able to start my trial, submit a theme file and activate it. All at the same flow!

@paulopmt1 paulopmt1 merged commit d45166d into trunk Jan 29, 2024
11 checks passed
@paulopmt1 paulopmt1 deleted the update/theme-upload-trial-cta branch January 29, 2024 22:07
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jan 29, 2024
@a8ci18n
Copy link

a8ci18n commented Jan 29, 2024

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/11099086

Some locales (Hebrew, Japanese) have been temporarily machine-translated due to translator availability. All other translations are usually ready within a few days. Untranslated and machine-translated strings will be sent for translation next Monday and are expected to be completed by the following Friday.

Thank you @vykes-mac for including a screenshot in the description! This is really helpful for our translators.

@matticbot
Copy link
Contributor

matticbot commented Jan 31, 2024

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~96 bytes added 📈 [gzipped])

name                   parsed_size           gzip_size
entry-main                 -1950 B  (-0.1%)     -424 B  (-0.1%)
entry-stepper              -1720 B  (-0.1%)     +174 B  (+0.0%)
entry-subscriptions        -1272 B  (-0.1%)     -346 B  (-0.1%)
entry-login                -1272 B  (-0.1%)     -346 B  (-0.1%)
entry-domains-landing      -1272 B  (-0.2%)     -346 B  (-0.2%)
entry-browsehappy          -1272 B  (-0.9%)     -346 B  (-0.9%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~16915 bytes added 📈 [gzipped])

name                             parsed_size           gzip_size
themes                              +86760 B  (+8.7%)   +20097 B  (+6.6%)
plugins                               +942 B  (+0.0%)    -2137 B  (-0.3%)
jetpack-cloud-plugin-management       +942 B  (+0.1%)    -2137 B  (-0.5%)
hosting                               +655 B  (+0.1%)     -167 B  (-0.1%)
signup                                -532 B  (-0.2%)      -63 B  (-0.1%)
link-in-bio-tld-flow                  -532 B  (-0.0%)      -63 B  (-0.0%)
update-design-flow                    -514 B  (-0.0%)      -55 B  (-0.0%)
devdocs                               -462 B  (-0.2%)     -123 B  (-0.2%)
checkout                              +206 B  (+0.0%)     +280 B  (+0.1%)
migrate                               +196 B  (+0.1%)     +835 B  (+1.0%)
import-hosted-site-flow               +196 B  (+0.0%)     +935 B  (+0.4%)
domains                               +103 B  (+0.0%)      +49 B  (+0.0%)
import-flow                            -85 B  (-0.0%)      +63 B  (+0.0%)
theme                                  -68 B  (-0.0%)     -178 B  (-0.1%)
with-theme-assembler-flow              +18 B  (+0.0%)      +12 B  (+0.1%)
update-options-flow                    +18 B  (+0.1%)      +13 B  (+0.3%)
site-setup-flow                        +18 B  (+0.0%)      +13 B  (+0.1%)
free-post-setup-flow                   +18 B  (+0.1%)      +11 B  (+0.3%)
free-flow                              +18 B  (+0.0%)      +13 B  (+0.2%)
assembler-first-flow                   +18 B  (+0.0%)      +12 B  (+0.1%)
ai-assembler-flow                      +18 B  (+0.0%)      +11 B  (+0.1%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~13742 bytes removed 📉 [gzipped])

name                                     parsed_size           gzip_size
async-load-signup-steps-theme-selection       -226 B  (-0.1%)    -1487 B  (-1.2%)
async-load-design-blocks                      -226 B  (-0.0%)    -1347 B  (-0.2%)
async-load-design                             -122 B  (-0.0%)    -1607 B  (-0.3%)
async-load-design-playground                   +12 B  (+0.0%)     -788 B  (-0.2%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

@a8ci18n
Copy link

a8ci18n commented Feb 3, 2024

Translation for this Pull Request has now been finished.

jjchrisdiehl pushed a commit that referenced this pull request Feb 5, 2024
…ge (#86878)

* all user to start free trial from themes upload page

* do not redirect to wp-admin themes upload if a trial was requested

* fix issue with theme upload not show when adding free trial

* poll site request if transfer complete and site is not atomic

* Removed unnecessary config check and merged trunk

* Keeping plugin config check as same as trunk

---------

Co-authored-by: Paulo Trentin <paulo@paulotrentin.com.br>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants