Allow MarkPackageForDestructionService to be called from within a transaction

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

The following discussion from !135487 (merged) should be addressed:

  • @10io started a discussion: (+4 comments)

    Don't we need a database transaction here?

    On L21, we are updating the package to mark it for destruction and on L28 we destroy the model_version. Don't we want these operation to either both succeed or both fail.

    Ah, I see. This is actually the first time that we destroy a package and do something else. Up to now, we had only the "destroy package" operation to do. In other words, we had only the MarkPackageForDestructionService to execute.

    The challenge here is that we have 2 services to execute. I think we should work towards having that transaction so that there is no way to end up in an inconsistent state.

    To fix this problem here, I think we will need to follow what is suggested:

    • In the Packages::Package model, include the AfterCommitQueue concern.
    • In #mark_package_files_for_destruction body function, use the #run_after_commit_or_now.
Edited by 🤖 GitLab Bot 🤖