Skip to content

Commit

Permalink
fix: request #37545 Deleting or moving an artifact can delete values …
Browse files Browse the repository at this point in the history
…from unrelated artifacts

Issue has been introduced by git #tuleap/stable/7f2e5e974596196bd96c2146c533353ecbcc592f.

Change-Id: I04ac31d1803abcd5ce46f121e1411825f063247b
  • Loading branch information
LeSuisse committed Mar 28, 2024
1 parent 7a8781e commit a0ba0ae
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 70 deletions.
Expand Up @@ -217,7 +217,6 @@ public function build(LoggerInterface $logger): ArchiveAndDeleteArtifactTask
new ComputedFieldDaoCache(new ComputedFieldDao()),
new RecentlyVisitedDao(),
new PendingArtifactRemovalDao(),
new ArtifactChangesetValueDeletorDAO(),
new PostArtifactMoveReferencesCleaner(
$cross_reference_manager,
new ReverseLinksRetriever(new ReverseLinksDao(), $tracker_artifact_factory),
Expand Down

This file was deleted.

Expand Up @@ -36,7 +36,6 @@ public function __construct(
private readonly ComputedFieldDaoCache $computed_dao_cache,
private readonly RecentlyVisitedDao $recently_visited_dao,
private readonly PendingArtifactRemovalDao $artifact_removal,
private readonly ArtifactChangesetValueDeletorDAO $changeset_value_deletor_DAO,
private readonly PostArtifactMoveReferencesCleaner $post_artifact_move_references_cleaner,
private readonly PostArtifactDeletionCleaner $artifact_deletion_cleaner,
) {
Expand All @@ -48,7 +47,6 @@ public function cleanDependencies(Artifact $artifact, DeletionContext $context,

$this->cleanReferences($artifact, $context, $user);

$this->changeset_value_deletor_DAO->cleanAllChangesetValueInTransaction($artifact);
$this->dao->deleteUnsubscribeNotificationForArtifact($artifact->getId());
// We do not keep trace of the history change here because it doesn't have any sense
$this->tracker_artifact_priority_manager->deletePriority($artifact);
Expand Down
Expand Up @@ -41,10 +41,35 @@ public function getPendingArtifactById($artifact_id)
return $this->getDB()->row($sql, $artifact_id);
}

public function removeArtifact($artifact_id)
public function removeArtifact($artifact_id): void
{
$sql = 'DELETE FROM plugin_tracker_artifact_pending_removal
WHERE id = ?';
$tables_to_clean_with_join = [
'tracker_changeset' => 'tracker_changeset.artifact_id = plugin_tracker_artifact_pending_removal.id',
'tracker_changeset_comment' => 'tracker_changeset_comment.changeset_id = tracker_changeset.id',
'tracker_changeset_comment_fulltext' => 'tracker_changeset_comment_fulltext.comment_id = tracker_changeset_comment.id',
'tracker_changeset_incomingmail' => 'tracker_changeset_incomingmail.changeset_id = tracker_changeset.id',
'tracker_changeset_value' => 'tracker_changeset_value.changeset_id = tracker_changeset.id',
'tracker_changeset_value_artifactlink' => 'tracker_changeset_value_artifactlink.changeset_value_id = tracker_changeset_value.id',
'tracker_changeset_value_computedfield_manual_value' => 'tracker_changeset_value_computedfield_manual_value.changeset_value_id = tracker_changeset_value.id',
'tracker_changeset_value_date' => 'tracker_changeset_value_date.changeset_value_id = tracker_changeset_value.id',
'tracker_changeset_value_file' => 'tracker_changeset_value_file.changeset_value_id = tracker_changeset_value.id',
'tracker_changeset_value_float' => 'tracker_changeset_value_float.changeset_value_id = tracker_changeset_value.id',
'tracker_changeset_value_int' => 'tracker_changeset_value_int.changeset_value_id = tracker_changeset_value.id',
'tracker_changeset_value_list' => 'tracker_changeset_value_list.changeset_value_id = tracker_changeset_value.id',
'tracker_changeset_value_openlist' => 'tracker_changeset_value_openlist.changeset_value_id = tracker_changeset_value.id',
'tracker_changeset_value_permissionsonartifact' => 'tracker_changeset_value_permissionsonartifact.changeset_value_id = tracker_changeset_value.id',
'tracker_changeset_value_text' => 'tracker_changeset_value_text.changeset_value_id = tracker_changeset_value.id',
];

$tables = \Psl\Str\join(array_keys($tables_to_clean_with_join), ', ');
$joins = '';
foreach ($tables_to_clean_with_join as $table => $join) {
$joins = $joins . sprintf('LEFT JOIN %s ON (%s) ', $table, $join);
}

$sql = "DELETE plugin_tracker_artifact_pending_removal, $tables
FROM plugin_tracker_artifact_pending_removal $joins
WHERE plugin_tracker_artifact_pending_removal.id = ?";

$this->getDB()->run($sql, $artifact_id);
}
Expand Down
Expand Up @@ -52,7 +52,6 @@ protected function setUp(): void
$computed_dao_cache = $this->createStub(ComputedFieldDaoCache::class);
$recently_visited_dao = $this->createStub(RecentlyVisitedDao::class);
$artifact_removal = $this->createStub(PendingArtifactRemovalDao::class);
$value_deletor = $this->createStub(ArtifactChangesetValueDeletorDAO::class);
$this->post_move_deletor = $this->createStub(PostArtifactMoveReferencesCleaner::class);
$this->artifact_deletor = $this->createStub(PostArtifactDeletionCleaner::class);

Expand All @@ -62,7 +61,6 @@ protected function setUp(): void
$computed_dao_cache->expects(self::atLeastOnce())->method('deleteAllArtifactCacheValues');
$recently_visited_dao->expects(self::atLeastOnce())->method('deleteVisitByArtifactId');
$artifact_removal->expects(self::atLeastOnce())->method('removeArtifact');
$value_deletor->expects(self::atLeastOnce())->method('cleanAllChangesetValueInTransaction');

$this->deletor = new ArtifactDependenciesCleaner(
$permissions_manager,
Expand All @@ -71,7 +69,6 @@ protected function setUp(): void
$computed_dao_cache,
$recently_visited_dao,
$artifact_removal,
$value_deletor,
$this->post_move_deletor,
$this->artifact_deletor
);
Expand Down

0 comments on commit a0ba0ae

Please sign in to comment.