Add Deprecation section of the Manage Package page #6912

scottbommarito · 2019-02-21T02:00:41Z

Future PRs:

CVE/CWE autocomplete
tooltips (the (i) icons in the spec)

Tons of screenshots below:

scottbommarito · 2019-02-21T02:05:29Z

src/Bootstrap/less/theme/base.less

+  // Bootstrap doesn't set the container width when the screen is xs.
+  // We want the container width to be the width of the viewport on xs screens.
+  // Otherwise, elements that depend on the width of the container could behave oddly on xs screens.
+  .container {


For more context--I had to add this to fix a UI bug with the multi-select dropdown on XS screens. The dropdown's button text is the list of every selected version, and when this list is greater than can display in the button, it relied on the container's width to properly overflow. At SM widths and above, the container width is set by Bootstrap, but at XS and smaller, it is set to "auto", so its width becomes equal to the longest item inside it, which caused it to take the width of the un-overflowed dropdown button. By setting the XS container width to the viewport's width, the dropdown button would then properly restrict itself to the size of the viewport.

scottbommarito · 2019-02-21T02:21:56Z

src/NuGetGallery/Controllers/PackagesController.cs


-            model.VersionSelectList = new SelectList(


For more context, I made this change because I discovered that our use of SelectList was fairly unclean.

SelectList provides nothing other than a way to construct a IEnumerable<SelectListItem> using reflection. We have no reason to use it because we can easily just rewrite our code to create the IEnumerable<SelectListItem> directly. I think this approach is much cleaner because it avoids the reflection and dynamic objects we currently use.

I discovered this while updating the SelectList in ManagePackageViewModel and updated it here as well because it was the only other place we used it.

scottbommarito · 2019-02-21T02:23:56Z

src/NuGetGallery/Views/Packages/_ManageDocumentation.cshtml

-                            })
-                    </label>
-                </div>
+                <label for="input-select-readme" id="readme-version-label">Select version</label>


This change was a minor UI fix I noticed while working on the page. The select list in the documentation section is bolded because it is inside the label. None of our other select lists are bold, so I made this change to align it with the others.

src/NuGetGallery/Controllers/ManageDeprecationJsonApiController.cs

xavierdecoster · 2019-02-21T09:41:15Z

src/NuGetGallery/Controllers/PackagesController.cs

+            var cves = _deprecationService.GetCvesById(cveIds);
+            if (cveIds.Count() != cves.Count)
+            {
+                return DeprecateErrorResponse(HttpStatusCode.NotFound, Strings.DeprecatePackage_MissingCve);


Per offline discussion, we need to cover for the scenario where vulnerability data is not up-to-date.

A CVE/CWE info may not be available in the autocomplete API due to the following reasons:

Data staleness on NuGet Gallery – We refresh every 24 hours to begin with.

Data staleness on the CVE/CWE repository – This could happen too as sometimes the ID is provided upon report but the corresponding data record is absent till the vulnerability is being explored/validated.

In these cases, we should visually warn but accept the input from the package publisher

I was originally going to postpone doing this until a future PR, but I think it probably wouldn't be too much work.

After looking into it more, I will postpone this to the future CVE/CWE autocomplete integration PR to reduce the size of this one.

xavierdecoster · 2019-02-21T09:41:33Z

src/NuGetGallery/Controllers/PackagesController.cs

+            var cwes = _deprecationService.GetCwesById(cweIds);
+            if (cweIds.Count() != cwes.Count)
+            {
+                return DeprecateErrorResponse(HttpStatusCode.NotFound, Strings.DeprecatePackage_MissingCwe);


Will do in the PR that integrates your CVE/CWE autocomplete work with this form.

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

joelverhagen · 2019-02-21T16:46:58Z

src/NuGetGallery/Controllers/ManageDeprecationJsonApiController.cs

+                .Where(p => p.PackageStatusKey == PackageStatus.Available)
+                .ToList()
+                .OrderByDescending(p => NuGetVersion.Parse(p.Version))
+                .Select(p => NuGetVersionFormatter.ToFullString(p.Version));


This is parsing twice, right? You should be able to both sort and get a full version string from NuGetVersion.

Also, why full version? Does this look reasonable with, say, the build metadata that client team puts on their packages?

Rewrote it to only parse once.

We use full version in all of our other version select lists, so I chose it here to be consistent. I don't mind making it normalized, but I feel like more information is better. I don't think it would look reasonable with how the client uses build metadata, but I would prefer to see how other users use build metadata as well before changing this.

src/NuGetGallery/Controllers/PackagesController.cs

src/NuGetGallery/Helpers/PackageHelper.cs

shishirx34

This is one gigantic PR. It is very difficult to thoroughly review so much changes.

Why not create separate PRs with smaller changes? Just from overview I would break this PR into

Deprecation Service
Feature flag
CSS/view changes.

src/NuGetGallery/App_Start/AppActivator.cs

src/NuGetGallery/Controllers/ManageDeprecationJsonApiController.cs

shishirx34 · 2019-02-23T00:30:33Z

src/NuGetGallery/Controllers/PackagesController.cs

+                    Text = PackageHelper.GetSelectListText(versionWithSymbols),
+                    Value = Url.DeleteSymbolsPackage(new TrivialPackageVersionModel(versionWithSymbols)),
+                    Selected = package == versionWithSymbols
+                });


Please ensure the delete symbols work flow runs fine after this change.

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

agr · 2019-02-25T21:41:23Z

src/NuGetGallery/Scripts/gallery/common-multi-select-dropdown.js

+        }
+    });
+
+    this.escapeKeyCode = 27;


Javascript has constants, if I am not mistaken. Why do we need a property?

Does it? I can't find the constant online...

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/const ?

Oh I see, I thought you meant that there was a constant for the escape keycode built into the language. I'll look into consts.

src/NuGetGallery/UrlHelperExtensions.cs

src/NuGetGallery/Controllers/PackagesController.cs

src/NuGetGallery/Scripts/gallery/common-multi-select-dropdown.js

loic-sharma · 2019-02-26T18:05:57Z

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

+        self.addId('');
+    };
+    this.addKeyDown = function (data, event) {
+        if (event.which === 13) { /* Enter */


13 [](start = 28, length = 2)

Consider adding a keycode like the escape key on the dropdown page

tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs

src/NuGetGallery/Controllers/ManageDeprecationJsonApiController.cs

Scott Bommarito added 7 commits February 20, 2019 12:54

starting

6078cbf

align with old pr

0ac33fe

minor ui fixes

d4ca262

fix awful CSHTML autoformat shenanigans

3c4b11f

add search icon

7fc111c

minor refactoring

f9eb54b

PackageHelper.GetSelectListText

024a725

scottbommarito requested review from joelverhagen, agr, loic-sharma, xavierdecoster, shishirx34, nugetlurker, ryuyu, chenriksson, cristinamanum, skofman1 and zhhyu February 21, 2019 02:00

scottbommarito commented Feb 21, 2019

View reviewed changes

use ientityrepository for cve/cwe

0852625

scottbommarito commented Feb 21, 2019

View reviewed changes

xavierdecoster reviewed Feb 21, 2019

View reviewed changes

Scott Bommarito added 2 commits February 21, 2019 08:08

fix some other tests

90f3c9c

add permissions check to deprecate endpoint

a6017bb

joelverhagen reviewed Feb 21, 2019

View reviewed changes

src/Bootstrap/less/theme/page-manage-deprecation.less Outdated Show resolved Hide resolved

joelverhagen reviewed Feb 21, 2019

View reviewed changes

src/NuGetGallery/Controllers/PackagesController.cs Outdated Show resolved Hide resolved

joelverhagen reviewed Feb 21, 2019

View reviewed changes

src/NuGetGallery/Controllers/PackagesController.cs Outdated Show resolved Hide resolved

joelverhagen reviewed Feb 21, 2019

View reviewed changes

src/NuGetGallery/Helpers/PackageHelper.cs Show resolved Hide resolved

fix deletesymbols lazy-loading

9875eb9

shishirx34 reviewed Feb 23, 2019

View reviewed changes

Scott Bommarito added 4 commits February 22, 2019 16:50

isnullorwhitespace for alternate packages

8365bca

getalternatepackageversions 200

c52dead

fix tests for 200

d7e092e

add feature flag test for manage

2ee63dd

agr reviewed Feb 25, 2019

View reviewed changes

src/Bootstrap/dist/css/bootstrap-theme.css Show resolved Hide resolved

agr reviewed Feb 25, 2019

View reviewed changes

src/Bootstrap/dist/css/bootstrap-theme.css Show resolved Hide resolved

agr reviewed Feb 25, 2019

View reviewed changes

Scott Bommarito added 3 commits February 25, 2019 16:03

deprecate unit tests and alert messages

ebb2345

packagedeprecationservicefacts

a793d50

remove escape key code property

94d114f

loic-sharma reviewed Feb 26, 2019

View reviewed changes

src/NuGetGallery/UrlHelperExtensions.cs Outdated Show resolved Hide resolved

loic-sharma reviewed Feb 26, 2019

View reviewed changes

src/NuGetGallery/Controllers/PackagesController.cs Outdated Show resolved Hide resolved

loic-sharma reviewed Feb 26, 2019

View reviewed changes

src/NuGetGallery/Scripts/gallery/common-multi-select-dropdown.js Outdated Show resolved Hide resolved

loic-sharma reviewed Feb 26, 2019

View reviewed changes

tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs Outdated Show resolved Hide resolved

loic-sharma reviewed Feb 26, 2019

View reviewed changes

src/NuGetGallery/Controllers/ManageDeprecationJsonApiController.cs Outdated Show resolved Hide resolved

loic-sharma approved these changes Feb 26, 2019

View reviewed changes

Scott Bommarito added 5 commits February 26, 2019 13:01

most loic comments

73e4469

reset submit error when form changes

f7ac5cd

revert deletesymbols changes

a2242a0

cvss test fix

7d47773

reduce data set of ReturnsSuccessful

0c35c21

This was referenced Feb 26, 2019

Add tooltips and "read more" links to the manage package deprecation form #6936

Closed

Minor fixes to the GET DeleteSymbols endpoint #6937

Merged

scottbommarito merged commit bfd3cae into dev Feb 26, 2019

scottbommarito deleted the sb-depform branch February 26, 2019 22:11

scottbommarito restored the sb-depform branch July 22, 2019 23:21

scottbommarito deleted the sb-depform branch July 22, 2019 23:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Deprecation section of the Manage Package page #6912

Add Deprecation section of the Manage Package page #6912

scottbommarito commented Feb 21, 2019 •

edited

Loading

scottbommarito Feb 21, 2019

scottbommarito Feb 21, 2019 •

edited

Loading

scottbommarito Feb 21, 2019

xavierdecoster Feb 21, 2019

scottbommarito Feb 21, 2019

scottbommarito Feb 21, 2019

xavierdecoster Feb 21, 2019

scottbommarito Feb 21, 2019

joelverhagen Feb 21, 2019

scottbommarito Feb 21, 2019

shishirx34 left a comment

shishirx34 Feb 23, 2019

scottbommarito Feb 23, 2019

agr Feb 25, 2019

scottbommarito Feb 25, 2019

agr Feb 25, 2019

scottbommarito Feb 26, 2019

loic-sharma Feb 26, 2019

Add Deprecation section of the Manage Package page #6912

Add Deprecation section of the Manage Package page #6912

Conversation

scottbommarito commented Feb 21, 2019 • edited Loading

Choose a reason for hiding this comment

scottbommarito Feb 21, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shishirx34 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scottbommarito commented Feb 21, 2019 •

edited

Loading

scottbommarito Feb 21, 2019 •

edited

Loading