Skip to content

Commit e62d613

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 3f0d4e0 commit e62d613

File tree

9 files changed

+355
-1
lines changed

9 files changed

+355
-1
lines changed

tests/unit/manage/test_views.py

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2135,7 +2135,7 @@ def test_delete_project_disallow_deletion(self):
21352135
pretend.call("manage.project.settings", project_name="foo")
21362136
]
21372137

2138-
def test_delete_project(self, db_request):
2138+
def test_delete_project(self, monkeypatch, db_request):
21392139
project = ProjectFactory.create(name="foo")
21402140

21412141
db_request.route_path = pretend.call_recorder(lambda *a, **kw: "/the-redirect")
@@ -2144,6 +2144,21 @@ def test_delete_project(self, db_request):
21442144
)
21452145
db_request.POST["confirm_project_name"] = project.normalized_name
21462146
db_request.user = UserFactory.create()
2147+
2148+
get_user_role_in_project = pretend.call_recorder(
2149+
lambda project, username, req: "Owner"
2150+
)
2151+
monkeypatch.setattr(views, "get_user_role_in_project", get_user_role_in_project)
2152+
get_project_contributors = pretend.call_recorder(
2153+
lambda project, req: [db_request.user]
2154+
)
2155+
monkeypatch.setattr(views, "get_project_contributors", get_project_contributors)
2156+
2157+
send_removed_project_email = pretend.call_recorder(lambda req, user, **k: None)
2158+
monkeypatch.setattr(
2159+
views, "send_removed_project_email", send_removed_project_email
2160+
)
2161+
21472162
db_request.remote_addr = "192.168.1.1"
21482163

21492164
result = views.delete_project(project, db_request)
@@ -2154,6 +2169,21 @@ def test_delete_project(self, db_request):
21542169
assert db_request.route_path.calls == [pretend.call("manage.projects")]
21552170
assert isinstance(result, HTTPSeeOther)
21562171
assert result.headers["Location"] == "/the-redirect"
2172+
assert get_user_role_in_project.calls == [
2173+
pretend.call(project, db_request.user.username, db_request,),
2174+
pretend.call(project, db_request.user.username, db_request,),
2175+
]
2176+
assert get_project_contributors.calls == [pretend.call(project, db_request,)]
2177+
assert send_removed_project_email.calls == [
2178+
pretend.call(
2179+
db_request,
2180+
db_request.user,
2181+
project_name=project.name,
2182+
submitter_name=db_request.user.username,
2183+
submitter_role="Owner",
2184+
recipient_role="Owner",
2185+
)
2186+
]
21572187
assert not (db_request.db.query(Project).filter(Project.name == "foo").count())
21582188

21592189

@@ -2320,6 +2350,7 @@ def test_delete_project_release(self, monkeypatch):
23202350
project=pretend.stub(
23212351
name="foobar", record_event=pretend.call_recorder(lambda *a, **kw: None)
23222352
),
2353+
created=datetime.datetime(2017, 2, 5, 17, 18, 18, 462_634),
23232354
)
23242355
request = pretend.stub(
23252356
POST={"confirm_version": release.version},
@@ -2336,7 +2367,25 @@ def test_delete_project_release(self, monkeypatch):
23362367
)
23372368
journal_obj = pretend.stub()
23382369
journal_cls = pretend.call_recorder(lambda **kw: journal_obj)
2370+
2371+
get_user_role_in_project = pretend.call_recorder(
2372+
lambda project, username, req: "Owner"
2373+
)
2374+
monkeypatch.setattr(views, "get_user_role_in_project", get_user_role_in_project)
2375+
get_project_contributors = pretend.call_recorder(
2376+
lambda project, request: [request.user]
2377+
)
2378+
monkeypatch.setattr(views, "get_project_contributors", get_project_contributors)
2379+
23392380
monkeypatch.setattr(views, "JournalEntry", journal_cls)
2381+
send_removed_project_release_email = pretend.call_recorder(
2382+
lambda req, contrib, **k: None
2383+
)
2384+
monkeypatch.setattr(
2385+
views,
2386+
"send_removed_project_release_email",
2387+
send_removed_project_release_email,
2388+
)
23402389

23412390
view = views.ManageProjectRelease(release, request)
23422391

@@ -2345,6 +2394,25 @@ def test_delete_project_release(self, monkeypatch):
23452394
assert isinstance(result, HTTPSeeOther)
23462395
assert result.headers["Location"] == "/the-redirect"
23472396

2397+
assert get_user_role_in_project.calls == [
2398+
pretend.call(release.project, request.user.username, request,),
2399+
pretend.call(release.project, request.user.username, request,),
2400+
]
2401+
assert get_project_contributors.calls == [
2402+
pretend.call(release.project, request,)
2403+
]
2404+
2405+
assert send_removed_project_release_email.calls == [
2406+
pretend.call(
2407+
request,
2408+
request.user,
2409+
release=release.version,
2410+
submitter_name=request.user.username,
2411+
submitter_role="Owner",
2412+
recipient_role="Owner",
2413+
)
2414+
]
2415+
23482416
assert request.db.delete.calls == [pretend.call(release)]
23492417
assert request.db.add.calls == [pretend.call(journal_obj)]
23502418
assert request.flags.enabled.calls == [

warehouse/email/__init__.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,40 @@ def send_two_factor_removed_email(request, user, method):
213213
return {"method": pretty_methods[method], "username": user.username}
214214

215215

216+
@_email("removed-project")
217+
def send_removed_project_email(
218+
request, user, *, project_name, submitter_name, submitter_role, recipient_role
219+
):
220+
recipient_role_descr = "an owner"
221+
if recipient_role == "Maintainer":
222+
recipient_role_descr = "a maintainer"
223+
224+
return {
225+
"project": project_name,
226+
"submitter": submitter_name,
227+
"submitter_role": submitter_role,
228+
"recipient_role_descr": recipient_role_descr,
229+
}
230+
231+
232+
@_email("removed-project-release")
233+
def send_removed_project_release_email(
234+
request, user, *, release, submitter_name, submitter_role, recipient_role
235+
):
236+
recipient_role_descr = "an owner"
237+
if recipient_role == "Maintainer":
238+
recipient_role_descr = "a maintainer"
239+
240+
return {
241+
"project": release.project.name,
242+
"release": release,
243+
"release_date": release.created.strftime("%b %d %Y"),
244+
"submitter": submitter_name,
245+
"submitter_role": submitter_role,
246+
"recipient_role_descr": recipient_role_descr,
247+
}
248+
249+
216250
def includeme(config):
217251
email_sending_class = config.maybe_dotted(config.registry.settings["mail.backend"])
218252
config.register_service_factory(email_sending_class.create_service, IEmailSender)

warehouse/manage/views.py

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@
4040
send_primary_email_change_email,
4141
send_two_factor_added_email,
4242
send_two_factor_removed_email,
43+
send_removed_project_email,
44+
send_removed_project_release_email,
4345
)
4446
from warehouse.i18n import localize as _
4547
from warehouse.macaroons.interfaces import IMacaroonService
@@ -812,6 +814,40 @@ def manage_project_settings(project, request):
812814
return {"project": project}
813815

814816

817+
def get_project_contributors(project, request):
818+
query_res = (
819+
request.db.query(Project)
820+
.join(User, Project.users)
821+
.filter(Project.name == project.name)
822+
.one()
823+
)
824+
return query_res.users
825+
826+
827+
def get_user_role_in_project(project, username, request):
828+
query_res = (
829+
request.db.query(Project)
830+
.join(User, Project.users)
831+
.filter(User.username == username, Project.name == project.name)
832+
.with_entities(Role.role_name)
833+
.distinct(Role.role_name)
834+
.all()
835+
)
836+
837+
user_role = ""
838+
# This check is needed because of
839+
# issue https://github.com/pypa/warehouse/issues/2745
840+
# which is not yet resolved and a user could be an owner
841+
# and a maintainer at the same time
842+
if len(query_res) == 2 and (
843+
query_res[0].role_name == "Owner" or query_res[1].role_name == "Owner"
844+
):
845+
user_role = "Owner"
846+
else:
847+
user_role = "Maintainer"
848+
return user_role
849+
850+
815851
@view_config(
816852
route_name="manage.project.delete_project",
817853
context=Project,
@@ -834,6 +870,24 @@ def delete_project(project, request):
834870
)
835871

836872
confirm_project(project, request, fail_route="manage.project.settings")
873+
874+
submitter_role = get_user_role_in_project(project, request.user.username, request)
875+
contributors = get_project_contributors(project, request)
876+
877+
for contributor in contributors:
878+
contributor_role = get_user_role_in_project(
879+
project, contributor.username, request
880+
)
881+
882+
send_removed_project_email(
883+
request,
884+
contributor,
885+
project_name=project.name,
886+
submitter_name=request.user.username,
887+
submitter_role=submitter_role,
888+
recipient_role=contributor_role,
889+
)
890+
837891
remove_project(project, request)
838892

839893
return HTTPSeeOther(request.route_path("manage.projects"))
@@ -966,6 +1020,11 @@ def delete_project_release(self):
9661020
)
9671021
)
9681022

1023+
submitter_role = get_user_role_in_project(
1024+
self.release.project, self.request.user.username, self.request
1025+
)
1026+
contributors = get_project_contributors(self.release.project, self.request)
1027+
9691028
self.request.db.add(
9701029
JournalEntry(
9711030
name=self.release.project.name,
@@ -991,6 +1050,20 @@ def delete_project_release(self):
9911050
f"Deleted release {self.release.version!r}", queue="success"
9921051
)
9931052

1053+
for contributor in contributors:
1054+
contributor_role = get_user_role_in_project(
1055+
self.release.project, contributor.username, self.request
1056+
)
1057+
1058+
send_removed_project_release_email(
1059+
self.request,
1060+
contributor,
1061+
release=self.release.version,
1062+
submitter_name=self.request.user.username,
1063+
submitter_role=submitter_role,
1064+
recipient_role=contributor_role,
1065+
)
1066+
9941067
return HTTPSeeOther(
9951068
self.request.route_path(
9961069
"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-project-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 project.
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 project.
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 %}
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-project-details">
25+
<li>{% trans project=project %}The project {{ project }} 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 project.
40+
{% endtrans %}</p>
41+
42+
{% endblock %}

0 commit comments

Comments
 (0)