Skip to content

Commit 04c437e

Browse files
committed
Add email notication on package/release removal
Until now, where there are multiple contributors on a single the project, if one of them deletes a release or the whole project the other contributors don't get any notification, which is problematic. Connected with issue #5714. Signed-off-by: Martin Vrachev <[email protected]>
1 parent 13d2e89 commit 04c437e

File tree

9 files changed

+367
-1
lines changed

9 files changed

+367
-1
lines changed

tests/unit/manage/test_views.py

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2111,7 +2111,7 @@ def test_delete_project_disallow_deletion(self):
21112111
pretend.call("manage.project.settings", project_name="foo")
21122112
]
21132113

2114-
def test_delete_project(self, db_request):
2114+
def test_delete_project(self, monkeypatch, db_request):
21152115
project = ProjectFactory.create(name="foo")
21162116

21172117
db_request.route_path = pretend.call_recorder(lambda *a, **kw: "/the-redirect")
@@ -2120,6 +2120,21 @@ def test_delete_project(self, db_request):
21202120
)
21212121
db_request.POST["confirm_project_name"] = project.normalized_name
21222122
db_request.user = UserFactory.create()
2123+
2124+
get_user_role_in_project = pretend.call_recorder(
2125+
lambda project, username, req: "Owner"
2126+
)
2127+
monkeypatch.setattr(views, "get_user_role_in_project", get_user_role_in_project)
2128+
get_project_contributors = pretend.call_recorder(
2129+
lambda project, req: [db_request.user]
2130+
)
2131+
monkeypatch.setattr(views, "get_project_contributors", get_project_contributors)
2132+
2133+
send_removed_package_email = pretend.call_recorder(lambda req, user, **k: None)
2134+
monkeypatch.setattr(
2135+
views, "send_removed_package_email", send_removed_package_email
2136+
)
2137+
21232138
db_request.remote_addr = "192.168.1.1"
21242139

21252140
result = views.delete_project(project, db_request)
@@ -2130,6 +2145,21 @@ def test_delete_project(self, db_request):
21302145
assert db_request.route_path.calls == [pretend.call("manage.projects")]
21312146
assert isinstance(result, HTTPSeeOther)
21322147
assert result.headers["Location"] == "/the-redirect"
2148+
assert get_user_role_in_project.calls == [
2149+
pretend.call(project, db_request.user.username, db_request,),
2150+
pretend.call(project, db_request.user.username, db_request,),
2151+
]
2152+
assert get_project_contributors.calls == [pretend.call(project, db_request,)]
2153+
assert send_removed_package_email.calls == [
2154+
pretend.call(
2155+
db_request,
2156+
db_request.user,
2157+
project_name=project.name,
2158+
submitter_name=db_request.user.username,
2159+
submitter_role="Owner",
2160+
recipient_role="Owner",
2161+
)
2162+
]
21332163
assert not (db_request.db.query(Project).filter(Project.name == "foo").count())
21342164

21352165

@@ -2296,6 +2326,7 @@ def test_delete_project_release(self, monkeypatch):
22962326
project=pretend.stub(
22972327
name="foobar", record_event=pretend.call_recorder(lambda *a, **kw: None)
22982328
),
2329+
created=datetime.datetime(2017, 2, 5, 17, 18, 18, 462_634),
22992330
)
23002331
request = pretend.stub(
23012332
POST={"confirm_version": release.version},
@@ -2312,7 +2343,25 @@ def test_delete_project_release(self, monkeypatch):
23122343
)
23132344
journal_obj = pretend.stub()
23142345
journal_cls = pretend.call_recorder(lambda **kw: journal_obj)
2346+
2347+
get_user_role_in_project = pretend.call_recorder(
2348+
lambda project, username, req: "Owner"
2349+
)
2350+
monkeypatch.setattr(views, "get_user_role_in_project", get_user_role_in_project)
2351+
get_project_contributors = pretend.call_recorder(
2352+
lambda project, request: [request.user]
2353+
)
2354+
monkeypatch.setattr(views, "get_project_contributors", get_project_contributors)
2355+
23152356
monkeypatch.setattr(views, "JournalEntry", journal_cls)
2357+
send_removed_package_release_email = pretend.call_recorder(
2358+
lambda req, contrib, **k: None
2359+
)
2360+
monkeypatch.setattr(
2361+
views,
2362+
"send_removed_package_release_email",
2363+
send_removed_package_release_email,
2364+
)
23162365

23172366
view = views.ManageProjectRelease(release, request)
23182367

@@ -2321,6 +2370,27 @@ def test_delete_project_release(self, monkeypatch):
23212370
assert isinstance(result, HTTPSeeOther)
23222371
assert result.headers["Location"] == "/the-redirect"
23232372

2373+
assert get_user_role_in_project.calls == [
2374+
pretend.call(release.project, request.user.username, request,),
2375+
pretend.call(release.project, request.user.username, request,),
2376+
]
2377+
assert get_project_contributors.calls == [
2378+
pretend.call(release.project, request,)
2379+
]
2380+
2381+
assert send_removed_package_release_email.calls == [
2382+
pretend.call(
2383+
request,
2384+
request.user,
2385+
project_name=release.project.name,
2386+
release=release.version,
2387+
release_date=release.created.strftime("%b %d %Y"),
2388+
submitter_name=request.user.username,
2389+
submitter_role="Owner",
2390+
recipient_role="Owner",
2391+
)
2392+
]
2393+
23242394
assert request.db.delete.calls == [pretend.call(release)]
23252395
assert request.db.add.calls == [pretend.call(journal_obj)]
23262396
assert request.flags.enabled.calls == [

warehouse/email/__init__.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,48 @@ def send_added_as_collaborator_email(request, user, *, submitter, project_name,
201201
return {"project": project_name, "submitter": submitter.username, "role": role}
202202

203203

204+
@_email("removed-package")
205+
def send_removed_package_email(
206+
request, user, *, project_name, submitter_name, submitter_role, recipient_role
207+
):
208+
recipient_role_descr = "an owner"
209+
if recipient_role == "Maintainer":
210+
recipient_role_descr = "a maintainer"
211+
212+
return {
213+
"project": project_name,
214+
"submitter": submitter_name,
215+
"submitter_role": submitter_role,
216+
"recipient_role_descr": recipient_role_descr,
217+
}
218+
219+
220+
@_email("removed-package-release")
221+
def send_removed_package_release_email(
222+
request,
223+
user,
224+
*,
225+
project_name,
226+
release,
227+
release_date,
228+
submitter_name,
229+
submitter_role,
230+
recipient_role
231+
):
232+
recipient_role_descr = "an owner"
233+
if recipient_role == "Maintainer":
234+
recipient_role_descr = "a maintainer"
235+
236+
return {
237+
"project": project_name,
238+
"release": release,
239+
"release_date": release_date,
240+
"submitter": submitter_name,
241+
"submitter_role": submitter_role,
242+
"recipient_role_descr": recipient_role_descr,
243+
}
244+
245+
204246
def includeme(config):
205247
email_sending_class = config.maybe_dotted(config.registry.settings["mail.backend"])
206248
config.register_service_factory(email_sending_class.create_service, IEmailSender)

warehouse/manage/views.py

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@
3838
send_email_verification_email,
3939
send_password_change_email,
4040
send_primary_email_change_email,
41+
send_removed_package_email,
42+
send_removed_package_release_email,
4143
)
4244
from warehouse.i18n import localize as _
4345
from warehouse.macaroons.interfaces import IMacaroonService
@@ -799,6 +801,40 @@ def manage_project_settings(project, request):
799801
return {"project": project}
800802

801803

804+
def get_project_contributors(project, request):
805+
query_res = (
806+
request.db.query(Project)
807+
.join(User, Project.users)
808+
.filter(Project.name == project.name)
809+
.one()
810+
)
811+
return query_res.users
812+
813+
814+
def get_user_role_in_project(project, username, request):
815+
query_res = (
816+
request.db.query(Project)
817+
.join(User, Project.users)
818+
.filter(User.username == username, Project.name == project.name)
819+
.with_entities(Role.role_name)
820+
.distinct(Role.role_name)
821+
.all()
822+
)
823+
824+
user_role = ""
825+
# This check is needed because of
826+
# issue https://github.com/pypa/warehouse/issues/2745
827+
# which is not yet resolved and a user could be an owner
828+
# and a maintainer at the same time
829+
if len(query_res) == 2 and (
830+
query_res[0].role_name == "Owner" or query_res[1].role_name == "Owner"
831+
):
832+
user_role = "Owner"
833+
else:
834+
user_role = "Maintainer"
835+
return user_role
836+
837+
802838
@view_config(
803839
route_name="manage.project.delete_project",
804840
context=Project,
@@ -821,6 +857,24 @@ def delete_project(project, request):
821857
)
822858

823859
confirm_project(project, request, fail_route="manage.project.settings")
860+
861+
submitter_role = get_user_role_in_project(project, request.user.username, request)
862+
contributors = get_project_contributors(project, request)
863+
864+
for contributor in contributors:
865+
contributor_role = get_user_role_in_project(
866+
project, contributor.username, request
867+
)
868+
869+
send_removed_package_email(
870+
request,
871+
contributor,
872+
project_name=project.name,
873+
submitter_name=request.user.username,
874+
submitter_role=submitter_role,
875+
recipient_role=contributor_role,
876+
)
877+
824878
remove_project(project, request)
825879

826880
return HTTPSeeOther(request.route_path("manage.projects"))
@@ -953,6 +1007,11 @@ def delete_project_release(self):
9531007
)
9541008
)
9551009

1010+
submitter_role = get_user_role_in_project(
1011+
self.release.project, self.request.user.username, self.request
1012+
)
1013+
contributors = get_project_contributors(self.release.project, self.request)
1014+
9561015
self.request.db.add(
9571016
JournalEntry(
9581017
name=self.release.project.name,
@@ -978,6 +1037,22 @@ def delete_project_release(self):
9781037
f"Deleted release {self.release.version!r}", queue="success"
9791038
)
9801039

1040+
for contributor in contributors:
1041+
contributor_role = get_user_role_in_project(
1042+
self.release.project, contributor.username, self.request
1043+
)
1044+
1045+
send_removed_package_release_email(
1046+
self.request,
1047+
contributor,
1048+
project_name=self.release.project.name,
1049+
release=self.release.version,
1050+
release_date=self.release.created.strftime("%b %d %Y"),
1051+
submitter_name=self.request.user.username,
1052+
submitter_role=submitter_role,
1053+
recipient_role=contributor_role,
1054+
)
1055+
9811056
return HTTPSeeOther(
9821057
self.request.route_path(
9831058
"manage.project.releases", project_name=self.release.project.name
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
{#
2+
# Licensed under the Apache License, Version 2.0 (the "License");
3+
# you may not use this file except in compliance with the License.
4+
# You may obtain a copy of the License at
5+
#
6+
# http://www.apache.org/licenses/LICENSE-2.0
7+
#
8+
# Unless required by applicable law or agreed to in writing, software
9+
# distributed under the License is distributed on an "AS IS" BASIS,
10+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
# See the License for the specific language governing permissions and
12+
# limitations under the License.
13+
-#}
14+
{% extends "email/_base/body.html" %}
15+
16+
{% block extra_style %}
17+
ul.collaborator-details {
18+
list-style-type: none;
19+
}
20+
{% endblock %}
21+
22+
{% block content %}
23+
<p>
24+
<ul class="removed-package-release">
25+
<li>{% trans project=project, release=release, date=release_date %}The {{ project }} release {{ release }} released on {{ date }} has been deleted.{% endtrans %}</li>
26+
<li>{% trans submitter=submitter, role=submitter_role %}<strong>Deleted by:</strong> {{ submitter }} with a role:
27+
{{ role }}.{% endtrans %}
28+
</li>
29+
</ul>
30+
</p>
31+
32+
<p>{% trans href='mailto:[email protected]', email_address='[email protected]' %}If this was a mistake, you can email <a
33+
href="{{ href }}">{{ email_address }}</a> to communicate with the PyPI administrators.{% endtrans %}</p>
34+
{% endblock %}
35+
36+
{% block reason %}
37+
38+
<p>{% trans recipient_role_descr=recipient_role_descr %}
39+
You are receiving this because you are {{ recipient_role_descr }} of this package.
40+
{% endtrans %}</p>
41+
42+
{% endblock %}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
{#
2+
# Licensed under the Apache License, Version 2.0 (the "License");
3+
# you may not use this file except in compliance with the License.
4+
# You may obtain a copy of the License at
5+
#
6+
# http://www.apache.org/licenses/LICENSE-2.0
7+
#
8+
# Unless required by applicable law or agreed to in writing, software
9+
# distributed under the License is distributed on an "AS IS" BASIS,
10+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
# See the License for the specific language governing permissions and
12+
# limitations under the License.
13+
-#}
14+
15+
{% extends "email/_base/body.txt" %}
16+
17+
{% block content %}
18+
{% trans project=project, release=release, date=release_date %}The {{ project }} release {{ release }} released on {{ date }} has been deleted.{% endtrans %}
19+
20+
{% trans submitter=submitter, role=submitter_role %}Deleted by: {{ submitter }} with a role: {{ role }}.{% endtrans %}
21+
22+
{% trans email_address='[email protected]' %}If this was a mistake, you can email {{ email_address }} to communicate with the PyPI administrators.{% endtrans %}
23+
{% endblock %}
24+
25+
{% block reason %}
26+
{% trans recipient_role_descr=recipient_role_descr %}
27+
You are receiving this because you are {{ recipient_role_descr }} of this package.
28+
{% endtrans %}
29+
{% endblock %}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
{#
2+
# Licensed under the Apache License, Version 2.0 (the "License");
3+
# you may not use this file except in compliance with the License.
4+
# You may obtain a copy of the License at
5+
#
6+
# http://www.apache.org/licenses/LICENSE-2.0
7+
#
8+
# Unless required by applicable law or agreed to in writing, software
9+
# distributed under the License is distributed on an "AS IS" BASIS,
10+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
# See the License for the specific language governing permissions and
12+
# limitations under the License.
13+
-#}
14+
15+
{% extends "email/_base/subject.txt" %}
16+
17+
{% block content %}
18+
{% trans project=project, release=release %}The {{ project }} release {{ release }} has been deleted.{% endtrans %}{% endblock %}

0 commit comments

Comments
 (0)