Deprecated classifiers (#3771)

* Add 'deprecated' column to Classifier

* Get all classifiers, filter in template

* Add UI to deprecate classifiers

* Indicate deprecated classifiers in admin UI

* Don't show deprecated classifiers in list_classifiers

* Don't show deprecated classifiers in search tree

* Add a page with a list of classifiers

* Add copy to clipboard link

* Validate deprecated classifiers on upload

Author: di

tests/unit/admin/test_routes.py

@@ -101,6 +101,11 @@ def test_includeme():
             '/admin/classifiers/add/',
             domain=warehouse,
         ),
+        pretend.call(
+            'admin.classifiers.deprecate',
+            '/admin/classifiers/deprecate/',
+            domain=warehouse,
+        ),
         pretend.call(
             "admin.blacklist.list",
             "/admin/blacklist/",

tests/unit/admin/views/test_classifiers.py

@@ -22,9 +22,8 @@
 class TestGetClassifiers:
 
     def test_get_classifiers(self, db_request):
-        classifier_a = ClassifierFactory(l5=0, classifier='I am first')
-        classifier_b = ClassifierFactory(l5=0, classifier='I am last')
-        ClassifierFactory(l5=1)  # Ignored because it has a nonzero L5
+        classifier_a = ClassifierFactory(classifier='I am first')
+        classifier_b = ClassifierFactory(classifier='I am last')
 
         assert views.get_classifiers(db_request) == {
             'classifiers': [classifier_a, classifier_b],
@@ -89,3 +88,18 @@ def test_add_parent_classifier(self, db_request):
         assert new.l3 == 0
         assert new.l4 == 0
         assert new.l5 == 0
+
+
+class TestDeprecateClassifier:
+
+    def test_deprecate_classifier(self, db_request):
+        classifier = ClassifierFactory(deprecated=False)
+
+        db_request.params = {'classifier_id': classifier.id}
+        db_request.session.flash = pretend.call_recorder(lambda *a, **kw: None)
+        db_request.route_path = lambda *a: '/the/path'
+
+        views.deprecate_classifier(db_request)
+        db_request.db.flush()
+
+        assert classifier.deprecated

tests/unit/forklift/test_legacy.py

@@ -39,6 +39,7 @@
 from warehouse.admin.flags import AdminFlag
 
 from ...common.db.accounts import UserFactory, EmailFactory
+from ...common.db.classifiers import ClassifierFactory
 from ...common.db.packaging import (
     ProjectFactory, ReleaseFactory, FileFactory, RoleFactory,
 )
@@ -326,6 +327,31 @@ def test_validate_description_content_type_invalid(self, data):
         with pytest.raises(ValidationError):
             legacy._validate_description_content_type(form, field)
 
+    def test_validate_no_deprecated_classifiers_valid(self, db_request):
+        valid_classifier = ClassifierFactory(deprecated=False)
+        validator = legacy._no_deprecated_classifiers(db_request)
+
+        form = pretend.stub()
+        field = pretend.stub(data=[valid_classifier.classifier])
+
+        validator(form, field)
+
+    def test_validate_no_deprecated_classifiers_invalid(self, db_request):
+        deprecated_classifier = ClassifierFactory(
+            classifier='AA: BB', deprecated=True,
+        )
+        validator = legacy._no_deprecated_classifiers(db_request)
+        db_request.registry = pretend.stub(
+            settings={'warehouse.domain': 'host'}
+        )
+        db_request.route_url = pretend.call_recorder(lambda *a, **kw: '/url')
+
+        form = pretend.stub()
+        field = pretend.stub(data=[deprecated_classifier.classifier])
+
+        with pytest.raises(ValidationError):
+            validator(form, field)
+
 
 def test_construct_dependencies():
     types = {
@@ -1522,6 +1548,48 @@ def test_upload_fails_with_invalid_classifier(self, pyramid_config,
             "for this field"
         )
 
+    def test_upload_fails_with_deprecated_classifier(
+            self, pyramid_config, db_request):
+        pyramid_config.testing_securitypolicy(userid=1)
+
+        user = UserFactory.create()
+        EmailFactory.create(user=user)
+        project = ProjectFactory.create()
+        release = ReleaseFactory.create(project=project, version="1.0")
+        RoleFactory.create(user=user, project=project)
+        classifier = ClassifierFactory(classifier='AA :: BB', deprecated=True)
+
+        filename = "{}-{}.tar.gz".format(project.name, release.version)
+
+        db_request.POST = MultiDict({
+            "metadata_version": "1.2",
+            "name": project.name,
+            "version": release.version,
+            "filetype": "sdist",
+            "md5_digest": "335c476dc930b959dda9ec82bd65ef19",
+            "content": pretend.stub(
+                filename=filename,
+                file=io.BytesIO(b"A fake file."),
+                type="application/tar",
+            ),
+        })
+        db_request.POST.extend([
+            ("classifiers", classifier.classifier),
+        ])
+        db_request.route_url = pretend.call_recorder(lambda *a, **kw: '/url')
+
+        with pytest.raises(HTTPBadRequest) as excinfo:
+            legacy.file_upload(db_request)
+
+        resp = excinfo.value
+
+        assert resp.status_code == 400
+        assert resp.status == (
+            "400 Invalid value for classifiers. "
+            "Error: Classifier 'AA :: BB' has been deprecated, see /url "
+            "for a list of valid classifiers."
+        )
+
     @pytest.mark.parametrize(
         "digests",
         [

tests/unit/test_routes.py

@@ -114,6 +114,7 @@ def add_policy(name, filename):
             traverse="/{username}",
             domain=warehouse,
         ),
+        pretend.call("classifiers", "/classifiers/", domain=warehouse),
         pretend.call("search", "/search/", domain=warehouse),
         pretend.call(
             "accounts.profile",

tests/unit/test_views.py

@@ -23,9 +23,9 @@
 
 from warehouse import views
 from warehouse.views import (
-    SEARCH_BOOSTS, SEARCH_FIELDS, current_user_indicator, forbidden, health,
-    httpexception_view, index, robotstxt, opensearchxml, search, force_status,
-    flash_messages, forbidden_include
+    SEARCH_BOOSTS, SEARCH_FIELDS, classifiers, current_user_indicator,
+    forbidden, health, httpexception_view, index, robotstxt, opensearchxml,
+    search, force_status, flash_messages, forbidden_include
 )
 
 from ..common.db.accounts import UserFactory
@@ -545,6 +545,15 @@ def test_raises_400_with_pagenum_type_str(self, monkeypatch, db_request):
         assert page_cls.calls == []
 
 
+def test_classifiers(db_request):
+    classifier_a = ClassifierFactory(classifier='I am first')
+    classifier_b = ClassifierFactory(classifier='I am last')
+
+    assert classifiers(db_request) == {
+        'classifiers': [(classifier_a.classifier,), (classifier_b.classifier,)]
+    }
+
+
 def test_health():
     request = pretend.stub(
         db=pretend.stub(

warehouse/admin/routes.py

@@ -102,6 +102,11 @@ def includeme(config):
         '/admin/classifiers/add/',
         domain=warehouse,
     )
+    config.add_route(
+        'admin.classifiers.deprecate',
+        '/admin/classifiers/deprecate/',
+        domain=warehouse,
+    )
 
     # Blacklist related Admin pages
     config.add_route(

warehouse/admin/templates/admin/classifiers/index.html

@@ -56,7 +56,12 @@ <h3 class="box-title">Add Sub-Classifier</h3>
         <select name="parent_id" data-target="child-classifier.parent" data-action="child-classifier#update">
           <option disabled selected>Select one</option>
           {% for classifier in classifiers %}
-          <option value="{{ classifier.id }}">{{ classifier.classifier }}</option>
+          {% if classifier.l5 == 0 %}
+          {# Only show classifiers that can be parent classifiers #}
+          <option value="{{ classifier.id }}" {{'disabled' if classifier.deprecated else ''}}>
+            {{ classifier.classifier }}{{ ' (Deprecated)' if classifier.deprecated else ''}}
+          </option>
+          {% endif %}
           {% endfor %}
         </select>
         <label for="child">Child:</label>
@@ -73,27 +78,30 @@ <h3 class="box-title">Add Sub-Classifier</h3>
   </form>
 </div>
 
-{#
 <div class="box box-primary">
   <div class="box-header with-border">
     <h3 class="box-title">Deprecate Classifier</h3>
   </div>
-  <div class="box-body">
-    <select>
-      <option disabled selected>Select one</option>
-      {% for classifier in classifiers %}
-      <option value="{{ classifier.id }}">{{ classifier.classifier }}</option>
-      {% endfor %}
-    </select>
-  </div>
-  <div class="box-footer">
-    Deprecate classifier
-    <code></code>?
-    <div class="pull-right">
-      <button type="submit" class="btn btn-primary">Deprecate</button>
+  <form method="POST" action="{{ request.route_path('admin.classifiers.deprecate') }}">
+    <input name="csrf_token" type="hidden" value="{{ request.session.get_csrf_token() }}">
+    <div class="box-body">
+      <div class="form-group col-sm-4">
+        <label for="parent_id">Classifier:</label>
+        <select name="classifier_id">
+          <option disabled selected>Select one</option>
+          {% for classifier in classifiers %}
+          <option value="{{ classifier.id }}" {{'disabled' if classifier.deprecated else ''}}>
+            {{ classifier.classifier }}{{ ' (Deprecated)' if classifier.deprecated else ''}}
+          </option>
+          {% endfor %}
+        </select>
+      </div>
     </div>
-  </div>
+    <div class="box-footer">
+      <div class="pull-right">
+        <button type="submit" class="btn btn-primary">Deprecate</button>
+      </div>
+    </div>
+  </form>
 </div>
-#}
-
 {% endblock content %}

warehouse/admin/views/classifiers.py

@@ -25,7 +25,6 @@
 def get_classifiers(request):
     classifiers = (
         request.db.query(Classifier)
-        .filter(Classifier.l5 == 0)  # Can be a parent
         .order_by(Classifier.classifier)
         .all()
     )
@@ -97,3 +96,28 @@ def add_child_classifier(self):
         )
 
         return HTTPSeeOther(self.request.route_path('admin.classifiers'))
+
+
+@view_config(
+    route_name='admin.classifiers.deprecate',
+    permission='admin',
+    request_method='POST',
+    uses_session=True,
+    require_methods=False,
+    require_csrf=True,
+)
+def deprecate_classifier(request):
+    classifier = (
+        request.db
+        .query(Classifier)
+        .get(request.params.get('classifier_id'))
+    )
+
+    classifier.deprecated = True
+
+    request.session.flash(
+        f'Successfully deprecated classifier {classifier.classifier!r}',
+        queue='success',
+    )
+
+    return HTTPSeeOther(request.route_path('admin.classifiers'))

warehouse/classifiers/models.py

@@ -10,7 +10,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from sqlalchemy import Column, Index, Integer, Text
+from sqlalchemy import Boolean, Column, Index, Integer, Text, sql
 
 from warehouse import db
 from warehouse.utils.attrs import make_repr
@@ -28,6 +28,7 @@ class Classifier(db.ModelBase):
 
     id = Column(Integer, primary_key=True, nullable=False)
     classifier = Column(Text, unique=True)
+    deprecated = Column(Boolean, nullable=False, server_default=sql.false())
     l2 = Column(Integer)
     l3 = Column(Integer)
     l4 = Column(Integer)

warehouse/forklift/legacy.py

@@ -696,6 +696,32 @@ def _is_duplicate_file(db_session, filename, hashes):
     return None
 
 
+def _no_deprecated_classifiers(request):
+    deprecated_classifiers = {
+        classifier.classifier
+        for classifier in (
+            request.db.query(Classifier.classifier)
+            .filter(Classifier.deprecated.is_(True))
+            .all()
+        )
+    }
+
+    def validate_no_deprecated_classifiers(form, field):
+        invalid_classifiers = set(field.data or []) & deprecated_classifiers
+        if invalid_classifiers:
+            first_invalid_classifier = sorted(invalid_classifiers)[0]
+            host = request.registry.settings.get("warehouse.domain")
+            classifiers_url = request.route_url('classifiers', _host=host)
+
+            raise wtforms.validators.ValidationError(
+                f'Classifier {first_invalid_classifier!r} has been '
+                f'deprecated, see {classifiers_url} for a list of valid '
+                'classifiers.'
+            )
+
+    return validate_no_deprecated_classifiers
+
+
 @view_config(
     route_name="forklift.legacy.file_upload",
     uses_session=True,
@@ -757,6 +783,9 @@ def file_upload(request):
     # Validate and process the incoming metadata.
     form = MetadataForm(request.POST)
 
+    # Add a validator for deprecated classifiers
+    form.classifiers.validators.append(_no_deprecated_classifiers(request))
+
     form.classifiers.choices = [
         (c.classifier, c.classifier) for c in all_classifiers
     ]

warehouse/legacy/api/pypi.py

@@ -81,8 +81,9 @@ def forbidden_legacy(exc, request):
 def list_classifiers(request):
     classifiers = (
         request.db.query(Classifier.classifier)
-               .order_by(Classifier.classifier)
-               .all()
+        .filter(Classifier.deprecated.is_(False))
+        .order_by(Classifier.classifier)
+        .all()
     )
 
     return Response(

warehouse/migrations/versions/e0ca60b6a30b_add_deprecated_column_to_classifiers.py

@@ -0,0 +1,41 @@
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""
+Add 'deprecated' column to classifiers
+
+Revision ID: e0ca60b6a30b
+Revises: 6714f3f04f0f
+Create Date: 2018-04-18 23:24:13.009357
+"""
+
+from alembic import op
+import sqlalchemy as sa
+
+
+revision = "e0ca60b6a30b"
+down_revision = "6714f3f04f0f"
+
+
+def upgrade():
+    op.add_column(
+        "trove_classifiers",
+        sa.Column(
+            "deprecated",
+            sa.Boolean(),
+            server_default=sa.text("false"),
+            nullable=False,
+        ),
+    )
+
+
+def downgrade():
+    op.drop_column("trove_classifiers", "deprecated")