Skip to content

Commit 5c98bb0

Browse files
author
Scott Bommarito
authored
Improve deprecation information UI on Display Package page with multiple reasons (#6961)
1 parent e428f5b commit 5c98bb0

5 files changed

Lines changed: 126 additions & 63 deletions

File tree

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

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

‎src/Bootstrap/less/theme/page-display-package.less‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,24 +24,30 @@
2424
display: flex;
2525
justify-content: space-between;
2626
vertical-align: middle;
27+
width: 100%;
2728

28-
.deprecation-expander-icon-container {
29+
.deprecation-expander-container {
2930
display: flex;
30-
align-items: center;
31-
31+
3232
.deprecation-expander-icon {
3333
position: unset;
3434
top: unset;
3535
}
36-
}
36+
37+
.deprecation-expander-info-right {
38+
padding-left: 15px;
39+
}
3740

38-
.deprecation-expander-info-right {
39-
padding-left: 15px;
41+
.deprecation-expander-severity-rating {
42+
margin-left: 5px;
43+
}
4044
}
4145
}
4246

4347
.deprecation-content-container {
4448
margin-top: 15px;
49+
padding-top: 15px;
50+
border-top: 1px solid lightgray;
4551

4652
p {
4753
margin-top: 5px;

‎src/NuGetGallery/ViewModels/DisplayPackageViewModel.cs‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,29 @@ public DisplayPackageViewModel(Package package, User currentUser, PackageDepreca
5252
DeprecationStatus = deprecation.Status;
5353

5454
CveIds = deprecation.Cves?.Select(c => c.CveId).ToList();
55-
CvssRating = deprecation.CvssRating;
5655
CweIds = deprecation.Cwes?.Select(c => c.CweId).ToList();
5756

57+
if (deprecation.CvssRating.HasValue)
58+
{
59+
var cvssRating = deprecation.CvssRating.Value;
60+
if (cvssRating < 4)
61+
{
62+
Severity = "Low";
63+
}
64+
else if (cvssRating < 7)
65+
{
66+
Severity = "Medium";
67+
}
68+
else if (cvssRating < 9)
69+
{
70+
Severity = "High";
71+
}
72+
else
73+
{
74+
Severity = "Critical";
75+
}
76+
}
77+
5878
AlternatePackageId = deprecation.AlternatePackageRegistration?.Id;
5979

6080
var alternatePackage = deprecation.AlternatePackage;
@@ -165,7 +185,7 @@ public bool HasNewerRelease
165185

166186
public PackageDeprecationStatus DeprecationStatus { get; set; }
167187
public IReadOnlyCollection<string> CveIds { get; set; }
168-
public decimal? CvssRating { get; set; }
188+
public string Severity { get; set; }
169189
public IReadOnlyCollection<string> CweIds { get; set; }
170190
public string AlternatePackageId { get; set; }
171191
public string AlternatePackageVersion { get; set; }

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

Lines changed: 25 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -33,61 +33,41 @@
3333
<div class="icon-text alert alert-warning">
3434
<div id="show-deprecation-content-container" class="deprecation-expander" tabindex="0" data-toggle="collapse" data-target="#deprecation-content-container" aria-expanded="false" aria-controls="deprecation-content-container" aria-labelledby="deprecation-container-label">
3535
<div class="deprecation-expander">
36-
<div class="deprecation-expander-icon-container">
36+
<div class="deprecation-expander-container">
3737
<i class="deprecation-expander-icon ms-Icon ms-Icon--Warning" aria-hidden="true"></i>
38-
</div>
39-
40-
@{
41-
var isVulnerable = Model.DeprecationStatus.HasFlag(PackageDeprecationStatus.Vulnerable);
42-
var isLegacy = Model.DeprecationStatus.HasFlag(PackageDeprecationStatus.Legacy);
43-
var deprecationReasonString = string.Empty;
44-
if (isVulnerable || isLegacy)
45-
{
46-
deprecationReasonString += " because it ";
47-
if (isVulnerable)
48-
{
49-
deprecationReasonString += "has vulnerabilities";
50-
}
5138

52-
if (isLegacy)
53-
{
39+
<div id="deprecation-container-label" class="deprecation-expander-info-right">
40+
@{
41+
var isVulnerable = Model.DeprecationStatus.HasFlag(PackageDeprecationStatus.Vulnerable);
42+
var isLegacy = Model.DeprecationStatus.HasFlag(PackageDeprecationStatus.Legacy);
5443
if (isVulnerable)
5544
{
56-
deprecationReasonString += " and ";
57-
}
45+
@:This package has been deprecated due to <b>vulnerabilities</b>.
5846

59-
deprecationReasonString += "is no longer maintained by its owners";
47+
if (isLegacy)
48+
{
49+
<br />
50+
@:This package is also <b>legacy</b> and is no longer maintained.
51+
}
52+
}
53+
else if (isLegacy)
54+
{
55+
@:This package has been deprecated because it is <b>legacy</b> and is no longer maintained.
56+
}
57+
else
58+
{
59+
@:This package has been deprecated.
60+
}
6061
}
61-
}
62-
}
63-
<div id="deprecation-container-label" class="deprecation-expander-info-right">This package has been deprecated@(deprecationReasonString).</div>
64-
</div>
62+
</div>
63+
</div>
6564

66-
<div class="deprecation-expander">
67-
@if (Model.CvssRating.HasValue)
68-
{
69-
string ratingString;
70-
if (Model.CvssRating < 4)
71-
{
72-
ratingString = "Low";
73-
}
74-
else if (Model.CvssRating < 7)
65+
<div class="deprecation-expander-container">
66+
@if (Model.Severity != null)
7567
{
76-
ratingString = "Medium";
68+
<div class="deprecation-expander-info-right deprecation-expander-container">Severity: <b class="deprecation-expander-severity-rating">@Model.Severity</b></div>
7769
}
78-
else if (Model.CvssRating < 9)
79-
{
80-
ratingString = "High";
81-
}
82-
else
83-
{
84-
ratingString = "Critical";
85-
}
86-
87-
<div class="deprecation-expander-info-right">CVSS rating: <b>@ratingString</b></div>
88-
}
8970

90-
<div class="deprecation-expander-icon-container">
9171
<i id="deprecation-expander-icon-right" class="deprecation-expander-icon deprecation-expander-info-right ms-Icon ms-Icon--ChevronDown" aria-hidden="true"></i>
9272
</div>
9373
</div>

‎tests/NuGetGallery.Facts/ViewModels/DisplayPackageViewModelFacts.cs‎

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -736,9 +736,19 @@ public void ReturnsExpectedUser(Package package, User currentUser, string expect
736736
}
737737
}
738738

739+
public enum SeverityString_State
740+
{
741+
None,
742+
Low,
743+
Medium,
744+
High,
745+
Critical
746+
}
747+
739748
public static IEnumerable<object[]> DeprecationFieldsAreSetAsExpected_Data =
740749
MemberDataHelper.Combine(
741750
MemberDataHelper.FlagEnumDataSet<PackageDeprecationStatus>(),
751+
MemberDataHelper.EnumDataSet<SeverityString_State>(),
742752
MemberDataHelper.BooleanDataSet(),
743753
MemberDataHelper.BooleanDataSet(),
744754
MemberDataHelper.BooleanDataSet(),
@@ -748,16 +758,39 @@ public void ReturnsExpectedUser(Package package, User currentUser, string expect
748758
[MemberData(nameof(DeprecationFieldsAreSetAsExpected_Data))]
749759
public void DeprecationFieldsAreSetAsExpected(
750760
PackageDeprecationStatus status,
761+
SeverityString_State severity,
751762
bool hasCves,
752763
bool hasCwes,
753764
bool hasAlternateRegistration,
754765
bool hasAlternatePackage)
755766
{
756767
// Arrange
768+
decimal? cvss;
769+
switch (severity)
770+
{
771+
case SeverityString_State.Critical:
772+
cvss = (decimal)9.5;
773+
break;
774+
case SeverityString_State.High:
775+
cvss = (decimal)7.5;
776+
break;
777+
case SeverityString_State.Medium:
778+
cvss = (decimal)5;
779+
break;
780+
case SeverityString_State.Low:
781+
cvss = (decimal)2;
782+
break;
783+
case SeverityString_State.None:
784+
cvss = null;
785+
break;
786+
default:
787+
throw new ArgumentOutOfRangeException(nameof(severity));
788+
}
789+
757790
var deprecation = new PackageDeprecation
758791
{
759792
Status = status,
760-
CvssRating = (decimal)5.5,
793+
CvssRating = cvss,
761794
CustomMessage = "hello"
762795
};
763796

@@ -825,9 +858,32 @@ public void DeprecationFieldsAreSetAsExpected(
825858

826859
// Assert
827860
Assert.Equal(status, model.DeprecationStatus);
828-
Assert.Equal(deprecation.CvssRating, model.CvssRating);
829861
Assert.Equal(deprecation.CustomMessage, model.CustomMessage);
830862

863+
string expectedString;
864+
switch (severity)
865+
{
866+
case SeverityString_State.Critical:
867+
expectedString = "Critical";
868+
break;
869+
case SeverityString_State.High:
870+
expectedString = "High";
871+
break;
872+
case SeverityString_State.Medium:
873+
expectedString = "Medium";
874+
break;
875+
case SeverityString_State.Low:
876+
expectedString = "Low";
877+
break;
878+
case SeverityString_State.None:
879+
expectedString = null;
880+
break;
881+
default:
882+
throw new ArgumentOutOfRangeException(nameof(severity));
883+
}
884+
885+
Assert.Equal(expectedString, model.Severity);
886+
831887
if (hasCves)
832888
{
833889
Assert.Equal(cveIds, model.CveIds);

0 commit comments

Comments
 (0)