-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Show install all plugins button on Manage Plugins and License key section #22615
base: 5.x-dev
Are you sure you want to change the base?
Conversation
…ense key section, #PG-3698
@@ -227,6 +229,32 @@ public function removePluginTrialRequest(string $pluginName): void | |||
} | |||
} | |||
|
|||
public function isInstallAllPaidPluginsVisible(): bool | |||
{ | |||
$consumer = StaticContainer::get('Piwik\Plugins\Marketplace\Consumer'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To test add below code
$consumer = StaticContainer::get('Piwik\Plugins\Marketplace\Consumer'); | |
return true; | |
$consumer = StaticContainer::get('Piwik\Plugins\Marketplace\Consumer'); |
|
||
public function getPaidPluginsToInstallAtOnce(): array | ||
{ | ||
$plugins = StaticContainer::get('Piwik\Plugins\Marketplace\Plugins'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To test also add this here
$plugins = StaticContainer::get('Piwik\Plugins\Marketplace\Plugins'); | |
return ['ActivityLog']; | |
$plugins = StaticContainer::get('Piwik\Plugins\Marketplace\Plugins'); |
If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'. |
@matomo-org/core-reviewers Can anyone check this ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments. Otherwise aren't there any UI tests we can adjust or add to have that covered somehow?
@@ -38,15 +44,23 @@ import { | |||
translate, | |||
MatomoUrl, | |||
} from 'CoreHome'; | |||
import InstallAllPaidPluginsButton from '../../../../Marketplace/vue/src/InstallAllPaidPluginsButton/InstallAllPaidPluginsButton.vue'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should only reference that relative, if it is within the same plugin. Here simply importing it from Marketplace
should also work.
$view->isInstallAllPaidPluginsVisible = $marketplace->isInstallAllPaidPluginsVisible(); | ||
$view->paidPluginsToInstallAtOnce = $marketplace->getPaidPluginsToInstallAtOnce(); | ||
$view->installAllPluginsNonce = Nonce::getNonce(\Piwik\Plugins\Marketplace\Controller::INSTALL_NONCE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if it might be smarter to change the InstallButton component to fetch all required information through a ajax request to a controller action (NOT api, due to the required token) instead of passing that through all controllers, views and vue components. Would be an additional request, but imho a lot simpler to reuse everywhere.
This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers |
Description:
Add code to show install all plugins button on Manage Plugins and License key section
Fixes: #PG-3698
Review