Repository navigation
Fix the outcome reporting and scope of the repository delete flow - #37
Merged
Merged
Conversation
A rejected poll of a task status (proxy error, dropped connection, refused status request) used to propagate as an ordinary error, so the delete dialog said "Not removed", the create flow said the distribution "was not created" and the revert said "Failed to revert", all after the server had accepted the request and the work may well have been done. waitForTask now rejects with a TaskUnknown (a TaskPending carrying the task href) when a status request fails. Delete, create-distribution and revert report "outcome unknown, check the task" with the task link and never claim a failure.
While the distributions and the repository were being removed (minutes with the default polling), Cancel, the close button and Escape stayed active. Closing unmounted the dialog, so the report of a partial failure (what was removed and what was not) went to a component that no longer existed, and a run that finished later closed whichever repository dialog was open by then. DeleteModal takes isCancelDisabled, which the repository dialog sets while deleting; the report stays on screen and Cancel works again once the run has reported. A finished delete now closes only the dialog of its own repository.
The delete dialog read every rejection of the delete as "the distributions could not be listed, so nothing was deleted". The action behind it also ran the page updates after the repository was removed (alert, closing the dialog, reloading the list), so a failure of any of those produced a false statement about a repository that no longer exists. The utility now rejects only with DeleteNotStarted, and only when the listing failed. The dialog says "nothing was deleted" for that error alone; any other error gets a message that some distributions or the repository may already have been removed. The page updates after a successful delete run independently and can no longer reject the delete.
The dialog listed the distributions when it opened, but the delete listed them again at confirmation and removed whatever came back. A distribution created or pointed at the repository in between was deleted without ever having been shown or its content guard flagged. The sets to delete also came from server side filters alone: Pulp ignores a query parameter a plugin does not know, so a missing repository filter would have answered with every distribution of the plugin. The listing now keeps only rows that name the repository themselves (repository on a distribution, repository_version on a publication). The delete takes the listing the dialog showed and removes exactly those that still serve the repository; a distribution that appeared since is not deleted, is reported by name, and keeps the repository.
The test asserted that Sync is enabled right after the task list request was issued, before the answer was applied, so it passed whether the mock answered with a running task or with an empty list. It now waits for the check to finish before asserting that no warning is shown and Sync is enabled, asserts the task filter (waiting and running tasks of the repository), and runs for the file plugin too.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes five defects found in review of the delete flow and its tests.
Every fix has a test that fails without it; the membership checks and the shown-set check were mutation-checked.