Skip to content

Commit 28aa425

Browse files
author
Scott Bommarito
authored
Remove unlist checkbox from Manage Deprecation form (#6965)
1 parent 5c98bb0 commit 28aa425

10 files changed

Lines changed: 37 additions & 126 deletions

File tree

‎src/Bootstrap/dist/css/bootstrap-theme.css‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/Bootstrap/less/theme/page-manage-deprecation.less‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@
3737
width: 65px;
3838
margin-right: 15px;
3939
}
40+
41+
.cvss-bold {
42+
font-weight: bold;
43+
}
4044
}
4145

4246
.security-detail-list-item {

‎src/NuGetGallery/Controllers/ManageDeprecationJsonApiController.cs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,7 @@ public virtual async Task<JsonResult> Deprecate(
105105
IEnumerable<string> cweIds,
106106
string alternatePackageId,
107107
string alternatePackageVersion,
108-
string customMessage,
109-
bool shouldUnlist)
108+
string customMessage)
110109
{
111110
var currentUser = GetCurrentUser();
112111
if (!_featureFlagService.IsManageDeprecationEnabled(GetCurrentUser()))
@@ -248,8 +247,7 @@ await _deprecationService.UpdateDeprecation(
248247
cwes,
249248
alternatePackageRegistration,
250249
alternatePackage,
251-
customMessage,
252-
shouldUnlist);
250+
customMessage);
253251

254252
return Json(HttpStatusCode.OK);
255253
}

‎src/NuGetGallery/Scripts/gallery/page-manage-deprecation.js‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -462,9 +462,6 @@ function ManageDeprecationViewModel(id, versionDeprecationStateDictionary, defau
462462
// The custom message to submit with the form.
463463
this.customMessage = ko.observable('');
464464

465-
// Whether or not the packages should be unlisted.
466-
this.shouldUnlist = ko.observable(true);
467-
468465
this.submitError = ko.observable();
469466
this.submit = function () {
470467
self.submitError(null);
@@ -489,8 +486,7 @@ function ManageDeprecationViewModel(id, versionDeprecationStateDictionary, defau
489486
cweIds: self.cwes.exportIds(),
490487
alternatePackageId: self.alternatePackageId(),
491488
alternatePackageVersion: self.alternatePackageVersion(),
492-
customMessage: self.customMessage(),
493-
shouldUnlist: self.shouldUnlist()
489+
customMessage: self.customMessage()
494490
}),
495491
success: function () {
496492
window.location.href = packageUrl;
@@ -522,7 +518,6 @@ function ManageDeprecationViewModel(id, versionDeprecationStateDictionary, defau
522518
versionData.AlternatePackageId = self.alternatePackageId();
523519
versionData.AlternatePackageVersion = self.alternatePackageVersion();
524520
versionData.CustomMessage = self.customMessage();
525-
versionData.ShouldUnlist = self.shouldUnlist();
526521
};
527522

528523
var loadDeprecationFormState = function (version) {
@@ -549,7 +544,6 @@ function ManageDeprecationViewModel(id, versionDeprecationStateDictionary, defau
549544
}
550545

551546
self.customMessage(versionData.CustomMessage);
552-
self.shouldUnlist(versionData.ShouldUnlist);
553547
};
554548

555549
// When the chosen versions are changed, remember the contents of the form in case the user navigates back to this version.

‎src/NuGetGallery/Services/IPackageDeprecationService.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,7 @@ Task UpdateDeprecation(
2424
IReadOnlyCollection<Cwe> cwe,
2525
PackageRegistration alternatePackageRegistration,
2626
Package alternatePackage,
27-
string customMessage,
28-
bool shouldUnlist);
27+
string customMessage);
2928

3029
/// <summary>
3130
/// Fetches all <see cref="Cve"/>s with a <see cref="Cve.CveId"/> contained in <paramref name="ids"/>.

‎src/NuGetGallery/Services/PackageDeprecationService.cs‎

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,15 @@ public class PackageDeprecationService : IPackageDeprecationService
1515
private readonly IEntityRepository<PackageDeprecation> _deprecationRepository;
1616
private readonly IEntityRepository<Cve> _cveRepository;
1717
private readonly IEntityRepository<Cwe> _cweRepository;
18-
private readonly IPackageService _packageService;
19-
private readonly IIndexingService _indexingService;
2018

2119
public PackageDeprecationService(
2220
IEntityRepository<PackageDeprecation> deprecationRepository,
2321
IEntityRepository<Cve> cveRepository,
24-
IEntityRepository<Cwe> cweRepository,
25-
IPackageService packageService,
26-
IIndexingService indexingService)
22+
IEntityRepository<Cwe> cweRepository)
2723
{
2824
_deprecationRepository = deprecationRepository ?? throw new ArgumentNullException(nameof(deprecationRepository));
2925
_cveRepository = cveRepository ?? throw new ArgumentNullException(nameof(cveRepository));
3026
_cweRepository = cweRepository ?? throw new ArgumentNullException(nameof(cweRepository));
31-
_packageService = packageService ?? throw new ArgumentNullException(nameof(packageService));
32-
_indexingService = indexingService ?? throw new ArgumentNullException(nameof(indexingService));
3327
}
3428

3529
public async Task UpdateDeprecation(
@@ -40,8 +34,7 @@ public async Task UpdateDeprecation(
4034
IReadOnlyCollection<Cwe> cwes,
4135
PackageRegistration alternatePackageRegistration,
4236
Package alternatePackage,
43-
string customMessage,
44-
bool shouldUnlist)
37+
string customMessage)
4538
{
4639
if (packages == null || !packages.Any())
4740
{
@@ -104,11 +97,6 @@ public async Task UpdateDeprecation(
10497
deprecation.AlternatePackage = alternatePackage;
10598

10699
deprecation.CustomMessage = customMessage;
107-
108-
if (shouldUnlist)
109-
{
110-
await _packageService.MarkPackageUnlistedAsync(package, commitChanges: false);
111-
}
112100
}
113101
}
114102

@@ -122,12 +110,6 @@ public async Task UpdateDeprecation(
122110
}
123111

124112
await _deprecationRepository.CommitChangesAsync();
125-
126-
// Update the indexing of the packages we updated the deprecation information of.
127-
foreach (var package in packages)
128-
{
129-
_indexingService.UpdatePackage(package);
130-
}
131113
}
132114

133115
public async Task<IReadOnlyCollection<Cve>> GetOrCreateCvesByIdAsync(IEnumerable<string> ids, bool commitChanges)

‎src/NuGetGallery/ViewModels/ManagePackageViewModel.cs‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -168,10 +168,6 @@ public VersionDeprecationState(Package package, string text)
168168
}
169169

170170
CustomMessage = deprecation.CustomMessage;
171-
172-
// It doesn't make sense to unlist packages that are already unlisted.
173-
// Additionally, if a package was not unlisted when it was deprecated, we shouldn't unlist it when its deprecation information is updated.
174-
ShouldUnlist = package.Listed && deprecation.Status == PackageDeprecationStatus.NotDeprecated;
175171
}
176172
}
177173

@@ -185,7 +181,6 @@ public VersionDeprecationState(Package package, string text)
185181
public string AlternatePackageId { get; }
186182
public string AlternatePackageVersion { get; }
187183
public string CustomMessage { get; }
188-
public bool ShouldUnlist { get; }
189184

190185
/// <remarks>
191186
/// Ideally, the fields of this class would be pascal-case.

‎src/NuGetGallery/Views/Packages/_ManageDeprecation.cshtml‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@
5959
</div>
6060
<div class="security-detail-input">
6161
<input id="selectedCvssRating" class="form-control cvss-input" name="selectedCvssRating" type="text" placeholder="0.0" size="4" maxlength="4" data-bind="textInput: selectedCvssRating, disable: !hasCvss()" />
62-
<span data-bind="text: cvssRatingLabel, css: { 'text-danger': cvssRatingIsInvalid }"></span>
62+
<span data-bind="text: cvssRatingLabel, css: { 'text-danger': cvssRatingIsInvalid, 'cvss-bold': !cvssRatingIsInvalid() }"></span>
6363
</div>
6464
</div>
6565
<div data-bind="template: { name: 'deprecation-security-detail-list-input-template', data: cwes }"></div>
@@ -98,12 +98,6 @@
9898
<label for="customMessage" class="deprecation-section-header">Provide custom message</label>
9999
<textarea id="customMessage" name="customMessage" class="form-control full-line" data-bind="textInput: customMessage" rows="2"></textarea>
100100
</div>
101-
<div class="deprecation-section form-group">
102-
<label>
103-
<input type="checkbox" value="true" data-bind="checked: shouldUnlist" />
104-
Unlist this package from search results
105-
</label>
106-
</div>
107101
</div>
108102

109103
<div data-bind="visible: submitError">

‎tests/NuGetGallery.Facts/Controllers/ManageDeprecationJsonApiControllerFacts.cs‎

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ public async Task ReturnsForbiddenIfFeatureFlagDisabled()
289289

290290
// Act
291291
var result = await controller.Deprecate(
292-
"id", null, false, false, false, null, null, null, null, null, null, false);
292+
"id", null, false, false, false, null, null, null, null, null, null);
293293

294294
// Assert
295295
AssertErrorResponse(controller, result, HttpStatusCode.Forbidden, Strings.DeprecatePackage_Forbidden);
@@ -317,7 +317,7 @@ public async Task ReturnsBadRequestIfNoVersions(IEnumerable<string> versions)
317317

318318
// Act
319319
var result = await controller.Deprecate(
320-
"id", versions, false, false, false, null, null, null, null, null, null, false);
320+
"id", versions, false, false, false, null, null, null, null, null, null);
321321

322322
// Assert
323323
AssertErrorResponse(controller, result, HttpStatusCode.BadRequest, Strings.DeprecatePackage_NoVersions);
@@ -357,7 +357,7 @@ public async Task ReturnsBadRequestIfCveIdInvalid(string invalidId)
357357

358358
// Act
359359
var result = await controller.Deprecate(
360-
"id", new[] { "1.0.0" }, false, false, false, new[] { "CVE-2019-1111", invalidId }, null, null, null, null, null, false);
360+
"id", new[] { "1.0.0" }, false, false, false, new[] { "CVE-2019-1111", invalidId }, null, null, null, null, null);
361361

362362
// Assert
363363
AssertErrorResponse(
@@ -387,7 +387,7 @@ public async Task ReturnsBadRequestIfCweIdInvalid()
387387

388388
// Act
389389
var result = await controller.Deprecate(
390-
"id", new[] { "1.0.0" }, false, false, false, null, null, new[] { "CWE-1", invalidId }, null, null, null, false);
390+
"id", new[] { "1.0.0" }, false, false, false, null, null, new[] { "CWE-1", invalidId }, null, null, null);
391391

392392
// Assert
393393
AssertErrorResponse(
@@ -417,7 +417,7 @@ public async Task ReturnsBadRequestIfCvssInvalid(decimal cvss)
417417

418418
// Act
419419
var result = await controller.Deprecate(
420-
"id", new[] { "1.0.0" }, false, false, false, null, cvss, null, null, null, null, false);
420+
"id", new[] { "1.0.0" }, false, false, false, null, cvss, null, null, null, null);
421421

422422
// Assert
423423
AssertErrorResponse(controller, result, HttpStatusCode.BadRequest, Strings.DeprecatePackage_InvalidCvss);
@@ -449,7 +449,7 @@ public async Task ReturnsNotFoundIfNoPackagesOrRegistrationMissing(IEnumerable<P
449449

450450
// Act
451451
var result = await controller.Deprecate(
452-
id, new[] { "1.0.0" }, false, false, false, null, null, null, null, null, null, false);
452+
id, new[] { "1.0.0" }, false, false, false, null, null, null, null, null, null);
453453

454454
// Assert
455455
AssertErrorResponse(controller, result, HttpStatusCode.NotFound, string.Format(Strings.DeprecatePackage_MissingRegistration, id));
@@ -511,7 +511,7 @@ public async Task ReturnsForbiddenIfNotOwner(User currentUser, User owner)
511511

512512
// Act
513513
var result = await controller.Deprecate(
514-
id, new[] { "1.0.0" }, false, false, false, null, null, null, null, null, null, false);
514+
id, new[] { "1.0.0" }, false, false, false, null, null, null, null, null, null);
515515

516516
// Assert
517517
AssertErrorResponse(controller, result, HttpStatusCode.Forbidden, Strings.DeprecatePackage_Forbidden);
@@ -586,7 +586,7 @@ public async Task ReturnsForbiddenIfLocked(User currentUser, User owner)
586586

587587
// Act
588588
var result = await controller.Deprecate(
589-
id, new[] { "1.0.0" }, false, false, false, null, null, null, null, null, null, false);
589+
id, new[] { "1.0.0" }, false, false, false, null, null, null, null, null, null);
590590

591591
// Assert
592592
AssertErrorResponse(
@@ -640,7 +640,7 @@ public async Task ReturnsNotFoundIfAlternatePackageRegistrationMissing(User curr
640640

641641
// Act
642642
var result = await controller.Deprecate(
643-
id, new[] { "1.0.0" }, false, false, false, null, null, null, alternatePackageId, null, null, false);
643+
id, new[] { "1.0.0" }, false, false, false, null, null, null, alternatePackageId, null, null);
644644

645645
// Assert
646646
AssertErrorResponse(
@@ -695,7 +695,7 @@ public async Task ReturnsNotFoundIfAlternatePackageVersionMissing(User currentUs
695695

696696
// Act
697697
var result = await controller.Deprecate(
698-
id, new[] { "1.0.0" }, false, false, false, null, null, null, alternatePackageId, alternatePackageVersion, null, false);
698+
id, new[] { "1.0.0" }, false, false, false, null, null, null, alternatePackageId, alternatePackageVersion, null);
699699

700700
// Assert
701701
AssertErrorResponse(
@@ -744,7 +744,7 @@ public async Task ReturnsNotFoundIfVersionMissing(User currentUser, User owner)
744744

745745
// Act
746746
var result = await controller.Deprecate(
747-
id, new[] { "1.0.0" }, false, false, false, null, null, null, null, null, null, false);
747+
id, new[] { "1.0.0" }, false, false, false, null, null, null, null, null, null);
748748

749749
// Assert
750750
AssertErrorResponse(
@@ -793,7 +793,7 @@ public async Task ReturnsNotFoundIfSomeVersionMissing(User currentUser, User own
793793

794794
// Act
795795
var result = await controller.Deprecate(
796-
id, new[] { package.NormalizedVersion, "1.0.0" }, false, false, false, null, null, null, null, null, null, false);
796+
id, new[] { package.NormalizedVersion, "1.0.0" }, false, false, false, null, null, null, null, null, null);
797797

798798
// Assert
799799
AssertErrorResponse(
@@ -855,7 +855,7 @@ public async Task ReturnsNotFoundIfGetCwesByIdThrowsArgumentException(User curre
855855

856856
// Act
857857
var result = await controller.Deprecate(
858-
id, new[] { package.NormalizedVersion }, false, false, false, null, null, null, null, null, null, false);
858+
id, new[] { package.NormalizedVersion }, false, false, false, null, null, null, null, null, null);
859859

860860
// Assert
861861
AssertErrorResponse(
@@ -916,7 +916,6 @@ public enum ReturnsSuccessful_AlternatePackage_State
916916
Owner_Data,
917917
PackageDeprecationStates_Data,
918918
MemberDataHelper.EnumDataSet<ReturnsSuccessful_AlternatePackage_State>(),
919-
MemberDataHelper.BooleanDataSet(),
920919
MemberDataHelper.BooleanDataSet()).ToList();
921920

922921
[Theory]
@@ -929,8 +928,7 @@ public async Task ReturnsSuccessful(
929928
bool isOther,
930929
PackageDeprecationStatus expectedStatus,
931930
ReturnsSuccessful_AlternatePackage_State alternatePackageState,
932-
bool hasAdditionalData,
933-
bool shouldUnlist)
931+
bool hasAdditionalData)
934932
{
935933
// Arrange
936934
var id = "id";
@@ -1029,8 +1027,7 @@ public async Task ReturnsSuccessful(
10291027
cwes,
10301028
alternatePackageState == ReturnsSuccessful_AlternatePackage_State.Registration ? alternatePackageRegistration : null,
10311029
alternatePackageState == ReturnsSuccessful_AlternatePackage_State.Package ? alternatePackage : null,
1032-
customMessage,
1033-
shouldUnlist))
1030+
customMessage))
10341031
.Completes()
10351032
.Verifiable();
10361033

@@ -1049,8 +1046,7 @@ public async Task ReturnsSuccessful(
10491046
cweIds,
10501047
alternatePackageId,
10511048
alternatePackageVersion,
1052-
customMessage,
1053-
shouldUnlist);
1049+
customMessage);
10541050

10551051
// Assert
10561052
AssertSuccessResponse(controller);

0 commit comments

Comments
 (0)