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

Update 1-click trial process to go via Markeplace e-shop #22451

Merged
merged 7 commits into from
Aug 5, 2024
Merged

Conversation

michalkleiner
Copy link
Contributor

Description:

Ref DEV-18349.

The behaviour on narrower screens was tweaked in dev, that may need adjusting, though I'm not sure what other options are there.

Review

@@ -455,9 +459,15 @@
padding: 24px;
height: 90px;
border-top: 1px solid #aaa;
margin-top: 1px; // to prevent images overflowing the border
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated to the change, but I notices some of the floating images were overlapping the border, so this fixes it.

@michalkleiner michalkleiner added Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. c: Marketplace For issues that affect the Matomo Plugin Marketplace where you can download plugins. Better processes Indicates an issue is about improving how we work. Needs Review PRs that need a code review and removed Needs Review PRs that need a code review labels Jul 29, 2024
@michalkleiner michalkleiner force-pushed the dev-18349 branch 2 times, most recently from f59ee66 to ffbafe9 Compare July 31, 2024 13:52
@michalkleiner michalkleiner added the Needs Review PRs that need a code review label Jul 31, 2024
@michalkleiner michalkleiner requested a review from a team July 31, 2024 13:56
Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

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

Generally seems to work as expected. Left a minor translation related comment.

Besides that the UI tests for starting a trial are now failing. Those need to be removed and/or adjusted to cover the new flow.

And I came across one thing that seems weird. For a plugin, which is installed, but the license is missing the super user sees a license missing warning, which is correct. But he normal user has a request trial button, while seeing the no license warning in the details modal. It will most likely not be possible at all to start a new trial in that case, as if the plugin is already installed, there most likely had been a trial or purchase before, so maybe we should simply not show the trial button in that case.

image

plugins/Marketplace/lang/en.json Outdated Show resolved Hide resolved
@michalkleiner
Copy link
Contributor Author

michalkleiner commented Aug 1, 2024

And I came across one thing that seems weird. For a plugin, which is installed, but the license is missing the super user sees a license missing warning, which is correct. But he normal user has a request trial button, while seeing the no license warning in the details modal. It will most likely not be possible at all to start a new trial in that case, as if the plugin is already installed, there most likely had been a trial or purchase before, so maybe we should simply not show the trial button in that case.

@sgiehl what combination of user access level did you use to get the screenshot? I can't reproduce it. When the plugin is installed and license is removed, a super user gets the notification that there's no license, and a regular user gets "More details" (after a tiny change), but a regular user never sees the notification as it requires "hasSomeAdminAccess" to be true.

@michalkleiner
Copy link
Contributor Author

Besides that the UI tests for starting a trial are now failing. Those need to be removed and/or adjusted to cover the new flow.

@sgiehl removed the Start Free Trial UI tests for now.

@michalkleiner
Copy link
Contributor Author

And I came across one thing that seems weird. For a plugin, which is installed, but the license is missing the super user sees a license missing warning, which is correct. But he normal user has a request trial button, while seeing the no license warning in the details modal. It will most likely not be possible at all to start a new trial in that case, as if the plugin is already installed, there most likely had been a trial or purchase before, so maybe we should simply not show the trial button in that case.

@sgiehl I've confirmed this was happening for a user that had admin access to another site ("some admin access"). After a change they see 'More details' on the plugin card, no license notification in the modal, but no 'Request trial' within the modal.

@michalkleiner michalkleiner requested review from sgiehl and a team August 2, 2024 08:45
Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

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

The previously reported things all seem fixed.
But I came across something else, but as that most likely was already the case before, it might not need to be fixed here.

When running Matomo on PHP 7.2, without a license for premium plugins, as super user it shows the More Details button for LoginSaml. I guess the Start Trial isn't shown, as it requires PHP 7.3. But the modal shows this:

image

So it shows that it can't be installed, but the warning with the reason is missing.

@michalkleiner
Copy link
Contributor Author

I've checked how this behaved on 5.0.0 before the on-prem marketplace changes and the warning wasn't shown either for premium plugins. I'll create an internal ticket to follow that up.

@caddoo-innocraft caddoo-innocraft changed the base branch from 5.x-dev to 5.1.x-dev August 5, 2024 02:18
@caddoo caddoo changed the base branch from 5.1.x-dev to 5.x-dev August 5, 2024 02:38
@caddoo caddoo changed the base branch from 5.x-dev to 5.1.x-dev August 5, 2024 03:21
@sgiehl sgiehl added this to the 5.1.1 milestone Aug 5, 2024
@sgiehl sgiehl merged commit 03895a3 into 5.1.x-dev Aug 5, 2024
22 of 25 checks passed
@sgiehl sgiehl deleted the dev-18349 branch August 5, 2024 13:27
caddoo pushed a commit that referenced this pull request Aug 6, 2024
* Update 1-click trial process to go via Markeplace e-shop

* Update styling and behaviour of modal footer when free trial dropdown is present

* Update expected UI screenshots due to 1px shift

* Use original lead-in text to leverage existing translations

* Remove StartFreeTrial tests

* Prevent displaying Request trial for plugins that have a missing license

* Update UI test screenshot
sgiehl pushed a commit that referenced this pull request Aug 7, 2024
* Update 1-click trial process to go via Markeplace e-shop

* Update styling and behaviour of modal footer when free trial dropdown is present

* Update expected UI screenshots due to 1px shift

* Use original lead-in text to leverage existing translations

* Remove StartFreeTrial tests

* Prevent displaying Request trial for plugins that have a missing license

* Update UI test screenshot
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Better processes Indicates an issue is about improving how we work. c: Marketplace For issues that affect the Matomo Plugin Marketplace where you can download plugins. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants