Skip to content

Mass delete templates in container version #522

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: 4.x-dev
Choose a base branch
from
Open

Mass delete templates in container version #522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f20375b
Mass delete templates in container version
ulcuber Jul 27, 2022
0bb9d7e
optimize import with mass delete
ulcuber Aug 4, 2022
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
60 changes: 44 additions & 16 deletions API/Import.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* @link https://matomo.org
* @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
*/

namespace Piwik\Plugins\TagManager\API;

use Piwik\API\Request;
Expand All @@ -16,6 +17,10 @@
use Piwik\Plugins\TagManager\Model\Trigger;
use Piwik\Plugins\TagManager\Model\Variable;
use Exception;
use Piwik\Date;
use Piwik\Plugins\TagManager\Dao\TagsDao;
use Piwik\Plugins\TagManager\Dao\TriggersDao;
use Piwik\Plugins\TagManager\Dao\VariablesDao;
use Piwik\Plugins\TagManager\Template\Tag\TagsProvider;
use Piwik\Plugins\TagManager\Template\Trigger\TriggersProvider;
use Piwik\Plugins\TagManager\Template\Variable\VariablesProvider;
Expand Down Expand Up @@ -62,16 +67,48 @@ class Import
*/
private $accessValidator;

public function __construct(Tag $tags, Trigger $triggers, Variable $variables, Container $containers, AccessValidator $accessValidator, TagsProvider $tagsProvider, TriggersProvider $triggersProvider, VariablesProvider $variablesProvider)
{
/**
* @var TagsDao
*/
protected $tagsDao;

/**
* @var TriggersDao
*/
protected $triggersDao;

/**
* @var VariablesDao
*/
protected $variablesDao;

public function __construct(
Tag $tags,
Trigger $triggers,
Variable $variables,
Container $containers,
AccessValidator $accessValidator,
TagsProvider $tagsProvider,
TriggersProvider $triggersProvider,
VariablesProvider $variablesProvider,
TagsDao $tagsDao,
TriggersDao $triggersDao,
VariablesDao $variablesDao
) {
$this->tags = $tags;
$this->triggers = $triggers;
$this->variables = $variables;
$this->containers = $containers;

$this->accessValidator = $accessValidator;

$this->tagsProvider = $tagsProvider;
$this->triggersProvider = $triggersProvider;
$this->variablesProvider = $variablesProvider;

$this->tagsDao = $tagsDao;
$this->triggersDao = $triggersDao;
$this->variablesDao = $variablesDao;
}

public function checkImportContainerIsPossible($exportedContainerVersion, $idSite, $idContainer)
Expand Down Expand Up @@ -119,17 +156,10 @@ public function importContainerVersion($exportedContainerVersion, $idSite, $idCo
{
$this->checkImportContainerIsPossible($exportedContainerVersion, $idSite, $idContainer);

foreach ($this->tags->getContainerTags($idSite, $idContainerVersion) as $tag) {
Copy link
Contributor

@AltamashShaikh AltamashShaikh Aug 5, 2022

Choose a reason for hiding this comment

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

@tsteur any specific reason we deleted 1 tag within a container at a time instead of deleting all at once like in this PR ?

$this->tags->deleteContainerTag($idSite, $idContainerVersion, $tag['idtag']);
}

foreach ($this->triggers->getContainerTriggers($idSite, $idContainerVersion) as $trigger) {
$this->triggers->deleteContainerTrigger($idSite, $idContainerVersion, $trigger['idtrigger']);
}

foreach ($this->variables->getContainerVariables($idSite, $idContainerVersion) as $variable) {
$this->variables->deleteContainerVariable($idSite, $idContainerVersion, $variable['idvariable']);
}
$now = Date::now()->getDatetime();
$this->tagsDao->deleteContainerTags($idSite, $idContainerVersion, $now);
Copy link
Contributor

@AltamashShaikh AltamashShaikh Aug 5, 2022

Choose a reason for hiding this comment

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

@ulcuber Can you create a deleteContainerTags() in Model/Tag.php and call the Model instead of dao

$this->triggersDao->deleteContainerTriggers($idSite, $idContainerVersion, $now);
Copy link
Contributor

@AltamashShaikh AltamashShaikh Aug 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Same here call deleteContainerTriggers() in Model/Trigger.php and call the Model instead of dao

$this->variablesDao->deleteContainerVariables($idSite, $idContainerVersion, $now);
Copy link
Contributor

@AltamashShaikh AltamashShaikh Aug 5, 2022

Choose a reason for hiding this comment

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

Same here Same here call deleteContainerVariables() in Model/Variable.php and call the Model instead of dao


$ecv = $exportedContainerVersion;

Expand All @@ -145,7 +175,7 @@ public function importContainerVersion($exportedContainerVersion, $idSite, $idCo
'defaultValue' => $variable['default_value'],
'lookupTable' => $variable['lookup_table'],
));
} catch (EntityRecursionException $e){
} catch (EntityRecursionException $e) {
throw new \Exception(Piwik::translate('TagManager_EntityRecursionExceptionForVariable', array($variable['name'] . '(' . $variable['type'] . ')')));
}
}
Expand Down Expand Up @@ -200,6 +230,4 @@ public function importContainerVersion($exportedContainerVersion, $idSite, $idCo
));
}
}


}
17 changes: 16 additions & 1 deletion Dao/TagsDao.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,21 @@ public function deleteContainerTag($idSite, $idContainerVersion, $idTag, $delete
Db::query($query, $bind);
}

/**
* @param int $idSite
* @param int $idContainerVersion
* @param string $deletedDate
*/
public function deleteContainerTags($idSite, $idContainerVersion, $deletedDate)
Copy link
Contributor

@AltamashShaikh AltamashShaikh Jul 28, 2022

Choose a reason for hiding this comment

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

@ulcuber You aiming to update Import.php too with this PR right ?

Can you add those changes too ?

Copy link
Contributor Author

@ulcuber ulcuber Jul 29, 2022

Choose a reason for hiding this comment

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

I have tried to split it into small atomic independent issues. I'll optimize import next week

Copy link
Contributor

@AltamashShaikh AltamashShaikh Aug 1, 2022

Choose a reason for hiding this comment

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

@ulcuber Its better to include all such changes in 1 PR so that we can test and ensure that new changes do not lead to any regressions, I would suggest to add the Importer changes in same PR too or folk a branch from this and add those changes if you want to keep your changes separately

{
$table = $this->tablePrefixed;

$query = "UPDATE $table SET status = ?, deleted_date = ? WHERE idsite = ? and idcontainerversion = ? and status != ?";
$bind = array(self::STATUS_DELETED, $deletedDate, $idSite, $idContainerVersion, self::STATUS_DELETED);

Db::query($query, $bind);
}

private function enrichTags($tags)
{
if (empty($tags)) {
Expand Down Expand Up @@ -217,7 +232,7 @@ private function enrichTag($tag)
$tag['idcontainerversion'] = (int) $tag['idcontainerversion'];
$tag['fire_delay'] = (int)$tag['fire_delay'];
$tag['priority'] = (int)$tag['priority'];

if ($tag['start_date'] === '0000-00-00 00:00:00') {
$tag['start_date'] = null;
}
Expand Down
15 changes: 15 additions & 0 deletions Dao/TriggersDao.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,21 @@ public function deleteContainerTrigger($idSite, $idContainerVersion, $idTrigger,
Db::query($query, $bind);
}

/**
* @param int $idSite
* @param int $idContainerVersion
* @param string $deletedDate
*/
public function deleteContainerTriggers($idSite, $idContainerVersion, $deletedDate)
{
$table = $this->tablePrefixed;

$query = "UPDATE $table SET status = ?, deleted_date = ? WHERE idsite = ? and idcontainerversion = ? and status != ?";
$bind = array(self::STATUS_DELETED, $deletedDate, $idSite, $idContainerVersion, self::STATUS_DELETED);

Db::query($query, $bind);
}

private function enrichTriggers($triggers)
{
if (empty($triggers)) {
Expand Down
15 changes: 15 additions & 0 deletions Dao/VariablesDao.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,21 @@ public function deleteContainerVariable($idSite, $idContainerVersion, $idVariabl
Db::query($query, $bind);
}

/**
* @param int $idSite
* @param int $idContainerVersion
* @param string $deletedDate
*/
public function deleteContainerVariables($idSite, $idContainerVersion, $deletedDate)
{
$table = $this->tablePrefixed;

$query = "UPDATE $table SET status = ?, deleted_date = ? WHERE idsite = ? and idcontainerversion = ? and status != ?";
$bind = array(self::STATUS_DELETED, $deletedDate, $idSite, $idContainerVersion, self::STATUS_DELETED);

Db::query($query, $bind);
}

private function enrichVariables($variables)
{
if (empty($variables)) {
Expand Down