Skip to content

Commit 8939292

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 e462bba commit 8939292

File tree

10 files changed

+831
-1
lines changed

10 files changed

+831
-1
lines changed

tests/unit/email/test_init.py

Lines changed: 398 additions & 0 deletions
Large diffs are not rendered by default.

tests/unit/manage/test_views.py

Lines changed: 143 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2310,7 +2310,75 @@ def test_delete_project_disallow_deletion(self):
23102310
pretend.call("manage.project.settings", project_name="foo")
23112311
]
23122312

2313-
def test_delete_project(self, db_request):
2313+
def test_get_project_contributors(self, db_request):
2314+
project = ProjectFactory.create(name="foo")
2315+
db_request.session = pretend.stub(
2316+
flash=pretend.call_recorder(lambda *a, **kw: None),
2317+
)
2318+
2319+
db_request.user = UserFactory.create()
2320+
project.users = [db_request.user]
2321+
2322+
res = views.get_project_contributors(project.name, db_request)
2323+
assert res == [db_request.user]
2324+
2325+
def test_get_user_role_in_project_single_role_owner(self, db_request):
2326+
project = ProjectFactory.create(name="foo")
2327+
db_request.session = pretend.stub(
2328+
flash=pretend.call_recorder(lambda *a, **kw: None),
2329+
)
2330+
db_request.user = UserFactory.create()
2331+
project.users = [db_request.user]
2332+
RoleFactory(user=db_request.user, project=project)
2333+
2334+
res = views.get_user_role_in_project(
2335+
project.name, db_request.user.username, db_request
2336+
)
2337+
assert res == "Owner"
2338+
2339+
def test_get_user_role_in_project_single_role_maintainer(self, db_request):
2340+
project = ProjectFactory.create(name="foo")
2341+
db_request.session = pretend.stub(
2342+
flash=pretend.call_recorder(lambda *a, **kw: None),
2343+
)
2344+
db_request.user = UserFactory.create()
2345+
project.users = [db_request.user]
2346+
RoleFactory(user=db_request.user, project=project, role_name="Maintainer")
2347+
2348+
res = views.get_user_role_in_project(
2349+
project.name, db_request.user.username, db_request
2350+
)
2351+
assert res == "Maintainer"
2352+
2353+
def test_get_user_role_in_project_two_roles_owner_and_maintainer(self, db_request):
2354+
project = ProjectFactory.create(name="foo")
2355+
db_request.session = pretend.stub(
2356+
flash=pretend.call_recorder(lambda *a, **kw: None),
2357+
)
2358+
db_request.user = UserFactory.create()
2359+
project.users = [db_request.user]
2360+
RoleFactory(user=db_request.user, project=project, role_name="Owner")
2361+
RoleFactory(user=db_request.user, project=project, role_name="Maintainer")
2362+
2363+
res = views.get_user_role_in_project(
2364+
project.name, db_request.user.username, db_request
2365+
)
2366+
assert res == "Owner"
2367+
2368+
def test_get_user_role_in_project_no_role(self, db_request):
2369+
project = ProjectFactory.create(name="foo")
2370+
db_request.session = pretend.stub(
2371+
flash=pretend.call_recorder(lambda *a, **kw: None),
2372+
)
2373+
db_request.user = UserFactory.create()
2374+
project.users = [db_request.user]
2375+
2376+
res = views.get_user_role_in_project(
2377+
project.name, db_request.user.username, db_request
2378+
)
2379+
assert res == ""
2380+
2381+
def test_delete_project(self, monkeypatch, db_request):
23142382
project = ProjectFactory.create(name="foo")
23152383

23162384
db_request.route_path = pretend.call_recorder(lambda *a, **kw: "/the-redirect")
@@ -2319,6 +2387,22 @@ def test_delete_project(self, db_request):
23192387
)
23202388
db_request.POST["confirm_project_name"] = project.normalized_name
23212389
db_request.user = UserFactory.create()
2390+
2391+
get_user_role_in_project = pretend.call_recorder(
2392+
lambda project_name, username, req: "Owner"
2393+
)
2394+
monkeypatch.setattr(views, "get_user_role_in_project", get_user_role_in_project)
2395+
2396+
get_project_contributors = pretend.call_recorder(
2397+
lambda project_name, req: [db_request.user]
2398+
)
2399+
monkeypatch.setattr(views, "get_project_contributors", get_project_contributors)
2400+
2401+
send_removed_project_email = pretend.call_recorder(lambda req, user, **k: None)
2402+
monkeypatch.setattr(
2403+
views, "send_removed_project_email", send_removed_project_email
2404+
)
2405+
23222406
db_request.remote_addr = "192.168.1.1"
23232407

23242408
result = views.delete_project(project, db_request)
@@ -2329,6 +2413,26 @@ def test_delete_project(self, db_request):
23292413
assert db_request.route_path.calls == [pretend.call("manage.projects")]
23302414
assert isinstance(result, HTTPSeeOther)
23312415
assert result.headers["Location"] == "/the-redirect"
2416+
2417+
assert get_user_role_in_project.calls == [
2418+
pretend.call(project.name, db_request.user.username, db_request,),
2419+
pretend.call(project.name, db_request.user.username, db_request,),
2420+
]
2421+
2422+
assert get_project_contributors.calls == [
2423+
pretend.call(project.name, db_request,)
2424+
]
2425+
2426+
assert send_removed_project_email.calls == [
2427+
pretend.call(
2428+
db_request,
2429+
db_request.user,
2430+
project_name=project.name,
2431+
submitter_name=db_request.user.username,
2432+
submitter_role="Owner",
2433+
recipient_role="Owner",
2434+
)
2435+
]
23322436
assert not (db_request.db.query(Project).filter(Project.name == "foo").count())
23332437

23342438

@@ -2495,6 +2599,7 @@ def test_delete_project_release(self, monkeypatch):
24952599
project=pretend.stub(
24962600
name="foobar", record_event=pretend.call_recorder(lambda *a, **kw: None)
24972601
),
2602+
created=datetime.datetime(2017, 2, 5, 17, 18, 18, 462_634),
24982603
)
24992604
request = pretend.stub(
25002605
POST={"confirm_version": release.version},
@@ -2511,7 +2616,25 @@ def test_delete_project_release(self, monkeypatch):
25112616
)
25122617
journal_obj = pretend.stub()
25132618
journal_cls = pretend.call_recorder(lambda **kw: journal_obj)
2619+
2620+
get_user_role_in_project = pretend.call_recorder(
2621+
lambda project_name, username, req: "Owner"
2622+
)
2623+
monkeypatch.setattr(views, "get_user_role_in_project", get_user_role_in_project)
2624+
get_project_contributors = pretend.call_recorder(
2625+
lambda project_name, request: [request.user]
2626+
)
2627+
monkeypatch.setattr(views, "get_project_contributors", get_project_contributors)
2628+
25142629
monkeypatch.setattr(views, "JournalEntry", journal_cls)
2630+
send_removed_project_release_email = pretend.call_recorder(
2631+
lambda req, contrib, **k: None
2632+
)
2633+
monkeypatch.setattr(
2634+
views,
2635+
"send_removed_project_release_email",
2636+
send_removed_project_release_email,
2637+
)
25152638

25162639
view = views.ManageProjectRelease(release, request)
25172640

@@ -2520,6 +2643,25 @@ def test_delete_project_release(self, monkeypatch):
25202643
assert isinstance(result, HTTPSeeOther)
25212644
assert result.headers["Location"] == "/the-redirect"
25222645

2646+
assert get_user_role_in_project.calls == [
2647+
pretend.call(release.project.name, request.user.username, request,),
2648+
pretend.call(release.project.name, request.user.username, request,),
2649+
]
2650+
assert get_project_contributors.calls == [
2651+
pretend.call(release.project.name, request,)
2652+
]
2653+
2654+
assert send_removed_project_release_email.calls == [
2655+
pretend.call(
2656+
request,
2657+
request.user,
2658+
release=release,
2659+
submitter_name=request.user.username,
2660+
submitter_role="Owner",
2661+
recipient_role="Owner",
2662+
)
2663+
]
2664+
25232665
assert request.db.delete.calls == [pretend.call(release)]
25242666
assert request.db.add.calls == [pretend.call(journal_obj)]
25252667
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.version,
243+
"release_date": release.created.strftime("%Y-%m-%d"),
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: 81 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_project_email,
42+
send_removed_project_release_email,
4143
send_two_factor_added_email,
4244
send_two_factor_removed_email,
4345
)
@@ -899,6 +901,46 @@ def manage_project_settings(project, request):
899901
return {"project": project}
900902

901903

904+
def get_project_contributors(project_name, request):
905+
query_res = (
906+
request.db.query(Project)
907+
.join(User, Project.users)
908+
.filter(Project.name == project_name)
909+
.one()
910+
)
911+
return query_res.users
912+
913+
914+
def get_user_role_in_project(project_name, username, request):
915+
raw_res = (
916+
request.db.query(Project)
917+
.join(User, Project.users)
918+
.filter(User.username == username, Project.name == project_name)
919+
.with_entities(Role.role_name)
920+
.distinct(Role.role_name)
921+
.all()
922+
)
923+
924+
query_res = []
925+
for el in raw_res:
926+
if el.role_name is not None:
927+
query_res.append(el)
928+
929+
user_role = ""
930+
# This check is needed because of
931+
# issue https://github.com/pypa/warehouse/issues/2745
932+
# which is not yet resolved and a user could be an owner
933+
# and a maintainer at the same time
934+
if len(query_res) == 2 and (
935+
query_res[0].role_name == "Owner" or query_res[1].role_name == "Owner"
936+
):
937+
user_role = "Owner"
938+
if len(query_res) == 1:
939+
user_role = query_res[0].role_name
940+
941+
return user_role
942+
943+
902944
@view_config(
903945
route_name="manage.project.delete_project",
904946
context=Project,
@@ -921,6 +963,26 @@ def delete_project(project, request):
921963
)
922964

923965
confirm_project(project, request, fail_route="manage.project.settings")
966+
967+
submitter_role = get_user_role_in_project(
968+
project.name, request.user.username, request
969+
)
970+
contributors = get_project_contributors(project.name, request)
971+
972+
for contributor in contributors:
973+
contributor_role = get_user_role_in_project(
974+
project.name, contributor.username, request
975+
)
976+
977+
send_removed_project_email(
978+
request,
979+
contributor,
980+
project_name=project.name,
981+
submitter_name=request.user.username,
982+
submitter_role=submitter_role,
983+
recipient_role=contributor_role,
984+
)
985+
924986
remove_project(project, request)
925987

926988
return HTTPSeeOther(request.route_path("manage.projects"))
@@ -1053,6 +1115,11 @@ def delete_project_release(self):
10531115
)
10541116
)
10551117

1118+
submitter_role = get_user_role_in_project(
1119+
self.release.project.name, self.request.user.username, self.request
1120+
)
1121+
contributors = get_project_contributors(self.release.project.name, self.request)
1122+
10561123
self.request.db.add(
10571124
JournalEntry(
10581125
name=self.release.project.name,
@@ -1078,6 +1145,20 @@ def delete_project_release(self):
10781145
f"Deleted release {self.release.version!r}", queue="success"
10791146
)
10801147

1148+
for contributor in contributors:
1149+
contributor_role = get_user_role_in_project(
1150+
self.release.project.name, contributor.username, self.request
1151+
)
1152+
1153+
send_removed_project_release_email(
1154+
self.request,
1155+
contributor,
1156+
release=self.release,
1157+
submitter_name=self.request.user.username,
1158+
submitter_role=submitter_role,
1159+
recipient_role=contributor_role,
1160+
)
1161+
10811162
return HTTPSeeOther(
10821163
self.request.route_path(
10831164
"manage.project.releases", project_name=self.release.project.name
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
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.{% endtrans %}</p>
40+
41+
{% endblock %}

0 commit comments

Comments
 (0)