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

Add event to alter delete website confirmation #22484

Merged
merged 22 commits into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
f0f1f4b
Adds event to alter delete website explanation, #PG-3574
AltamashShaikh Aug 7, 2024
0273a81
Merge branch '5.x-dev' into PG-3574-delete-website-message-event
AltamashShaikh Aug 7, 2024
5281d4d
Refactor code to getDeleteExplantionText only when deleteAction is tr…
AltamashShaikh Aug 7, 2024
c5927a2
Merge branch '5.x-dev' into PG-3574-delete-website-message-event
AltamashShaikh Aug 7, 2024
84ef82f
Ui test updated
AltamashShaikh Aug 7, 2024
a71dc60
Merged with 5.x-dev
AltamashShaikh Aug 8, 2024
986af11
Updated UI screenshot
AltamashShaikh Aug 8, 2024
6df4383
Merge branch '5.x-dev' into PG-3574-delete-website-message-event
AltamashShaikh Aug 12, 2024
2b4ebe7
Merged with 5.x-dev
AltamashShaikh Aug 20, 2024
0654b26
Updated UI screenshot
AltamashShaikh Aug 20, 2024
6bb6f7c
Merge branch '5.x-dev' into PG-3574-delete-website-message-event
AltamashShaikh Aug 22, 2024
2bbd371
Apply PR feedback
AltamashShaikh Aug 22, 2024
4d9d66e
Updated UI screenshot
AltamashShaikh Aug 22, 2024
64f7943
Apply PR feedbacks
AltamashShaikh Aug 29, 2024
1bb2940
Merge branch '5.x-dev' into PG-3574-delete-website-message-event
AltamashShaikh Aug 29, 2024
4a789d5
Merge branch '5.x-dev' into PG-3574-delete-website-message-event
AltamashShaikh Aug 30, 2024
aad1b5d
Fixes failing tests
AltamashShaikh Aug 30, 2024
b307e65
Merge branch 'PG-3574-delete-website-message-event' of github.com:mat…
AltamashShaikh Aug 30, 2024
2750757
Removes TagManager dependent test
AltamashShaikh Aug 30, 2024
f8cd678
Changes for even space
AltamashShaikh Sep 3, 2024
d4cd09a
Merge branch '5.x-dev' into PG-3574-delete-website-message-event
michalkleiner Sep 3, 2024
03f6cb2
Applied PR feedback
AltamashShaikh Sep 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions plugins/SitesManager/API.php
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,24 @@ public function getSitesWithAdminAccess($fetchAliasUrls = false, $pattern = fals
return $sites;
}

/**
* Returns the messages to warn users on site deletion.
*
* @param int $idSite
* @return array messages to warn users
* @throws Exception if the website ID doesn't exist or the user doesn't have super user access to it
* @internal
* @unsanitized
*/
public function getMessagesToWarnOnSiteRemoval(int $idSite): array
{
$messages = [];
Piwik::checkUserHasSuperUserAccess($idSite);
Piwik::postEvent('SitesManager.getMessagesToWarnOnSiteRemoval', [$idSite, &$messages]);
Copy link
Member

Choose a reason for hiding this comment

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

We should add a comment above the event to document what it is good for. Like it'S e.g. the case here:

/**
* Triggered before a task is executed.
*
* A plugin can listen to it and modify whether a specific task should be executed or not. This way
* you can force certain tasks to be executed more often or for example to be never executed.
*
* @param bool &$shouldExecuteTask Decides whether the task will be executed.
* @param Task $task The task that is about to be executed.
*/
Piwik::postEvent('ScheduledTasks.shouldExecuteTask', array(&$shouldExecuteTask, $task));

And when looking at other events it seems we use to pass the referenced parameter first, and then the other. So you may switch idSite and messages

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sgiehl Added as suggested 👍


return $messages;
}

/**
* Returns the list of websites with the 'view' access for the current user.
* For the superUser it doesn't return any result because the superUser has admin access on all the websites (use getSitesWithAtLeastViewAccess() instead).
Expand Down
19 changes: 19 additions & 0 deletions plugins/SitesManager/tests/Integration/ApiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,25 @@ public function testGetSitesWithAdminAccessShouldApplyPatternAndLimit()
$this->assertReturnedSitesContainsSiteIds([5, 15], $sites);
}

public function testGetMessagesToWarnOnSiteRemovalShouldReturnDefaultValue()
{
$pluginManager = Plugin\Manager::getInstance();
if ($pluginManager->isPluginActivated('TagManager')) {
$pluginManager->deactivatePlugin('TagManager');
}
API::getInstance()->addSite("site1", ["http://piwik.net", "http://piwik.com"]);
API::getInstance()->addSite("site2", ["http://piwik.com", "http://piwik.net"]);
$this->assertEmpty(API::getInstance()->getMessagesToWarnOnSiteRemoval(1));
$this->assertEmpty(API::getInstance()->getMessagesToWarnOnSiteRemoval(2));
}

public function testGetMessagesToWarnOnSiteRemovalShouldThrowExceptionIfNotSuperUser()
{
$this->expectException(\Exception::class);
$this->createManySitesWithAdminAccess(1);
API::getInstance()->getMessagesToWarnOnSiteRemoval(1);
}

private function createManySitesWithAdminAccess($numSites)
{
for ($i = 1; $i <= $numSites; $i++) {
Expand Down
Loading
Loading