Skip to content

Commit

Permalink
Add email notication on package/release removal
Browse files Browse the repository at this point in the history
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 pypi#5714.

Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
  • Loading branch information
MVrachev committed Feb 10, 2020
1 parent c4cb589 commit 7a0ea1f
Show file tree
Hide file tree
Showing 9 changed files with 361 additions and 1 deletion.
70 changes: 69 additions & 1 deletion tests/unit/manage/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2135,7 +2135,7 @@ def test_delete_project_disallow_deletion(self):
pretend.call("manage.project.settings", project_name="foo")
]

def test_delete_project(self, db_request):
def test_delete_project(self, monkeypatch, db_request):
project = ProjectFactory.create(name="foo")

db_request.route_path = pretend.call_recorder(lambda *a, **kw: "/the-redirect")
Expand All @@ -2144,6 +2144,21 @@ def test_delete_project(self, db_request):
)
db_request.POST["confirm_project_name"] = project.normalized_name
db_request.user = UserFactory.create()

get_user_role_in_project = pretend.call_recorder(
lambda project, username, req: "Owner"
)
monkeypatch.setattr(views, "get_user_role_in_project", get_user_role_in_project)
get_project_contributors = pretend.call_recorder(
lambda project, req: [db_request.user]
)
monkeypatch.setattr(views, "get_project_contributors", get_project_contributors)

send_removed_project_email = pretend.call_recorder(lambda req, user, **k: None)
monkeypatch.setattr(
views, "send_removed_project_email", send_removed_project_email
)

db_request.remote_addr = "192.168.1.1"

result = views.delete_project(project, db_request)
Expand All @@ -2154,6 +2169,21 @@ def test_delete_project(self, db_request):
assert db_request.route_path.calls == [pretend.call("manage.projects")]
assert isinstance(result, HTTPSeeOther)
assert result.headers["Location"] == "/the-redirect"
assert get_user_role_in_project.calls == [
pretend.call(project, db_request.user.username, db_request,),
pretend.call(project, db_request.user.username, db_request,),
]
assert get_project_contributors.calls == [pretend.call(project, db_request,)]
assert send_removed_project_email.calls == [
pretend.call(
db_request,
db_request.user,
project_name=project.name,
submitter_name=db_request.user.username,
submitter_role="Owner",
recipient_role="Owner",
)
]
assert not (db_request.db.query(Project).filter(Project.name == "foo").count())


Expand Down Expand Up @@ -2320,6 +2350,7 @@ def test_delete_project_release(self, monkeypatch):
project=pretend.stub(
name="foobar", record_event=pretend.call_recorder(lambda *a, **kw: None)
),
created=datetime.datetime(2017, 2, 5, 17, 18, 18, 462_634),
)
request = pretend.stub(
POST={"confirm_version": release.version},
Expand All @@ -2336,7 +2367,25 @@ def test_delete_project_release(self, monkeypatch):
)
journal_obj = pretend.stub()
journal_cls = pretend.call_recorder(lambda **kw: journal_obj)

get_user_role_in_project = pretend.call_recorder(
lambda project, username, req: "Owner"
)
monkeypatch.setattr(views, "get_user_role_in_project", get_user_role_in_project)
get_project_contributors = pretend.call_recorder(
lambda project, request: [request.user]
)
monkeypatch.setattr(views, "get_project_contributors", get_project_contributors)

monkeypatch.setattr(views, "JournalEntry", journal_cls)
send_removed_project_release_email = pretend.call_recorder(
lambda req, contrib, **k: None
)
monkeypatch.setattr(
views,
"send_removed_project_release_email",
send_removed_project_release_email,
)

view = views.ManageProjectRelease(release, request)

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

assert get_user_role_in_project.calls == [
pretend.call(release.project, request.user.username, request,),
pretend.call(release.project, request.user.username, request,),
]
assert get_project_contributors.calls == [
pretend.call(release.project, request,)
]

assert send_removed_project_release_email.calls == [
pretend.call(
request,
request.user,
release=release.version,
submitter_name=request.user.username,
submitter_role="Owner",
recipient_role="Owner",
)
]

assert request.db.delete.calls == [pretend.call(release)]
assert request.db.add.calls == [pretend.call(journal_obj)]
assert request.flags.enabled.calls == [
Expand Down
40 changes: 40 additions & 0 deletions warehouse/email/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,46 @@ def send_two_factor_removed_email(request, user, method):
return {"method": pretty_methods[method], "username": user.username}


@_email("removed-project")
def send_removed_project_email(
request, user, *, project_name, submitter_name, submitter_role, recipient_role
):
recipient_role_descr = "an owner"
if recipient_role == "Maintainer":
recipient_role_descr = "a maintainer"

return {
"project": project_name,
"submitter": submitter_name,
"submitter_role": submitter_role,
"recipient_role_descr": recipient_role_descr,
}


@_email("removed-project-release")
def send_removed_project_release_email(
request,
user,
*,
release,
submitter_name,
submitter_role,
recipient_role
):
recipient_role_descr = "an owner"
if recipient_role == "Maintainer":
recipient_role_descr = "a maintainer"

return {
"project": release.project.name,
"release": release,
"release_date": release.created.strftime("%b %d %Y"),
"submitter": submitter_name,
"submitter_role": submitter_role,
"recipient_role_descr": recipient_role_descr,
}


def includeme(config):
email_sending_class = config.maybe_dotted(config.registry.settings["mail.backend"])
config.register_service_factory(email_sending_class.create_service, IEmailSender)
Expand Down
73 changes: 73 additions & 0 deletions warehouse/manage/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
send_primary_email_change_email,
send_two_factor_added_email,
send_two_factor_removed_email,
send_removed_project_email,
send_removed_project_release_email,
)
from warehouse.i18n import localize as _
from warehouse.macaroons.interfaces import IMacaroonService
Expand Down Expand Up @@ -812,6 +814,40 @@ def manage_project_settings(project, request):
return {"project": project}


def get_project_contributors(project, request):
query_res = (
request.db.query(Project)
.join(User, Project.users)
.filter(Project.name == project.name)
.one()
)
return query_res.users


def get_user_role_in_project(project, username, request):
query_res = (
request.db.query(Project)
.join(User, Project.users)
.filter(User.username == username, Project.name == project.name)
.with_entities(Role.role_name)
.distinct(Role.role_name)
.all()
)

user_role = ""
# This check is needed because of
# issue https://github.com/pypa/warehouse/issues/2745
# which is not yet resolved and a user could be an owner
# and a maintainer at the same time
if len(query_res) == 2 and (
query_res[0].role_name == "Owner" or query_res[1].role_name == "Owner"
):
user_role = "Owner"
else:
user_role = "Maintainer"
return user_role


@view_config(
route_name="manage.project.delete_project",
context=Project,
Expand All @@ -834,6 +870,24 @@ def delete_project(project, request):
)

confirm_project(project, request, fail_route="manage.project.settings")

submitter_role = get_user_role_in_project(project, request.user.username, request)
contributors = get_project_contributors(project, request)

for contributor in contributors:
contributor_role = get_user_role_in_project(
project, contributor.username, request
)

send_removed_project_email(
request,
contributor,
project_name=project.name,
submitter_name=request.user.username,
submitter_role=submitter_role,
recipient_role=contributor_role,
)

remove_project(project, request)

return HTTPSeeOther(request.route_path("manage.projects"))
Expand Down Expand Up @@ -966,6 +1020,11 @@ def delete_project_release(self):
)
)

submitter_role = get_user_role_in_project(
self.release.project, self.request.user.username, self.request
)
contributors = get_project_contributors(self.release.project, self.request)

self.request.db.add(
JournalEntry(
name=self.release.project.name,
Expand All @@ -991,6 +1050,20 @@ def delete_project_release(self):
f"Deleted release {self.release.version!r}", queue="success"
)

for contributor in contributors:
contributor_role = get_user_role_in_project(
self.release.project, contributor.username, self.request
)

send_removed_project_release_email(
self.request,
contributor,
release=self.release.version,
submitter_name=self.request.user.username,
submitter_role=submitter_role,
recipient_role=contributor_role,
)

return HTTPSeeOther(
self.request.route_path(
"manage.project.releases", project_name=self.release.project.name
Expand Down
42 changes: 42 additions & 0 deletions warehouse/templates/email/removed-project-release/body.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
{#
# 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.
-#}
{% extends "email/_base/body.html" %}

{% block extra_style %}
ul.collaborator-details {
list-style-type: none;
}
{% endblock %}

{% block content %}
<p>
<ul class="removed-project-release">
<li>{% trans project=project, release=release, date=release_date %}The {{ project }} release {{ release }} released on {{ date }} has been deleted.{% endtrans %}</li>
<li>{% trans submitter=submitter, role=submitter_role %}<strong>Deleted by:</strong> {{ submitter }} with a role:
{{ role }}.{% endtrans %}
</li>
</ul>
</p>

<p>{% trans href='mailto:admin@pypi.org', email_address='admin@pypi.org' %}If this was a mistake, you can email <a
href="{{ href }}">{{ email_address }}</a> to communicate with the PyPI administrators.{% endtrans %}</p>
{% endblock %}

{% block reason %}

<p>{% trans recipient_role_descr=recipient_role_descr %}
You are receiving this because you are {{ recipient_role_descr }} of this project.
{% endtrans %}</p>

{% endblock %}
29 changes: 29 additions & 0 deletions warehouse/templates/email/removed-project-release/body.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{#
# 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.
-#}

{% extends "email/_base/body.txt" %}

{% block content %}
{% trans project=project, release=release, date=release_date %}The {{ project }} release {{ release }} released on {{ date }} has been deleted.{% endtrans %}

{% trans submitter=submitter, role=submitter_role %}Deleted by: {{ submitter }} with a role: {{ role }}.{% endtrans %}

{% trans email_address='admin@pypi.org' %}If this was a mistake, you can email {{ email_address }} to communicate with the PyPI administrators.{% endtrans %}
{% endblock %}

{% block reason %}
{% trans recipient_role_descr=recipient_role_descr %}
You are receiving this because you are {{ recipient_role_descr }} of this project.
{% endtrans %}
{% endblock %}
18 changes: 18 additions & 0 deletions warehouse/templates/email/removed-project-release/subject.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{#
# 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.
-#}

{% extends "email/_base/subject.txt" %}

{% block content %}
{% trans project=project, release=release %}The {{ project }} release {{ release }} has been deleted.{% endtrans %}{% endblock %}

0 comments on commit 7a0ea1f

Please sign in to comment.