wp-rocket icon indicating copy to clipboard operation
wp-rocket copied to clipboard

Closes #5397 Update license renewal banners content

Open remyperona opened this issue 2 years ago • 6 comments

Description

Update the license renewal banners to display the accurate price for not grandfathered users, with no discount.

Fixes #5397

Type of change

  • [x] Enhancement (non-breaking change which improves an existing functionality)

Is the solution different from the one proposed during the grooming?

No

How Has This Been Tested?

  • [x] Automated tests

Checklist:

  • [x] My code follows the style guidelines of this project
  • [x] I have performed a self-review of my own code
  • [x] My changes generate no new warnings
  • [x] I have added tests that prove my fix is effective or that my feature works
  • [x] New and existing unit tests pass locally with my changes

remyperona avatar Sep 13 '22 17:09 remyperona

@Tabrisrp

  1. According to the spreadsheet the text that should be displayed in the expired banner is:

Your WP Rocket license is expired!

Your website could be much faster if it could take advantage of our new features and enhancements. 🚀

which is different from the one in the plugin: https://github.com/wp-media/wp-rocket/blob/94c76535083e13e707d732d0bd70e4b2b5b16ce7/inc/Engine/License/views/renewal-expired-banner.php#L18

Although the differences are small, I've confirmed with @Agathemed that we should be using the one in the spreadsheet.

  1. Also, when displaying the prices of the discounted licenses the number are rounded, e.g. 199.2 becomes 199.

This is happening after passing $this->get_discount_price() to umber_format_i18n() here:

https://github.com/wp-media/wp-rocket/blob/94c76535083e13e707d732d0bd70e4b2b5b16ce7/inc/Engine/License/Renewal.php#L119

Can you please look into these?

vmanthos avatar Sep 16 '22 12:09 vmanthos

@vmanthos Thank you I missed that, it should be good now

remyperona avatar Sep 16 '22 12:09 remyperona

@Tabrisrp Please find the test note/question so far below:

  • Discount % is displayed to grandfather while expired > 15 days => we shall display regular price in this case
  • Shouldn't this transient wpr_dashboard_seen expire based on expired since when not once it's set?
  • While testing non-grandfather Scenario, by adding return false here after this line: https://github.com/wp-media/wp-rocket/blob/94c76535083e13e707d732d0bd70e4b2b5b16ce7/inc/Engine/License/Renewal.php#L304 => The price displayed in the banner is as if the discount was applied while discount text not displayed (here no discount and regular price shall be there, not sure if we have API problem or we aren't simulating that type of user right, can you please check that at any duration?)

Mai-Saad avatar Sep 20 '22 07:09 Mai-Saad

Discount % is displayed to grandfather while expired > 15 days => we shall display regular price in this case

Should be fixed with the latest commit

Shouldn't this transient wpr_dashboard_seen expire based on expired since when not once it's set?

I don't really understand what you mean here. Do we have any issue with the current values?

While testing non-grandfather Scenario, by adding return false here after this line:

The price discount is calculated separately and doesn't use is_grandfathered() method, so you won't get the expected result doing that

remyperona avatar Sep 20 '22 15:09 remyperona

I don't really understand what you mean here. Do we have any issue with the current values?

Well, if we have persona 1 mentioned in the sheet (this persona shall see the bubble as long as popup change or settings page not accessed yet)

  • user open dashboard after 7 days from wpr expired => user saw the notification bubble and related transient set to 2 weeks
  • one week later, the popup msg will change, since that transient has not expired and the user already accessed the dashboard, the bubble won't be displayed till wpr_dashboard_seen expired, right? (I am fine with that as I barely notice the bubble anyway. But as per requirements, the bubble shall be displayed when expired > 15) @piotrbak @Tabrisrp what do you think?

Mai-Saad avatar Sep 22 '22 08:09 Mai-Saad

@Mai-Saad I think we're okay here. That's a small and edgy case.

piotrbak avatar Sep 22 '22 10:09 piotrbak