Skip to content

Commit 2ae4229

Browse files
xnixni
committed
Add deprecation alternative (#4626)
1 parent 87af637 commit 2ae4229

File tree

5 files changed

+187
-7
lines changed

5 files changed

+187
-7
lines changed

‎tests/unit/admin/views/test_classifiers.py‎

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,9 @@ def test_add_parent_classifier(self, db_request):
8383

8484
class TestDeprecateClassifier:
8585
def test_deprecate_classifier(self, db_request):
86-
classifier = ClassifierFactory(deprecated=False)
86+
classifier = ClassifierFactory(
87+
classifier="Classifier :: For Testing", deprecated=False
88+
)
8789

8890
db_request.params = {"classifier_id": classifier.id}
8991
db_request.session.flash = pretend.call_recorder(lambda *a, **kw: None)
@@ -93,3 +95,83 @@ def test_deprecate_classifier(self, db_request):
9395
db_request.db.flush()
9496

9597
assert classifier.deprecated
98+
assert db_request.session.flash.calls == [
99+
pretend.call(
100+
"Deprecated classifier 'Classifier :: For Testing'", queue="success"
101+
)
102+
]
103+
104+
def test_deprecation_with_alternative(self, db_request):
105+
classifier = ClassifierFactory(
106+
classifier="Classifier :: For tSeting", deprecated=False
107+
)
108+
alternative = ClassifierFactory(
109+
classifier="Classifier :: For Testing", deprecated=False
110+
)
111+
db_request.params = {
112+
"classifier_id": classifier.id,
113+
"deprecated_by": alternative.id,
114+
}
115+
116+
db_request.session.flash = pretend.call_recorder(lambda *a, **kw: None)
117+
db_request.route_path = lambda *a: "/the/path"
118+
119+
views.deprecate_classifier(db_request)
120+
db_request.db.flush()
121+
122+
assert classifier.deprecated
123+
assert db_request.session.flash.calls == [
124+
pretend.call(
125+
(
126+
"Deprecated classifier 'Classifier :: For tSeting' "
127+
"in favour of 'Classifier :: For Testing'"
128+
),
129+
queue="success",
130+
)
131+
]
132+
133+
def test_self_deprecation(self, db_request):
134+
classifier = ClassifierFactory(deprecated=False)
135+
db_request.params = {
136+
"classifier_id": classifier.id,
137+
"deprecated_by": classifier.id,
138+
}
139+
140+
db_request.session.flash = pretend.call_recorder(lambda *a, **kw: None)
141+
db_request.route_path = lambda *a: "/the/path"
142+
143+
views.deprecate_classifier(db_request)
144+
db_request.db.flush()
145+
146+
assert not classifier.deprecated
147+
assert db_request.session.flash.calls == [
148+
pretend.call(
149+
"You can not deprecate the classifier in favour of itself",
150+
queue="error",
151+
)
152+
]
153+
154+
def test_deprecation_in_favour_of_deprecated(self, db_request):
155+
classifier = ClassifierFactory(deprecated=False)
156+
alternative = ClassifierFactory(deprecated=True)
157+
db_request.params = {
158+
"classifier_id": classifier.id,
159+
"deprecated_by": alternative.id,
160+
}
161+
162+
db_request.session.flash = pretend.call_recorder(lambda *a, **kw: None)
163+
db_request.route_path = lambda *a: "/the/path"
164+
165+
views.deprecate_classifier(db_request)
166+
db_request.db.flush()
167+
168+
assert not classifier.deprecated
169+
assert db_request.session.flash.calls == [
170+
pretend.call(
171+
(
172+
"You can not deprecate the classifier in favour of "
173+
"already deprecated classifier"
174+
),
175+
queue="error",
176+
)
177+
]

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,16 @@ <h3 class="box-title">Deprecate Classifier</h3>
9595
</option>
9696
{% endfor %}
9797
</select>
98+
<label for="deprecated_by">Deprecated in favour of (optional):</label>
99+
<select name="deprecated_by" id="deprecated_by">
100+
<option selected>(Empty)</option>
101+
{% for classifier in classifiers %}
102+
<!-- TODO: mark currently selected package as disabled -->
103+
<option value="{{ classifier.id }}" {{'disabled' if classifier.deprecated else ''}}>
104+
{{ classifier.classifier }}{{ ' (Deprecated)' if classifier.deprecated else ''}}
105+
</option>
106+
{% endfor %}
107+
</select>
98108
</div>
99109
</div>
100110
<div class="box-footer">

‎warehouse/admin/views/classifiers.py‎

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,39 @@ def add_child_classifier(self):
9494
require_csrf=True,
9595
)
9696
def deprecate_classifier(request):
97-
classifier = request.db.query(Classifier).get(request.params.get("classifier_id"))
97+
deprecated_classifier_id = request.params.get("classifier_id")
98+
alternative_classifier_id = request.params.get("deprecated_by")
9899

99-
classifier.deprecated = True
100+
if deprecated_classifier_id == alternative_classifier_id:
101+
message = f"You can not deprecate the classifier in favour of itself"
102+
request.session.flash(message, queue="error")
103+
return HTTPSeeOther(request.route_path("admin.classifiers"))
100104

101-
request.session.flash(
102-
f"Deprecated classifier {classifier.classifier!r}", queue="success"
103-
)
105+
if alternative_classifier_id:
106+
alternative_classifier = request.db.query(Classifier).get(
107+
alternative_classifier_id
108+
)
109+
if alternative_classifier.deprecated:
110+
# It is not a 100% protection from circular dependencies, because if
111+
# exactly at the same moment one admin will deprecate classifier A
112+
# in favour of classifier B, and another admin will deprecate classifier B
113+
# in favour of classifier A, this will create a circular dependency.
114+
message = (
115+
f"You can not deprecate the classifier "
116+
f"in favour of already deprecated classifier"
117+
)
118+
request.session.flash(message, queue="error")
119+
return HTTPSeeOther(request.route_path("admin.classifiers"))
120+
121+
deprecated_classifier = request.db.query(Classifier).get(deprecated_classifier_id)
122+
123+
deprecated_classifier.deprecated = True
124+
deprecated_classifier.deprecated_by = alternative_classifier_id
125+
126+
message = f"Deprecated classifier {deprecated_classifier.classifier!r}"
127+
if alternative_classifier_id:
128+
message += f" in favour of {alternative_classifier.classifier!r}"
129+
130+
request.session.flash(message, queue="success")
104131

105132
return HTTPSeeOther(request.route_path("admin.classifiers"))

‎warehouse/classifiers/models.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
# See the License for the specific language governing permissions and
1111
# limitations under the License.
1212

13-
from sqlalchemy import Boolean, Column, Integer, Text, sql
13+
from sqlalchemy import Boolean, Column, ForeignKey, Integer, Text, sql
1414

1515
from warehouse import db
1616
from warehouse.utils.attrs import make_repr
@@ -25,6 +25,9 @@ class Classifier(db.ModelBase):
2525
id = Column(Integer, primary_key=True, nullable=False)
2626
classifier = Column(Text, unique=True)
2727
deprecated = Column(Boolean, nullable=False, server_default=sql.false())
28+
deprecated_by = Column(
29+
ForeignKey(f"{__tablename__}.id", onupdate="CASCADE", ondelete="SET NULL")
30+
)
2831
l2 = Column(Integer)
2932
l3 = Column(Integer)
3033
l4 = Column(Integer)

‎warehouse/migrations/versions/e4cc4f905659_add_deprecated_by_column_for_classifiers.py‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
# Licensed under the Apache License, Version 2.0 (the "License");
2+
# you may not use this file except in compliance with the License.
3+
# You may obtain a copy of the License at
4+
#
5+
# http://www.apache.org/licenses/LICENSE-2.0
6+
#
7+
# Unless required by applicable law or agreed to in writing, software
8+
# distributed under the License is distributed on an "AS IS" BASIS,
9+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
10+
# See the License for the specific language governing permissions and
11+
# limitations under the License.
12+
"""
13+
add deprecated_by column for classifiers
14+
15+
Revision ID: e4cc4f905659
16+
Revises: e82c3a017d60
17+
Create Date: 2018-10-27 13:30:20.984033
18+
"""
19+
20+
from alembic import op
21+
import sqlalchemy as sa
22+
23+
24+
revision = "e4cc4f905659"
25+
down_revision = "e82c3a017d60"
26+
27+
# Note: It is VERY important to ensure that a migration does not lock for a
28+
# long period of time and to ensure that each individual migration does
29+
# not break compatibility with the *previous* version of the code base.
30+
# This is because the migrations will be ran automatically as part of the
31+
# deployment process, but while the previous version of the code is still
32+
# up and running. Thus backwards incompatible changes must be broken up
33+
# over multiple migrations inside of multiple pull requests in order to
34+
# phase them in over multiple deploys.
35+
36+
37+
def upgrade():
38+
# ### commands auto generated by Alembic - please adjust! ###
39+
op.add_column(
40+
"trove_classifiers", sa.Column("deprecated_by", sa.Integer(), nullable=True)
41+
)
42+
op.create_foreign_key(
43+
None,
44+
"trove_classifiers",
45+
"trove_classifiers",
46+
["deprecated_by"],
47+
["id"],
48+
onupdate="CASCADE",
49+
ondelete="SET NULL",
50+
)
51+
# ### end Alembic commands ###
52+
53+
54+
def downgrade():
55+
# ### commands auto generated by Alembic - please adjust! ###
56+
op.drop_constraint(None, "trove_classifiers", type_="foreignkey")
57+
op.drop_column("trove_classifiers", "deprecated_by")
58+
# ### end Alembic commands ###

0 commit comments

Comments
 (0)