Add ability to reflow hard-deletes from Admin area #4899
Conversation
| @@ -165,6 +168,30 @@ public class PackageDeleteService | |||
| UpdateSearchIndex(); | |||
| } | |||
|
|
|||
| public Task ReflowHardDeletedPackagesAsync(string id, string version, User deletedBy) | |||
| { | |||
There was a problem hiding this comment.
deletedBy does not seem used.
There was a problem hiding this comment.
What is the purpose of this method?
There was a problem hiding this comment.
it reflows a hard-deleted package
|
|
||
| [HttpPost] | ||
| [ValidateAntiForgeryToken] | ||
| public virtual async Task<ActionResult> Reflow(HardDeleteReflowViewModel model) |
There was a problem hiding this comment.
We should support bulk by splitting lines, ID Version. Similar to the delete UI.
There was a problem hiding this comment.
agreed, will add
There was a problem hiding this comment.
@joelverhagen just pushed this change
|
Screenshot plz |
| } | ||
| } | ||
|
|
||
| var auditRecord = new PackageAuditRecord(normalizedId, normalizedVersion, string.Empty, null, null, AuditedPackageAction.Delete, "reflow hard-deleted package"); |
There was a problem hiding this comment.
label null and string.Empty parameters
|
Related: #4876 |
|
|
||
| if (existingPackage != null) | ||
| { | ||
| throw new ArgumentException("The package exists! You can only reflow hard-deleted packages that do not exist."); |
There was a problem hiding this comment.
Consider throwing a UserSafeException instead of an ArgumentException so that the Reflow admin action gets a real error message using e.GetUserSafeMessage(). Also, consider adding the normalizedId and normalizedVersion to the exception message just for convenience's sake.
There was a problem hiding this comment.
done, looks nice
| @@ -156,8 +155,81 @@ public virtual ActionResult Reflow() | |||
|
|
|||
| TempData["Message"] = e.GetUserSafeMessage(); | |||
|
|
|||
| return Reflow(); | |||
| return View("Reflow"); | |||
There was a problem hiding this comment.
nameof(Reflow). The view name in general should match the method name.
There was a problem hiding this comment.
(this is an MVC convention)
|
|
||
| <div class="form-field"> | ||
| @Html.LabelFor(m => m.BulkList) | ||
| @Html.TextAreaFor(m => m.BulkList, 10, 50, null) |
There was a problem hiding this comment.
Spoke offline about this, need width: 100% i think
There was a problem hiding this comment.
it was form-field!!!
|
|
||
| @if (Model == null) | ||
| { | ||
| @Html.Partial("_ReflowSingle") |
There was a problem hiding this comment.
Spoke offline, I don't think we need the single if we have the bulk.
| return View("Reflow"); | ||
| } | ||
|
|
||
| var lines = request.BulkList.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries); |
There was a problem hiding this comment.
Super nit: I personally like to use StringReader for this so you don't have the ugly \r \n` constant in your code. But this is totally fine.
There was a problem hiding this comment.
In the future I will use StringReader.
| { | ||
| if (string.IsNullOrEmpty(id)) | ||
| { | ||
| throw new UserSafeException($"Must supply an ID for the hard-deleted package to reflow."); |
There was a problem hiding this comment.
$ [](start = 44, length = 1)
Nit, unused $ on this exception and the one on line 180.
#4876
Existing delete page:
New page (just for reflowing hard deletes):
