From a9f6c663f43c5b84a6b339b239a52305bf06000a Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Thu, 7 Apr 2022 21:17:55 -0700 Subject: [PATCH 01/46] admin-new-organization-requested email template --- warehouse/email/__init__.py | 16 ++++++++++++++ .../body.html | 20 ++++++++++++++++++ .../admin-new-organization-requested/body.txt | 21 +++++++++++++++++++ .../subject.txt | 17 +++++++++++++++ 4 files changed, 74 insertions(+) create mode 100644 warehouse/templates/email/admin-new-organization-requested/body.html create mode 100644 warehouse/templates/email/admin-new-organization-requested/body.txt create mode 100644 warehouse/templates/email/admin-new-organization-requested/subject.txt diff --git a/warehouse/email/__init__.py b/warehouse/email/__init__.py index 752dc4392887..3e0f77e609b0 100644 --- a/warehouse/email/__init__.py +++ b/warehouse/email/__init__.py @@ -186,6 +186,22 @@ def wrapper(request, user_or_users, **kwargs): return inner +# Email templates for administrators. + + +@_email("admin-new-organization-requested") +def send_admin_new_organization_requested_email( + request, user, *, organization_name, initiator_username +): + return { + "initiator_username": initiator_username, + "organization_name": organization_name, + } + + +# Email templates for users. + + @_email("password-reset", allow_unverified=True) def send_password_reset_email(request, user_and_email): user, _ = user_and_email diff --git a/warehouse/templates/email/admin-new-organization-requested/body.html b/warehouse/templates/email/admin-new-organization-requested/body.html new file mode 100644 index 000000000000..a9be9761f447 --- /dev/null +++ b/warehouse/templates/email/admin-new-organization-requested/body.html @@ -0,0 +1,20 @@ +{# + # 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 content %} +

{{ initiator_username }} has requested approval to create a new PyPI organization named "{{ organization_name }}".

+ +

Please follow this link to approve or decline the request.

+{% endblock %} diff --git a/warehouse/templates/email/admin-new-organization-requested/body.txt b/warehouse/templates/email/admin-new-organization-requested/body.txt new file mode 100644 index 000000000000..3f7bacb1fbf2 --- /dev/null +++ b/warehouse/templates/email/admin-new-organization-requested/body.txt @@ -0,0 +1,21 @@ +{# + # 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 %} +{{ initiator_username }} has requested approval to create a new PyPI organization named "{{ organization_name }}". + +Please follow this link to approve or decline the request: +{{ request.route_url('admin.organization.approve', _query={'organization_name': organization_name}) }} +{% endblock %} diff --git a/warehouse/templates/email/admin-new-organization-requested/subject.txt b/warehouse/templates/email/admin-new-organization-requested/subject.txt new file mode 100644 index 000000000000..12f6f80639b3 --- /dev/null +++ b/warehouse/templates/email/admin-new-organization-requested/subject.txt @@ -0,0 +1,17 @@ +{# + # 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 subject %}{% trans %}{{ initiator_username }} has requested approval to create an organization named "{{ organization_name }}"{% endtrans %}{% endblock %} From aa2a93976360013871a141e1bd3741f6c69ce6c7 Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Tue, 12 Apr 2022 11:18:18 -0700 Subject: [PATCH 02/46] new-orgnization-requested email template --- warehouse/email/__init__.py | 5 +++++ .../new-organization-requested/body.html | 20 +++++++++++++++++++ .../email/new-organization-requested/body.txt | 20 +++++++++++++++++++ .../new-organization-requested/subject.txt | 17 ++++++++++++++++ 4 files changed, 62 insertions(+) create mode 100644 warehouse/templates/email/new-organization-requested/body.html create mode 100644 warehouse/templates/email/new-organization-requested/body.txt create mode 100644 warehouse/templates/email/new-organization-requested/subject.txt diff --git a/warehouse/email/__init__.py b/warehouse/email/__init__.py index 3e0f77e609b0..3a564cc2d043 100644 --- a/warehouse/email/__init__.py +++ b/warehouse/email/__init__.py @@ -283,6 +283,11 @@ def send_primary_email_change_email(request, user_and_email): } +@_email("new-organization-requested") +def send_new_organization_requested_email(request, user, *, organization_name): + return {"organization_name": organization_name} + + @_email("collaborator-added") def send_collaborator_added_email( request, email_recipients, *, user, submitter, project_name, role diff --git a/warehouse/templates/email/new-organization-requested/body.html b/warehouse/templates/email/new-organization-requested/body.html new file mode 100644 index 000000000000..c4c9582cbffb --- /dev/null +++ b/warehouse/templates/email/new-organization-requested/body.html @@ -0,0 +1,20 @@ +{# + # 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 content %} +

Your request for a new PyPI organization named "{{ organization_name }}" has been submitted.

+ +

You will receive another email when the PyPI organization has been approved.

+{% endblock %} diff --git a/warehouse/templates/email/new-organization-requested/body.txt b/warehouse/templates/email/new-organization-requested/body.txt new file mode 100644 index 000000000000..64803627e61a --- /dev/null +++ b/warehouse/templates/email/new-organization-requested/body.txt @@ -0,0 +1,20 @@ +{# + # 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 %} +Your request for a new PyPI organization named "{{ organization_name }}" has been submitted. + +You will receive another email when the PyPI organization has been approved. +{% endblock %} diff --git a/warehouse/templates/email/new-organization-requested/subject.txt b/warehouse/templates/email/new-organization-requested/subject.txt new file mode 100644 index 000000000000..168df349326b --- /dev/null +++ b/warehouse/templates/email/new-organization-requested/subject.txt @@ -0,0 +1,17 @@ +{# + # 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 subject %}{% trans %}Your request for a new organization named "{{ organization_name }}" has been submitted{% endtrans %}{% endblock %} From 0bde7fa492dabd9414d05175a61097a307ac89ec Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Tue, 12 Apr 2022 12:14:30 -0700 Subject: [PATCH 03/46] Test admin-new-organization-requested email --- tests/unit/email/test_init.py | 95 +++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/tests/unit/email/test_init.py b/tests/unit/email/test_init.py index 5ae27303bb8e..6e22a8062e36 100644 --- a/tests/unit/email/test_init.py +++ b/tests/unit/email/test_init.py @@ -413,6 +413,101 @@ def retry(exc): assert task.retry.calls == [pretend.call(exc=exc)] +class TestSendAdminNewOrganizationrequestedEmail: + def test_send_admin_new_organization_requested_email( + self, pyramid_request, pyramid_config, monkeypatch + ): + admin_user = pretend.stub( + id="admin", + username="admin", + name="PyPI Adminstrator", + email="admin@pypi.org", + primary_email=pretend.stub(email="admin@pypi.org", verified=True), + ) + initiator_user = pretend.stub( + id="id", + username="username", + name="", + email="email@example.com", + primary_email=pretend.stub(email="email@example.com", verified=True), + ) + organization_name = "example" + + subject_renderer = pyramid_config.testing_add_renderer( + "email/admin-new-organization-requested/subject.txt" + ) + subject_renderer.string_response = "Email Subject" + body_renderer = pyramid_config.testing_add_renderer( + "email/admin-new-organization-requested/body.txt" + ) + body_renderer.string_response = "Email Body" + html_renderer = pyramid_config.testing_add_renderer( + "email/admin-new-organization-requested/body.html" + ) + html_renderer.string_response = "Email HTML Body" + + send_email = pretend.stub( + delay=pretend.call_recorder(lambda *args, **kwargs: None) + ) + pyramid_request.task = pretend.call_recorder(lambda *args, **kwargs: send_email) + monkeypatch.setattr(email, "send_email", send_email) + + pyramid_request.db = pretend.stub( + query=lambda a: pretend.stub( + filter=lambda *a: pretend.stub( + one=lambda: pretend.stub(user_id=admin_user.id) + ) + ), + ) + pyramid_request.user = initiator_user + pyramid_request.registry.settings = {"mail.sender": "noreply@example.com"} + + result = email.send_admin_new_organization_requested_email( + pyramid_request, + admin_user, + organization_name=organization_name, + initiator_username=initiator_user.username, + ) + + assert result == { + "organization_name": organization_name, + "initiator_username": initiator_user.username, + } + subject_renderer.assert_() + body_renderer.assert_( + organization_name=organization_name, + initiator_username=initiator_user.username, + ) + html_renderer.assert_( + organization_name=organization_name, + initiator_username=initiator_user.username, + ) + assert pyramid_request.task.calls == [pretend.call(send_email)] + assert send_email.delay.calls == [ + pretend.call( + f"{admin_user.name} <{admin_user.email}>", + { + "subject": "Email Subject", + "body_text": "Email Body", + "body_html": ( + "\n\n" + "

Email HTML Body

\n\n" + ), + }, + { + "tag": "account:email:sent", + "user_id": admin_user.id, + "additional": { + "from_": "noreply@example.com", + "to": admin_user.email, + "subject": "Email Subject", + "redact_ip": True, + }, + }, + ) + ] + + class TestSendPasswordResetEmail: @pytest.mark.parametrize( ("verified", "email_addr"), From 6fb99e75e2ef9a93f62ff9b3142986a3593ba057 Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Tue, 12 Apr 2022 12:25:26 -0700 Subject: [PATCH 04/46] Test new-organization-requested email --- tests/unit/email/test_init.py | 78 +++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/tests/unit/email/test_init.py b/tests/unit/email/test_init.py index 6e22a8062e36..2f30796e50db 100644 --- a/tests/unit/email/test_init.py +++ b/tests/unit/email/test_init.py @@ -1355,6 +1355,84 @@ def test_primary_email_change_email_unverified( assert send_email.delay.calls == [] +class TestSendNewOrganizationrequestedEmail: + def test_send_new_organization_requested_email( + self, pyramid_request, pyramid_config, monkeypatch + ): + initiator_user = pretend.stub( + id="id", + username="username", + name="", + email="email@example.com", + primary_email=pretend.stub(email="email@example.com", verified=True), + ) + organization_name = "example" + + subject_renderer = pyramid_config.testing_add_renderer( + "email/new-organization-requested/subject.txt" + ) + subject_renderer.string_response = "Email Subject" + body_renderer = pyramid_config.testing_add_renderer( + "email/new-organization-requested/body.txt" + ) + body_renderer.string_response = "Email Body" + html_renderer = pyramid_config.testing_add_renderer( + "email/new-organization-requested/body.html" + ) + html_renderer.string_response = "Email HTML Body" + + send_email = pretend.stub( + delay=pretend.call_recorder(lambda *args, **kwargs: None) + ) + pyramid_request.task = pretend.call_recorder(lambda *args, **kwargs: send_email) + monkeypatch.setattr(email, "send_email", send_email) + + pyramid_request.db = pretend.stub( + query=lambda a: pretend.stub( + filter=lambda *a: pretend.stub( + one=lambda: pretend.stub(user_id=initiator_user.id) + ) + ), + ) + pyramid_request.user = initiator_user + pyramid_request.registry.settings = {"mail.sender": "noreply@example.com"} + + result = email.send_new_organization_requested_email( + pyramid_request, + initiator_user, + organization_name=organization_name, + ) + + assert result == {"organization_name": organization_name} + subject_renderer.assert_() + body_renderer.assert_(organization_name=organization_name) + html_renderer.assert_(organization_name=organization_name) + assert pyramid_request.task.calls == [pretend.call(send_email)] + assert send_email.delay.calls == [ + pretend.call( + f"{initiator_user.username} <{initiator_user.email}>", + { + "subject": "Email Subject", + "body_text": "Email Body", + "body_html": ( + "\n\n" + "

Email HTML Body

\n\n" + ), + }, + { + "tag": "account:email:sent", + "user_id": initiator_user.id, + "additional": { + "from_": "noreply@example.com", + "to": initiator_user.email, + "subject": "Email Subject", + "redact_ip": False, + }, + }, + ) + ] + + class TestCollaboratorAddedEmail: def test_collaborator_added_email( self, pyramid_request, pyramid_config, monkeypatch From e8f1b5cc108befa0032e7870f1167717e0e576b8 Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Tue, 12 Apr 2022 15:07:58 -0700 Subject: [PATCH 05/46] Translate *-organization-requested consistently --- .../email/admin-new-organization-requested/body.html | 4 ++-- .../templates/email/admin-new-organization-requested/body.txt | 4 ++-- .../email/admin-new-organization-requested/subject.txt | 2 +- .../templates/email/new-organization-requested/body.html | 4 ++-- warehouse/templates/email/new-organization-requested/body.txt | 4 ++-- .../templates/email/new-organization-requested/subject.txt | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/warehouse/templates/email/admin-new-organization-requested/body.html b/warehouse/templates/email/admin-new-organization-requested/body.html index a9be9761f447..bd710600326e 100644 --- a/warehouse/templates/email/admin-new-organization-requested/body.html +++ b/warehouse/templates/email/admin-new-organization-requested/body.html @@ -14,7 +14,7 @@ {% extends "email/_base/body.html" %} {% block content %} -

{{ initiator_username }} has requested approval to create a new PyPI organization named "{{ organization_name }}".

+

{% trans organization_name=organization_name, initiator_username=initiator_username, initiator_profile=request.route_url('accounts.profile', username=initiator_username) %}{{ initiator_username }} has requested approval to create a new PyPI organization named "{{ organization_name }}".{% endtrans %}

-

Please follow this link to approve or decline the request.

+

{% trans approve_organization_link=request.route_url('admin.organization.approve', _query={'organization_name': organization_name}) %}Please follow this link to approve or decline the request.{% endtrans %}

{% endblock %} diff --git a/warehouse/templates/email/admin-new-organization-requested/body.txt b/warehouse/templates/email/admin-new-organization-requested/body.txt index 3f7bacb1fbf2..ba6b9d686e8b 100644 --- a/warehouse/templates/email/admin-new-organization-requested/body.txt +++ b/warehouse/templates/email/admin-new-organization-requested/body.txt @@ -14,8 +14,8 @@ {% extends "email/_base/body.txt" %} {% block content %} -{{ initiator_username }} has requested approval to create a new PyPI organization named "{{ organization_name }}". +{% trans organization_name=organization_name, initiator_username=initiator_username %}{{ initiator_username }} has requested approval to create a new PyPI organization named "{{ organization_name }}".{% endtrans %} -Please follow this link to approve or decline the request: +{% trans %}Please follow this link to approve or decline the request:{% endtrans %} {{ request.route_url('admin.organization.approve', _query={'organization_name': organization_name}) }} {% endblock %} diff --git a/warehouse/templates/email/admin-new-organization-requested/subject.txt b/warehouse/templates/email/admin-new-organization-requested/subject.txt index 12f6f80639b3..d2c61a084e96 100644 --- a/warehouse/templates/email/admin-new-organization-requested/subject.txt +++ b/warehouse/templates/email/admin-new-organization-requested/subject.txt @@ -14,4 +14,4 @@ {% extends "email/_base/subject.txt" %} -{% block subject %}{% trans %}{{ initiator_username }} has requested approval to create an organization named "{{ organization_name }}"{% endtrans %}{% endblock %} +{% block subject %}{% trans organization_name=organization_name, initiator_username=initiator_username %}{{ initiator_username }} has requested approval to create an organization named "{{ organization_name }}"{% endtrans %}{% endblock %} diff --git a/warehouse/templates/email/new-organization-requested/body.html b/warehouse/templates/email/new-organization-requested/body.html index c4c9582cbffb..b09819e63566 100644 --- a/warehouse/templates/email/new-organization-requested/body.html +++ b/warehouse/templates/email/new-organization-requested/body.html @@ -14,7 +14,7 @@ {% extends "email/_base/body.html" %} {% block content %} -

Your request for a new PyPI organization named "{{ organization_name }}" has been submitted.

+

{% trans organization_name=organization_name %}Your request for a new PyPI organization named "{{ organization_name }}" has been submitted.{% endtrans %}

-

You will receive another email when the PyPI organization has been approved.

+

{% trans %}You will receive another email when the PyPI organization has been approved.{% endtrans %}

{% endblock %} diff --git a/warehouse/templates/email/new-organization-requested/body.txt b/warehouse/templates/email/new-organization-requested/body.txt index 64803627e61a..73ac8c2e6b29 100644 --- a/warehouse/templates/email/new-organization-requested/body.txt +++ b/warehouse/templates/email/new-organization-requested/body.txt @@ -14,7 +14,7 @@ {% extends "email/_base/body.txt" %} {% block content %} -Your request for a new PyPI organization named "{{ organization_name }}" has been submitted. +{% trans organization_name=organization_name %}Your request for a new PyPI organization named "{{ organization_name }}" has been submitted.{% endtrans %} -You will receive another email when the PyPI organization has been approved. +{% trans %}You will receive another email when the PyPI organization has been approved.{% endtrans %} {% endblock %} diff --git a/warehouse/templates/email/new-organization-requested/subject.txt b/warehouse/templates/email/new-organization-requested/subject.txt index 168df349326b..9d44efa1d1f5 100644 --- a/warehouse/templates/email/new-organization-requested/subject.txt +++ b/warehouse/templates/email/new-organization-requested/subject.txt @@ -14,4 +14,4 @@ {% extends "email/_base/subject.txt" %} -{% block subject %}{% trans %}Your request for a new organization named "{{ organization_name }}" has been submitted{% endtrans %}{% endblock %} +{% block subject %}{% trans organization_name=organization_name %}Your request for a new organization named "{{ organization_name }}" has been submitted{% endtrans %}{% endblock %} From d7d792351dcf7f463e64c7ef4a2a40fa9dc5a7bd Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Tue, 12 Apr 2022 15:34:45 -0700 Subject: [PATCH 06/46] `make translations` for *-organization-requested --- warehouse/locale/messages.pot | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/warehouse/locale/messages.pot b/warehouse/locale/messages.pot index 45cbb56c6e03..79f1e025bce8 100644 --- a/warehouse/locale/messages.pot +++ b/warehouse/locale/messages.pot @@ -1292,6 +1292,21 @@ msgid "" "%(initiator_username)s to a project on %(site)s." msgstr "" +#: warehouse/templates/email/admin-new-organization-requested/body.html:17 +#, python-format +msgid "" +"%(initiator_username)s has " +"requested approval to create a new PyPI organization named " +"\"%(organization_name)s\"." +msgstr "" + +#: warehouse/templates/email/admin-new-organization-requested/body.html:19 +#, python-format +msgid "" +"Please follow this link to " +"approve or decline the request." +msgstr "" + #: warehouse/templates/email/basic-auth-with-2fa/body.html:17 #: warehouse/templates/email/password-compromised-hibp/body.html:18 #: warehouse/templates/email/password-compromised/body.html:18 @@ -1329,6 +1344,19 @@ msgid "" "to publish." msgstr "" +#: warehouse/templates/email/new-organization-requested/body.html:17 +#, python-format +msgid "" +"Your request for a new PyPI organization named \"%(organization_name)s\" " +"has been submitted." +msgstr "" + +#: warehouse/templates/email/new-organization-requested/body.html:19 +msgid "" +"You will receive another email when the PyPI organization has been " +"approved." +msgstr "" + #: warehouse/templates/email/oidc-provider-added/body.html:19 #, python-format msgid "" From bec70581f294ef81222caa07603b4763c00e4344 Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Wed, 13 Apr 2022 05:11:01 -0700 Subject: [PATCH 07/46] Remove translations from admin-* emails On second thought, we probably don't want localization for admin emails. --- warehouse/locale/messages.pot | 15 --------------- .../admin-new-organization-requested/body.html | 4 ++-- .../admin-new-organization-requested/body.txt | 4 ++-- .../admin-new-organization-requested/subject.txt | 2 +- 4 files changed, 5 insertions(+), 20 deletions(-) diff --git a/warehouse/locale/messages.pot b/warehouse/locale/messages.pot index 79f1e025bce8..853ee9b6cff7 100644 --- a/warehouse/locale/messages.pot +++ b/warehouse/locale/messages.pot @@ -1292,21 +1292,6 @@ msgid "" "%(initiator_username)s to a project on %(site)s." msgstr "" -#: warehouse/templates/email/admin-new-organization-requested/body.html:17 -#, python-format -msgid "" -"%(initiator_username)s has " -"requested approval to create a new PyPI organization named " -"\"%(organization_name)s\"." -msgstr "" - -#: warehouse/templates/email/admin-new-organization-requested/body.html:19 -#, python-format -msgid "" -"Please follow this link to " -"approve or decline the request." -msgstr "" - #: warehouse/templates/email/basic-auth-with-2fa/body.html:17 #: warehouse/templates/email/password-compromised-hibp/body.html:18 #: warehouse/templates/email/password-compromised/body.html:18 diff --git a/warehouse/templates/email/admin-new-organization-requested/body.html b/warehouse/templates/email/admin-new-organization-requested/body.html index bd710600326e..a9be9761f447 100644 --- a/warehouse/templates/email/admin-new-organization-requested/body.html +++ b/warehouse/templates/email/admin-new-organization-requested/body.html @@ -14,7 +14,7 @@ {% extends "email/_base/body.html" %} {% block content %} -

{% trans organization_name=organization_name, initiator_username=initiator_username, initiator_profile=request.route_url('accounts.profile', username=initiator_username) %}{{ initiator_username }} has requested approval to create a new PyPI organization named "{{ organization_name }}".{% endtrans %}

+

{{ initiator_username }} has requested approval to create a new PyPI organization named "{{ organization_name }}".

-

{% trans approve_organization_link=request.route_url('admin.organization.approve', _query={'organization_name': organization_name}) %}Please follow this link to approve or decline the request.{% endtrans %}

+

Please follow this link to approve or decline the request.

{% endblock %} diff --git a/warehouse/templates/email/admin-new-organization-requested/body.txt b/warehouse/templates/email/admin-new-organization-requested/body.txt index ba6b9d686e8b..3f7bacb1fbf2 100644 --- a/warehouse/templates/email/admin-new-organization-requested/body.txt +++ b/warehouse/templates/email/admin-new-organization-requested/body.txt @@ -14,8 +14,8 @@ {% extends "email/_base/body.txt" %} {% block content %} -{% trans organization_name=organization_name, initiator_username=initiator_username %}{{ initiator_username }} has requested approval to create a new PyPI organization named "{{ organization_name }}".{% endtrans %} +{{ initiator_username }} has requested approval to create a new PyPI organization named "{{ organization_name }}". -{% trans %}Please follow this link to approve or decline the request:{% endtrans %} +Please follow this link to approve or decline the request: {{ request.route_url('admin.organization.approve', _query={'organization_name': organization_name}) }} {% endblock %} diff --git a/warehouse/templates/email/admin-new-organization-requested/subject.txt b/warehouse/templates/email/admin-new-organization-requested/subject.txt index d2c61a084e96..bf52ffab575e 100644 --- a/warehouse/templates/email/admin-new-organization-requested/subject.txt +++ b/warehouse/templates/email/admin-new-organization-requested/subject.txt @@ -14,4 +14,4 @@ {% extends "email/_base/subject.txt" %} -{% block subject %}{% trans organization_name=organization_name, initiator_username=initiator_username %}{{ initiator_username }} has requested approval to create an organization named "{{ organization_name }}"{% endtrans %}{% endblock %} +{% block subject %}{{ initiator_username }} has requested approval to create an organization named "{{ organization_name }}"{% endblock %} From 08fb16649c070463bcd7b6728d502e2ed3c6aaad Mon Sep 17 00:00:00 2001 From: sterbo Date: Fri, 8 Apr 2022 14:49:51 -0700 Subject: [PATCH 08/46] Initial cut at db model architecture --- warehouse/organizations/__init__.py | 13 ++ warehouse/organizations/models.py | 212 ++++++++++++++++++++++++++++ 2 files changed, 225 insertions(+) create mode 100644 warehouse/organizations/__init__.py create mode 100644 warehouse/organizations/models.py diff --git a/warehouse/organizations/__init__.py b/warehouse/organizations/__init__.py new file mode 100644 index 000000000000..5847d0b1544f --- /dev/null +++ b/warehouse/organizations/__init__.py @@ -0,0 +1,13 @@ +# 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. + +# TODO: Nothing to see here yet. These aren't the droids you're looking for. \ No newline at end of file diff --git a/warehouse/organizations/models.py b/warehouse/organizations/models.py new file mode 100644 index 000000000000..a546be8b21e7 --- /dev/null +++ b/warehouse/organizations/models.py @@ -0,0 +1,212 @@ +# 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. + +import enum + +from collections import OrderedDict +from urllib.parse import urlparse + +import packaging.utils + +from citext import CIText +from pyramid.authorization import Allow +from pyramid.threadlocal import get_current_request +from sqlalchemy import ( + BigInteger, + Boolean, + CheckConstraint, + Column, + DateTime, + Enum, + Float, + ForeignKey, + Index, + Integer, + String, + Table, + Text, + UniqueConstraint, + func, + orm, + sql, +) +from sqlalchemy.dialects.postgresql import JSONB, UUID +from sqlalchemy.ext.associationproxy import association_proxy +from sqlalchemy.ext.declarative import declared_attr # type: ignore +from sqlalchemy.ext.hybrid import hybrid_property +from sqlalchemy.orm import validates +from sqlalchemy.orm.exc import MultipleResultsFound, NoResultFound +from sqlalchemy.sql import expression +from sqlalchemy_utils.types.url import URLType +from trove_classifiers import sorted_classifiers + +from warehouse import db +from warehouse.accounts.models import User +from warehouse.classifiers.models import Classifier +from warehouse.integrations.vulnerabilities.models import VulnerabilityRecord +from warehouse.sitemap.models import SitemapMixin +from warehouse.utils import dotted_navigator +from warehouse.utils.attrs import make_repr +# TODO: Cleanup unused imports + +class OrganizationFactory: + def __init__(self, request): + self.request = request + + def __getitem__(self, organization): + try: + return ( + self.request.db.query(Organization) + .filter(Organization.name == organization) + .one() + ) + except NoResultFound: + raise KeyError from None + + +#TODO: Determine if this should also utilize SitemapMixin and TwoFactorRequireable +#class Project(SitemapMixin, TwoFactorRequireable, db.Model): +class Organization(db.Model): + __tablename__ = "organizations" + __table_args__ = ( + CheckConstraint( + "name ~* '^([A-Z0-9]|[A-Z0-9][A-Z0-9._-]*[A-Z0-9])$'::text", + name="organizations_valid_name", + ), + ) + + __repr__ = make_repr("name") + + name = Column(Text, nullable=False) + normalized_name = orm.column_property(func.normalize_pep426_name(name)) + display_name = Column(Text, nullable=False) + orgtype = Column(Text, nullable=False) + url = Column(URLType, nullable=False) + desc = Column(Text, nullable=False) + is_active = Column(Boolean) + created = Column( + DateTime(timezone=False), + nullable=False, + server_default=sql.func.now(), + index=True, + ) + + # TODO: Determine if cascade applies to any of these relationships + users = orm.relationship(User, secondary=OrganizationRole.__table__, backref="organizations") # many-to-many + projects = orm.relationship("Project", secondary=OrganizationProject.__table__, backref="organizations") # many-to-many + #teams = orm.relationship("Team", primaryjoin=lambda: sql.and_(Team.organization_id == Organization.id), backref="organizations") # one-to-many + #teams = orm.relationship("Team", primaryjoin="and_(Organization.id==Team.organization_id", backref="organizations") # one-to-many + +# TODO: +# def __getitem__(self, name): ??? +# def __acl__(self): +# Do we want any properties? + +class OrganizationNameCatalog(db.Model): + + __tablename__ = "organization_name_catalog" + __table_args__ = ( + Index("organization_name_catalog_name_idx", "name"), + Index("organization_name_catalog_organization_id_idx", "organization_id"), + UniqueConstraint("name", "organization_id", name="_organization_name_catalog_name_organization_uc"), + ) + + __repr__ = make_repr("name", "organization_id") + + name = Column(Text, nullable=False) + organization_id = Column( + ForeignKey("organizations.id", onupdate="CASCADE", ondelete="CASCADE"), + nullable=False, + ) + +class OrganizationRole(db.Model): + + __tablename__ = "organization_roles" + __table_args__ = ( + Index("organization_roles_user_id_idx", "user_id"), + Index("organization_roles_organization_id_idx", "organization_id"), + UniqueConstraint("user_id", "organization_id", name="_organization_roles_user_organization_uc"), + ) + + __repr__ = make_repr("role_name") + + role_name = Column(Text, nullable=False) + user_id = Column( + ForeignKey("users.id", onupdate="CASCADE", ondelete="CASCADE"), nullable=False + ) + organization_id = Column( + ForeignKey("organizations.id", onupdate="CASCADE", ondelete="CASCADE"), + nullable=False, + ) + + user = orm.relationship(User, lazy=False) + organization = orm.relationship("Organization", lazy=False) + +class OrganizationProject(db.Model): + + __tablename__ = "organization_project" + __table_args__ = ( + Index("organization_project_organization_id_idx", "organization_id"), + Index("organization_project_project_id_idx", "project_id"), + UniqueConstraint("organization_id", "project_id", name="_organization_project_organization_project_uc"), + ) + + __repr__ = make_repr("project_id", "organization_id", "is_active") + + is_active = Column(Boolean) + organization_id = Column( + ForeignKey("organizations.id", onupdate="CASCADE", ondelete="CASCADE"), nullable=False + ) + project_id = Column( + ForeignKey("projects.id", onupdate="CASCADE", ondelete="CASCADE"), + nullable=False, + ) + + organization = orm.relationship("Organization", lazy=False) + project = orm.relationship("Project", lazy=False) + +class OrganizationInvitationStatus(enum.Enum): + + Pending = "pending" + Expired = "expired" + + +class OrganizationInvitation(db.Model): + + __tablename__ = "organization_invitations" + __table_args__ = ( + Index("organization_invitations_user_id_idx", "user_id"), + UniqueConstraint( + "user_id", "organization_id", name="_organization_invitations_user_organization_uc" + ), + ) + + __repr__ = make_repr("invite_status", "user", "organization") + + invite_status = Column( + Enum(OrganizationInvitationStatus, values_callable=lambda x: [e.value for e in x]), + nullable=False, + ) + token = Column(Text, nullable=False) + user_id = Column( + ForeignKey("users.id", onupdate="CASCADE", ondelete="CASCADE"), + nullable=False, + index=True, + ) + organization_id = Column( + ForeignKey("organizations.id", onupdate="CASCADE", ondelete="CASCADE"), + nullable=False, + index=True, + ) + + user = orm.relationship(User, lazy=False) + organization = orm.relationship("Organization", lazy=False) \ No newline at end of file From 28edd0f07d5f7b1931d98701c9320a560f4f2ce8 Mon Sep 17 00:00:00 2001 From: sterbo Date: Sun, 10 Apr 2022 13:57:26 -0700 Subject: [PATCH 09/46] Ran code thru make reformat and lint (pypa#11070) --- warehouse/organizations/__init__.py | 2 +- warehouse/organizations/models.py | 194 ++++++++++++++-------------- 2 files changed, 99 insertions(+), 97 deletions(-) diff --git a/warehouse/organizations/__init__.py b/warehouse/organizations/__init__.py index 5847d0b1544f..7223de0e0995 100644 --- a/warehouse/organizations/__init__.py +++ b/warehouse/organizations/__init__.py @@ -10,4 +10,4 @@ # See the License for the specific language governing permissions and # limitations under the License. -# TODO: Nothing to see here yet. These aren't the droids you're looking for. \ No newline at end of file +# TODO: Nothing to see here yet. These aren't the droids you're looking for. diff --git a/warehouse/organizations/models.py b/warehouse/organizations/models.py index a546be8b21e7..579f8717cd22 100644 --- a/warehouse/organizations/models.py +++ b/warehouse/organizations/models.py @@ -12,69 +12,104 @@ import enum -from collections import OrderedDict -from urllib.parse import urlparse - -import packaging.utils - -from citext import CIText -from pyramid.authorization import Allow -from pyramid.threadlocal import get_current_request from sqlalchemy import ( - BigInteger, Boolean, CheckConstraint, Column, DateTime, Enum, - Float, ForeignKey, Index, - Integer, - String, - Table, Text, UniqueConstraint, func, orm, sql, ) -from sqlalchemy.dialects.postgresql import JSONB, UUID -from sqlalchemy.ext.associationproxy import association_proxy -from sqlalchemy.ext.declarative import declared_attr # type: ignore -from sqlalchemy.ext.hybrid import hybrid_property -from sqlalchemy.orm import validates -from sqlalchemy.orm.exc import MultipleResultsFound, NoResultFound -from sqlalchemy.sql import expression + +# from sqlalchemy.orm.exc import NoResultFound from sqlalchemy_utils.types.url import URLType -from trove_classifiers import sorted_classifiers from warehouse import db from warehouse.accounts.models import User -from warehouse.classifiers.models import Classifier -from warehouse.integrations.vulnerabilities.models import VulnerabilityRecord -from warehouse.sitemap.models import SitemapMixin -from warehouse.utils import dotted_navigator from warehouse.utils.attrs import make_repr -# TODO: Cleanup unused imports -class OrganizationFactory: - def __init__(self, request): - self.request = request - def __getitem__(self, organization): - try: - return ( - self.request.db.query(Organization) - .filter(Organization.name == organization) - .one() - ) - except NoResultFound: - raise KeyError from None +class OrganizationRole(db.Model): + + __tablename__ = "organization_roles" + __table_args__ = ( + Index("organization_roles_user_id_idx", "user_id"), + Index("organization_roles_organization_id_idx", "organization_id"), + UniqueConstraint( + "user_id", + "organization_id", + name="_organization_roles_user_organization_uc", + ), + ) + + __repr__ = make_repr("role_name") + + role_name = Column(Text, nullable=False) + user_id = Column( + ForeignKey("users.id", onupdate="CASCADE", ondelete="CASCADE"), nullable=False + ) + organization_id = Column( + ForeignKey("organizations.id", onupdate="CASCADE", ondelete="CASCADE"), + nullable=False, + ) + + user = orm.relationship(User, lazy=False) + organization = orm.relationship("Organization", lazy=False) + + +class OrganizationProject(db.Model): + + __tablename__ = "organization_project" + __table_args__ = ( + Index("organization_project_organization_id_idx", "organization_id"), + Index("organization_project_project_id_idx", "project_id"), + UniqueConstraint( + "organization_id", + "project_id", + name="_organization_project_organization_project_uc", + ), + ) + + __repr__ = make_repr("project_id", "organization_id", "is_active") + + is_active = Column(Boolean) + organization_id = Column( + ForeignKey("organizations.id", onupdate="CASCADE", ondelete="CASCADE"), + nullable=False, + ) + project_id = Column( + ForeignKey("projects.id", onupdate="CASCADE", ondelete="CASCADE"), + nullable=False, + ) + + organization = orm.relationship("Organization", lazy=False) + project = orm.relationship("Project", lazy=False) -#TODO: Determine if this should also utilize SitemapMixin and TwoFactorRequireable -#class Project(SitemapMixin, TwoFactorRequireable, db.Model): +# TODO: Determine if we will need a factory class +# class OrganizationFactory: +# def __init__(self, request): +# self.request = request +# +# def __getitem__(self, organization): +# try: +# return ( +# self.request.db.query(Organization) +# .filter(Organization.name == organization) +# .one() +# ) +# except NoResultFound: +# raise KeyError from None + + +# TODO: Determine if this should also utilize SitemapMixin and TwoFactorRequireable +# class Project(SitemapMixin, TwoFactorRequireable, db.Model): class Organization(db.Model): __tablename__ = "organizations" __table_args__ = ( @@ -99,25 +134,33 @@ class Organization(db.Model): server_default=sql.func.now(), index=True, ) - - # TODO: Determine if cascade applies to any of these relationships - users = orm.relationship(User, secondary=OrganizationRole.__table__, backref="organizations") # many-to-many - projects = orm.relationship("Project", secondary=OrganizationProject.__table__, backref="organizations") # many-to-many - #teams = orm.relationship("Team", primaryjoin=lambda: sql.and_(Team.organization_id == Organization.id), backref="organizations") # one-to-many - #teams = orm.relationship("Team", primaryjoin="and_(Organization.id==Team.organization_id", backref="organizations") # one-to-many + + # TODO: Determine if cascade applies to any of these relationships + users = orm.relationship( + User, secondary=OrganizationRole.__table__, backref="organizations" # type: ignore # noqa + ) # many-to-many + projects = orm.relationship( + "Project", secondary=OrganizationProject.__table__, backref="organizations" # type: ignore # noqa + ) # many-to-many + # TODO: # def __getitem__(self, name): ??? -# def __acl__(self): +# def __acl__(self): # Do we want any properties? + class OrganizationNameCatalog(db.Model): __tablename__ = "organization_name_catalog" __table_args__ = ( Index("organization_name_catalog_name_idx", "name"), Index("organization_name_catalog_organization_id_idx", "organization_id"), - UniqueConstraint("name", "organization_id", name="_organization_name_catalog_name_organization_uc"), + UniqueConstraint( + "name", + "organization_id", + name="_organization_name_catalog_name_organization_uc", + ), ) __repr__ = make_repr("name", "organization_id") @@ -128,51 +171,6 @@ class OrganizationNameCatalog(db.Model): nullable=False, ) -class OrganizationRole(db.Model): - - __tablename__ = "organization_roles" - __table_args__ = ( - Index("organization_roles_user_id_idx", "user_id"), - Index("organization_roles_organization_id_idx", "organization_id"), - UniqueConstraint("user_id", "organization_id", name="_organization_roles_user_organization_uc"), - ) - - __repr__ = make_repr("role_name") - - role_name = Column(Text, nullable=False) - user_id = Column( - ForeignKey("users.id", onupdate="CASCADE", ondelete="CASCADE"), nullable=False - ) - organization_id = Column( - ForeignKey("organizations.id", onupdate="CASCADE", ondelete="CASCADE"), - nullable=False, - ) - - user = orm.relationship(User, lazy=False) - organization = orm.relationship("Organization", lazy=False) - -class OrganizationProject(db.Model): - - __tablename__ = "organization_project" - __table_args__ = ( - Index("organization_project_organization_id_idx", "organization_id"), - Index("organization_project_project_id_idx", "project_id"), - UniqueConstraint("organization_id", "project_id", name="_organization_project_organization_project_uc"), - ) - - __repr__ = make_repr("project_id", "organization_id", "is_active") - - is_active = Column(Boolean) - organization_id = Column( - ForeignKey("organizations.id", onupdate="CASCADE", ondelete="CASCADE"), nullable=False - ) - project_id = Column( - ForeignKey("projects.id", onupdate="CASCADE", ondelete="CASCADE"), - nullable=False, - ) - - organization = orm.relationship("Organization", lazy=False) - project = orm.relationship("Project", lazy=False) class OrganizationInvitationStatus(enum.Enum): @@ -186,14 +184,18 @@ class OrganizationInvitation(db.Model): __table_args__ = ( Index("organization_invitations_user_id_idx", "user_id"), UniqueConstraint( - "user_id", "organization_id", name="_organization_invitations_user_organization_uc" + "user_id", + "organization_id", + name="_organization_invitations_user_organization_uc", ), ) __repr__ = make_repr("invite_status", "user", "organization") invite_status = Column( - Enum(OrganizationInvitationStatus, values_callable=lambda x: [e.value for e in x]), + Enum( + OrganizationInvitationStatus, values_callable=lambda x: [e.value for e in x] + ), nullable=False, ) token = Column(Text, nullable=False) @@ -209,4 +211,4 @@ class OrganizationInvitation(db.Model): ) user = orm.relationship(User, lazy=False) - organization = orm.relationship("Organization", lazy=False) \ No newline at end of file + organization = orm.relationship("Organization", lazy=False) From 9de28f6b0729c308fc4dccea60c0af4f8162651b Mon Sep 17 00:00:00 2001 From: sterbo Date: Sun, 10 Apr 2022 14:09:10 -0700 Subject: [PATCH 10/46] Added tests for new db models (pypa#11070) --- tests/common/db/organizations.py | 88 ++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 tests/common/db/organizations.py diff --git a/tests/common/db/organizations.py b/tests/common/db/organizations.py new file mode 100644 index 000000000000..cc439b738296 --- /dev/null +++ b/tests/common/db/organizations.py @@ -0,0 +1,88 @@ +# 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. + +import datetime + +import factory +import faker + +from warehouse.rganizations.models import ( + Organization, + OrganizationNameCatalog, + OrganizationProject, + OrganizationRole, + OrganizationRoleInvitation, +) + +from .accounts import UserFactory +from .base import WarehouseFactory +from .packaging import ProjectFactory + +fake = faker.Faker() + + +class OrganizationFactory(WarehouseFactory): + class Meta: + model = Organization + + id = factory.Faker("uuid4", cast_to=None) + name = factory.Faker("name") + normalized_name = factory.Faker("norm") + display_name = factory.Faker("display") + orgtype = factory.Faker("sometype") + url = factory.Faker("uri") + desc = factory.Faker("description") + is_active = True + created = factory.Faker( + "date_time_between_dates", + datetime_start=datetime.datetime(2020, 1, 1), + datetime_end=datetime.datetime(2022, 1, 1), + ) + user = factory.SubFactory(UserFactory) + project = factory.SubFactory(ProjectFactory) + + +class OrganizationNameCatalogFactory(WarehouseFactory): + class Meta: + model = OrganizationNameCatalog + + name = factory.Faker("orgname") + organization_id = factory.Faker("uuid4", cast_to=None) + + +class OrganizationRoleFactory(WarehouseFactory): + class Meta: + model = OrganizationRole + + role_name = "Owner" + user = factory.SubFactory(UserFactory) + organization = factory.SubFactory(OrganizationFactory) + + +class OrganizationRoleInvitationFactory(WarehouseFactory): + class Meta: + model = OrganizationRoleInvitation + + invite_status = "pending" + token = "test_token" + user = factory.SubFactory(UserFactory) + organization = factory.SubFactory(OrganizationFactory) + + +class OrganizationProjectFactory(WarehouseFactory): + class Meta: + model = OrganizationProject + + id = factory.Faker("uuid4", cast_to=None) + is_active = True + organization = factory.SubFactory(OrganizationFactory) + project = factory.SubFactory(ProjectFactory) From 6efcb99fa64fc70dffb218f85473ee83399b0927 Mon Sep 17 00:00:00 2001 From: sterbo Date: Tue, 12 Apr 2022 12:18:33 -0700 Subject: [PATCH 11/46] Numerous tweaks to db models --- tests/common/db/organizations.py | 6 +++--- warehouse/organizations/models.py | 21 ++++++++++++++++++--- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/tests/common/db/organizations.py b/tests/common/db/organizations.py index cc439b738296..9d1a8164eb16 100644 --- a/tests/common/db/organizations.py +++ b/tests/common/db/organizations.py @@ -38,9 +38,9 @@ class Meta: name = factory.Faker("name") normalized_name = factory.Faker("norm") display_name = factory.Faker("display") - orgtype = factory.Faker("sometype") - url = factory.Faker("uri") - desc = factory.Faker("description") + orgtype = factory.Faker("Community") + link_url = factory.Faker("uri") + description = factory.Faker("description") is_active = True created = factory.Faker( "date_time_between_dates", diff --git a/warehouse/organizations/models.py b/warehouse/organizations/models.py index 579f8717cd22..cf52e5241868 100644 --- a/warehouse/organizations/models.py +++ b/warehouse/organizations/models.py @@ -108,6 +108,12 @@ class OrganizationProject(db.Model): # raise KeyError from None +class OrganizationType(enum.Enum): + + Pending = "Community" + Expired = "Company" + + # TODO: Determine if this should also utilize SitemapMixin and TwoFactorRequireable # class Project(SitemapMixin, TwoFactorRequireable, db.Model): class Organization(db.Model): @@ -124,16 +130,25 @@ class Organization(db.Model): name = Column(Text, nullable=False) normalized_name = orm.column_property(func.normalize_pep426_name(name)) display_name = Column(Text, nullable=False) - orgtype = Column(Text, nullable=False) - url = Column(URLType, nullable=False) - desc = Column(Text, nullable=False) + orgtype = Column( + Enum(OrganizationType, values_callable=lambda x: [e.value for e in x]), + nullable=False, + ) + link_url = Column(URLType, nullable=False) + description = Column(Text, nullable=False) is_active = Column(Boolean) + is_approved = Column(Boolean) created = Column( DateTime(timezone=False), nullable=False, server_default=sql.func.now(), index=True, ) + date_approved = Column( + DateTime(timezone=False), + nullable=True, + onupdate=func.now(), + ) # TODO: Determine if cascade applies to any of these relationships users = orm.relationship( From 086e2c7d94c642133986c3612c004409e066c188 Mon Sep 17 00:00:00 2001 From: sterbo Date: Tue, 12 Apr 2022 15:21:45 -0700 Subject: [PATCH 12/46] Require value for is_active and ran reformat. --- warehouse/organizations/models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/warehouse/organizations/models.py b/warehouse/organizations/models.py index cf52e5241868..cf06d44d153d 100644 --- a/warehouse/organizations/models.py +++ b/warehouse/organizations/models.py @@ -78,7 +78,7 @@ class OrganizationProject(db.Model): __repr__ = make_repr("project_id", "organization_id", "is_active") - is_active = Column(Boolean) + is_active = Column(Boolean, nullable=False, default=False) organization_id = Column( ForeignKey("organizations.id", onupdate="CASCADE", ondelete="CASCADE"), nullable=False, @@ -136,7 +136,7 @@ class Organization(db.Model): ) link_url = Column(URLType, nullable=False) description = Column(Text, nullable=False) - is_active = Column(Boolean) + is_active = Column(Boolean, nullable=False, default=False) is_approved = Column(Boolean) created = Column( DateTime(timezone=False), From df033eb9bfba67761aab9c7553674f1d2f0c6685 Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Wed, 13 Apr 2022 08:15:46 -0700 Subject: [PATCH 13/46] Regenerate migrations for Organization models docker-compose run web python -m warehouse db revision --autogenerate --message "Create Organization models" make reformat Forced to regenerate migrations due to recent database changes (#11157). --- ...614a7fcb40ed_create_organization_models.py | 274 ++++++++++++++++++ 1 file changed, 274 insertions(+) create mode 100644 warehouse/migrations/versions/614a7fcb40ed_create_organization_models.py diff --git a/warehouse/migrations/versions/614a7fcb40ed_create_organization_models.py b/warehouse/migrations/versions/614a7fcb40ed_create_organization_models.py new file mode 100644 index 000000000000..4a8b238687ca --- /dev/null +++ b/warehouse/migrations/versions/614a7fcb40ed_create_organization_models.py @@ -0,0 +1,274 @@ +# 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. +""" +Create Organization models + +Revision ID: 614a7fcb40ed +Revises: 84262e097c26 +Create Date: 2022-04-13 17:23:17.396325 +""" + +import sqlalchemy as sa + +from alembic import op +from sqlalchemy.dialects import postgresql + +revision = "614a7fcb40ed" +down_revision = "84262e097c26" + +# Note: It is VERY important to ensure that a migration does not lock for a +# long period of time and to ensure that each individual migration does +# not break compatibility with the *previous* version of the code base. +# This is because the migrations will be ran automatically as part of the +# deployment process, but while the previous version of the code is still +# up and running. Thus backwards incompatible changes must be broken up +# over multiple migrations inside of multiple pull requests in order to +# phase them in over multiple deploys. + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_table( + "organizations", + sa.Column( + "id", + postgresql.UUID(as_uuid=True), + server_default=sa.text("gen_random_uuid()"), + nullable=False, + ), + sa.Column("name", sa.Text(), nullable=False), + sa.Column("display_name", sa.Text(), nullable=False), + sa.Column( + "orgtype", + sa.Enum("Community", "Company", name="organizationtype"), + nullable=False, + ), + sa.Column("link_url", sqlalchemy_utils.types.url.URLType(), nullable=False), + sa.Column("description", sa.Text(), nullable=False), + sa.Column("is_active", sa.Boolean(), nullable=False), + sa.Column("is_approved", sa.Boolean(), nullable=True), + sa.Column( + "created", sa.DateTime(), server_default=sa.text("now()"), nullable=False + ), + sa.Column("date_approved", sa.DateTime(), nullable=True), + sa.CheckConstraint( + "name ~* '^([A-Z0-9]|[A-Z0-9][A-Z0-9._-]*[A-Z0-9])$'::text", + name="organizations_valid_name", + ), + sa.PrimaryKeyConstraint("id"), + ) + op.create_index( + op.f("ix_organizations_created"), "organizations", ["created"], unique=False + ) + op.create_table( + "organization_invitations", + sa.Column( + "id", + postgresql.UUID(as_uuid=True), + server_default=sa.text("gen_random_uuid()"), + nullable=False, + ), + sa.Column( + "invite_status", + sa.Enum("pending", "expired", name="organizationinvitationstatus"), + nullable=False, + ), + sa.Column("token", sa.Text(), nullable=False), + sa.Column("user_id", postgresql.UUID(as_uuid=True), nullable=False), + sa.Column("organization_id", postgresql.UUID(as_uuid=True), nullable=False), + sa.ForeignKeyConstraint( + ["organization_id"], + ["organizations.id"], + onupdate="CASCADE", + ondelete="CASCADE", + ), + sa.ForeignKeyConstraint( + ["user_id"], ["users.id"], onupdate="CASCADE", ondelete="CASCADE" + ), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint( + "user_id", + "organization_id", + name="_organization_invitations_user_organization_uc", + ), + ) + op.create_index( + op.f("ix_organization_invitations_organization_id"), + "organization_invitations", + ["organization_id"], + unique=False, + ) + op.create_index( + op.f("ix_organization_invitations_user_id"), + "organization_invitations", + ["user_id"], + unique=False, + ) + op.create_index( + "organization_invitations_user_id_idx", + "organization_invitations", + ["user_id"], + unique=False, + ) + op.create_table( + "organization_name_catalog", + sa.Column( + "id", + postgresql.UUID(as_uuid=True), + server_default=sa.text("gen_random_uuid()"), + nullable=False, + ), + sa.Column("name", sa.Text(), nullable=False), + sa.Column("organization_id", postgresql.UUID(as_uuid=True), nullable=False), + sa.ForeignKeyConstraint( + ["organization_id"], + ["organizations.id"], + onupdate="CASCADE", + ondelete="CASCADE", + ), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint( + "name", + "organization_id", + name="_organization_name_catalog_name_organization_uc", + ), + ) + op.create_index( + "organization_name_catalog_name_idx", + "organization_name_catalog", + ["name"], + unique=False, + ) + op.create_index( + "organization_name_catalog_organization_id_idx", + "organization_name_catalog", + ["organization_id"], + unique=False, + ) + op.create_table( + "organization_project", + sa.Column( + "id", + postgresql.UUID(as_uuid=True), + server_default=sa.text("gen_random_uuid()"), + nullable=False, + ), + sa.Column("is_active", sa.Boolean(), nullable=False), + sa.Column("organization_id", postgresql.UUID(as_uuid=True), nullable=False), + sa.Column("project_id", postgresql.UUID(as_uuid=True), nullable=False), + sa.ForeignKeyConstraint( + ["organization_id"], + ["organizations.id"], + onupdate="CASCADE", + ondelete="CASCADE", + ), + sa.ForeignKeyConstraint( + ["project_id"], ["projects.id"], onupdate="CASCADE", ondelete="CASCADE" + ), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint( + "organization_id", + "project_id", + name="_organization_project_organization_project_uc", + ), + ) + op.create_index( + "organization_project_organization_id_idx", + "organization_project", + ["organization_id"], + unique=False, + ) + op.create_index( + "organization_project_project_id_idx", + "organization_project", + ["project_id"], + unique=False, + ) + op.create_table( + "organization_roles", + sa.Column( + "id", + postgresql.UUID(as_uuid=True), + server_default=sa.text("gen_random_uuid()"), + nullable=False, + ), + sa.Column("role_name", sa.Text(), nullable=False), + sa.Column("user_id", postgresql.UUID(as_uuid=True), nullable=False), + sa.Column("organization_id", postgresql.UUID(as_uuid=True), nullable=False), + sa.ForeignKeyConstraint( + ["organization_id"], + ["organizations.id"], + onupdate="CASCADE", + ondelete="CASCADE", + ), + sa.ForeignKeyConstraint( + ["user_id"], ["users.id"], onupdate="CASCADE", ondelete="CASCADE" + ), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint( + "user_id", + "organization_id", + name="_organization_roles_user_organization_uc", + ), + ) + op.create_index( + "organization_roles_organization_id_idx", + "organization_roles", + ["organization_id"], + unique=False, + ) + op.create_index( + "organization_roles_user_id_idx", + "organization_roles", + ["user_id"], + unique=False, + ) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_index("organization_roles_user_id_idx", table_name="organization_roles") + op.drop_index( + "organization_roles_organization_id_idx", table_name="organization_roles" + ) + op.drop_table("organization_roles") + op.drop_index( + "organization_project_project_id_idx", table_name="organization_project" + ) + op.drop_index( + "organization_project_organization_id_idx", table_name="organization_project" + ) + op.drop_table("organization_project") + op.drop_index( + "organization_name_catalog_organization_id_idx", + table_name="organization_name_catalog", + ) + op.drop_index( + "organization_name_catalog_name_idx", table_name="organization_name_catalog" + ) + op.drop_table("organization_name_catalog") + op.drop_index( + "organization_invitations_user_id_idx", table_name="organization_invitations" + ) + op.drop_index( + op.f("ix_organization_invitations_user_id"), + table_name="organization_invitations", + ) + op.drop_index( + op.f("ix_organization_invitations_organization_id"), + table_name="organization_invitations", + ) + op.drop_table("organization_invitations") + op.drop_index(op.f("ix_organizations_created"), table_name="organizations") + op.drop_table("organizations") + # ### end Alembic commands ### From 054b6c7017dd041a56f2100bdfb79c233447ab2f Mon Sep 17 00:00:00 2001 From: sterbo Date: Tue, 12 Apr 2022 12:18:33 -0700 Subject: [PATCH 14/46] Numerous tweaks to alembic scripts --- ...614a7fcb40ed_create_organization_models.py | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/warehouse/migrations/versions/614a7fcb40ed_create_organization_models.py b/warehouse/migrations/versions/614a7fcb40ed_create_organization_models.py index 4a8b238687ca..2ee6db3cf4d7 100644 --- a/warehouse/migrations/versions/614a7fcb40ed_create_organization_models.py +++ b/warehouse/migrations/versions/614a7fcb40ed_create_organization_models.py @@ -18,6 +18,7 @@ """ import sqlalchemy as sa +import sqlalchemy_utils from alembic import op from sqlalchemy.dialects import postgresql @@ -47,14 +48,12 @@ def upgrade(): ), sa.Column("name", sa.Text(), nullable=False), sa.Column("display_name", sa.Text(), nullable=False), - sa.Column( - "orgtype", - sa.Enum("Community", "Company", name="organizationtype"), - nullable=False, - ), + sa.Column("orgtype", sa.Text(), nullable=False), sa.Column("link_url", sqlalchemy_utils.types.url.URLType(), nullable=False), sa.Column("description", sa.Text(), nullable=False), - sa.Column("is_active", sa.Boolean(), nullable=False), + sa.Column( + "is_active", sa.Boolean(), nullable=False, server_default=sa.sql.false() + ), sa.Column("is_approved", sa.Boolean(), nullable=True), sa.Column( "created", sa.DateTime(), server_default=sa.text("now()"), nullable=False @@ -77,11 +76,7 @@ def upgrade(): server_default=sa.text("gen_random_uuid()"), nullable=False, ), - sa.Column( - "invite_status", - sa.Enum("pending", "expired", name="organizationinvitationstatus"), - nullable=False, - ), + sa.Column("invite_status", sa.Text(), nullable=False), sa.Column("token", sa.Text(), nullable=False), sa.Column("user_id", postgresql.UUID(as_uuid=True), nullable=False), sa.Column("organization_id", postgresql.UUID(as_uuid=True), nullable=False), @@ -162,7 +157,9 @@ def upgrade(): server_default=sa.text("gen_random_uuid()"), nullable=False, ), - sa.Column("is_active", sa.Boolean(), nullable=False), + sa.Column( + "is_active", sa.Boolean(), nullable=False, server_default=sa.sql.false() + ), sa.Column("organization_id", postgresql.UUID(as_uuid=True), nullable=False), sa.Column("project_id", postgresql.UUID(as_uuid=True), nullable=False), sa.ForeignKeyConstraint( From 540bce5436a0d0a5ae4aacd218d82e98d6163b9a Mon Sep 17 00:00:00 2001 From: sterbo Date: Tue, 12 Apr 2022 19:38:42 -0700 Subject: [PATCH 15/46] Initial implementation of organization service. --- tests/unit/organizations/__init__.py | 18 ++++ warehouse/organizations/interfaces.py | 67 ++++++++++++ warehouse/organizations/services.py | 141 ++++++++++++++++++++++++++ 3 files changed, 226 insertions(+) create mode 100644 tests/unit/organizations/__init__.py create mode 100644 warehouse/organizations/interfaces.py create mode 100644 warehouse/organizations/services.py diff --git a/tests/unit/organizations/__init__.py b/tests/unit/organizations/__init__.py new file mode 100644 index 000000000000..9b5fe147153d --- /dev/null +++ b/tests/unit/organizations/__init__.py @@ -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. + +from warehouse.organizations.interfaces import IOrganizationService +from warehouse.organizations.services import database_organization_factory + + +def includeme(config): + config.register_service_factory(database_organization_factory, IOrganizationService) diff --git a/warehouse/organizations/interfaces.py b/warehouse/organizations/interfaces.py new file mode 100644 index 000000000000..258b1e0e9004 --- /dev/null +++ b/warehouse/organizations/interfaces.py @@ -0,0 +1,67 @@ +# 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. + +from zope.interface import Interface + + +class IOrganizationService(Interface): + def get_organization(organization_id): + """ + Return the organization object that represents the given organizationid, or None if + there is no organization for that ID. + """ + + def get_organization_by_name(name): + """ + Return the organization object corresponding with the given organization name, or None + if there is no organization with that name. + """ + + def find_organizationid(name): + """ + Find the unique organization identifier for the given name or None if there + is no organization with the given name. + """ + + def add_organization(name, display_name, orgtype, link_url, description): + """ + Accepts a organization object, and attempts to create an organization with those + attributes. + """ + + def add_catalog_entry(name, organization_id): + """ + Adds the organization name to the organization name catalog + """ + + def add_organization_role(role_name, user_id, organization_id): + """ + Adds the organization role to the specified user and org + """ + + def approve_organization(organization_id): + """ + Performs operations necessary to approve an organization + """ + + def decline_organization(organization_id): + """ + Performs operations necessary to reject approval of an organization + """ + + def record_event(organization_id, *, tag, additional=None): + """ + Creates a new OrganizationEvent for the given organization with the given + tag, IP address, and additional metadata. + + Returns the event. + """ diff --git a/warehouse/organizations/services.py b/warehouse/organizations/services.py new file mode 100644 index 000000000000..886dc1e5e116 --- /dev/null +++ b/warehouse/organizations/services.py @@ -0,0 +1,141 @@ +# 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. + +import datetime + +from sqlalchemy.orm.exc import NoResultFound +from zope.interface import implementer + +from warehouse.organizations.interfaces import IOrganizationService +from warehouse.organizations.models import ( + Organization, + OrganizationNameCatalog, + OrganizationRole, +) + + +@implementer(IOrganizationService) +class DatabaseOrganizationService: + def __init__(self, db_session): + self.db = db_session + + def get_organization(self, organization_id): + """ + Return the organization object that represents the given organizationid, + or None if there is no organization for that ID. + """ + return self.db.query(Organization).get(organization_id) + + def get_organization_by_name(self, name): + """ + Return the organization object corresponding with the given organization name, + or None if there is no organization with that name. + """ + organization_id = self.find_organizationid(name) + return ( + None if organization_id is None else self.get_organization(organization_id) + ) + + def find_organizationid(self, name): + """ + Find the unique organization identifier for the given name or None if there + is no organization with the given name. + """ + try: + organization = ( + self.db.query(Organization.id).filter(Organization.name == name).one() + ) + except NoResultFound: + return + + return organization.id + + def add_organization(self, name, display_name, orgtype, link_url, description): + """ + Accepts a organization object, and attempts to create an organization with those + attributes. + """ + organization = Organization( + name=name, + display_name=display_name, + orgtype=orgtype, + link_url=link_url, + description=description, + ) + self.db.add(organization) + self.db.flush() + + return organization + + def add_catalog_entry(self, name, organization_id): + """ + Adds the organization name to the organization name catalog + """ + organization = self.get_organization(organization_id) + catalog_entry = OrganizationNameCatalog( + name=name, organization_id=organization.id + ) + + self.db.add(catalog_entry) + self.db.flush() + + return catalog_entry + + def add_organization_role(self, role_name, user_id, organization_id): + """ + Adds the organization role to the specified user and org + """ + organization = self.get_organization(organization_id) + role = OrganizationRole( + role_name=role_name, user_id=user_id, organization_id=organization.id + ) + + self.db.add(role) + self.db.flush() + + return role + + def approve_organization(self, organization_id): + """ + Performs operations necessary to approve an Organization + """ + organization = self.get_organization(organization_id) + organization.is_active = True + organization.is_approved = True + organization.date_approved = datetime.datetime.now() + # self.db.flush() + + return organization + + def decline_organization(self, organization_id): + """ + Performs operations necessary to reject approval of an Organization + """ + organization = self.get_organization(organization_id) + organization.is_approved = False + organization.date_approved = datetime.datetime.now() + # self.db.flush() + + return organization + + def record_event(self, organization_id, *, tag, additional=None): + """ + Creates a new OrganizationEvent for the given organization with the given + tag, IP address, and additional metadata. + + Returns the event. + """ + raise NotImplementedError + + +def database_organization_factory(context, request): + return DatabaseOrganizationService(request.db) From 0a95425c609728aba828587a96ac872a1afce7c9 Mon Sep 17 00:00:00 2001 From: sterbo Date: Wed, 13 Apr 2022 19:36:31 -0700 Subject: [PATCH 16/46] Implementing organization events functionality. --- ...85d158c3c_add_organization_events_table.py | 78 +++++++++++++++++++ warehouse/organizations/models.py | 59 ++++++++------ warehouse/organizations/services.py | 10 ++- 3 files changed, 122 insertions(+), 25 deletions(-) create mode 100644 warehouse/migrations/versions/4a985d158c3c_add_organization_events_table.py diff --git a/warehouse/migrations/versions/4a985d158c3c_add_organization_events_table.py b/warehouse/migrations/versions/4a985d158c3c_add_organization_events_table.py new file mode 100644 index 000000000000..02b22b512a09 --- /dev/null +++ b/warehouse/migrations/versions/4a985d158c3c_add_organization_events_table.py @@ -0,0 +1,78 @@ +# 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. +""" +add_organization_events_table + +Revision ID: 4a985d158c3c +Revises: 614a7fcb40ed +Create Date: 2022-04-14 02:25:50.805348 +""" + +import sqlalchemy as sa + +from alembic import op +from sqlalchemy.dialects import postgresql + +revision = "4a985d158c3c" +down_revision = "614a7fcb40ed" + +# Note: It is VERY important to ensure that a migration does not lock for a +# long period of time and to ensure that each individual migration does +# not break compatibility with the *previous* version of the code base. +# This is because the migrations will be ran automatically as part of the +# deployment process, but while the previous version of the code is still +# up and running. Thus backwards incompatible changes must be broken up +# over multiple migrations inside of multiple pull requests in order to +# phase them in over multiple deploys. + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_table( + "organization_events", + sa.Column( + "id", + postgresql.UUID(as_uuid=True), + server_default=sa.text("gen_random_uuid()"), + nullable=False, + ), + sa.Column("organization_id", postgresql.UUID(as_uuid=True), nullable=False), + sa.Column("tag", sa.String(), nullable=False), + sa.Column( + "time", sa.DateTime(), server_default=sa.text("now()"), nullable=False + ), + sa.Column("ip_address", sa.String(), nullable=False), + sa.Column("additional", postgresql.JSONB(astext_type=sa.Text()), nullable=True), + sa.ForeignKeyConstraint( + ["organization_id"], + ["organizations.id"], + initially="DEFERRED", + deferrable=True, + ), + sa.PrimaryKeyConstraint("id"), + ) + op.create_index( + op.f("ix_organization_events_organization_id"), + "organization_events", + ["organization_id"], + unique=False, + ) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_index( + op.f("ix_organization_events_organization_id"), table_name="organization_events" + ) + op.drop_table("organization_events") + # ### end Alembic commands ### diff --git a/warehouse/organizations/models.py b/warehouse/organizations/models.py index cf06d44d153d..a5ab8d31426c 100644 --- a/warehouse/organizations/models.py +++ b/warehouse/organizations/models.py @@ -20,12 +20,14 @@ Enum, ForeignKey, Index, + String, Text, UniqueConstraint, func, orm, sql, ) +from sqlalchemy.dialects.postgresql import JSONB, UUID # from sqlalchemy.orm.exc import NoResultFound from sqlalchemy_utils.types.url import URLType @@ -92,22 +94,6 @@ class OrganizationProject(db.Model): project = orm.relationship("Project", lazy=False) -# TODO: Determine if we will need a factory class -# class OrganizationFactory: -# def __init__(self, request): -# self.request = request -# -# def __getitem__(self, organization): -# try: -# return ( -# self.request.db.query(Organization) -# .filter(Organization.name == organization) -# .one() -# ) -# except NoResultFound: -# raise KeyError from None - - class OrganizationType(enum.Enum): Pending = "Community" @@ -153,16 +139,45 @@ class Organization(db.Model): # TODO: Determine if cascade applies to any of these relationships users = orm.relationship( User, secondary=OrganizationRole.__table__, backref="organizations" # type: ignore # noqa - ) # many-to-many + ) projects = orm.relationship( "Project", secondary=OrganizationProject.__table__, backref="organizations" # type: ignore # noqa - ) # many-to-many + ) + + events = orm.relationship( + "OrganizationEvent", + backref="organizations", + cascade="all, delete-orphan", + lazy=True, + ) + + # TODO: + # def __acl__(self): + def record_event(self, *, tag, ip_address, additional): + session = orm.object_session(self) + organization = OrganizationEvent( + user=self, tag=tag, ip_address=ip_address, additional=additional + ) + session.add(organization) + session.flush() -# TODO: -# def __getitem__(self, name): ??? -# def __acl__(self): -# Do we want any properties? + return organization + + +class OrganizationEvent(db.Model): + __tablename__ = "organization_events" + + organization_id = Column( + UUID(as_uuid=True), + ForeignKey("organizations.id", deferrable=True, initially="DEFERRED"), + nullable=False, + index=True, + ) + tag = Column(String, nullable=False) + time = Column(DateTime, nullable=False, server_default=sql.func.now()) + ip_address = Column(String, nullable=False) + additional = Column(JSONB, nullable=True) class OrganizationNameCatalog(db.Model): diff --git a/warehouse/organizations/services.py b/warehouse/organizations/services.py index 886dc1e5e116..43fbaf42c342 100644 --- a/warehouse/organizations/services.py +++ b/warehouse/organizations/services.py @@ -25,8 +25,9 @@ @implementer(IOrganizationService) class DatabaseOrganizationService: - def __init__(self, db_session): + def __init__(self, db_session, remote_addr): self.db = db_session + self.remote_addr = remote_addr def get_organization(self, organization_id): """ @@ -134,8 +135,11 @@ def record_event(self, organization_id, *, tag, additional=None): Returns the event. """ - raise NotImplementedError + organization = self.get_organization(organization_id) + return organization.record_event( + tag=tag, ip_address=self.remote_addr, additional=additional + ) def database_organization_factory(context, request): - return DatabaseOrganizationService(request.db) + return DatabaseOrganizationService(request.db, remote_addr=request.remote_addr) From e0eb6e2302df4be71cebada7d9e18e3e4ae8c331 Mon Sep 17 00:00:00 2001 From: sterbo Date: Thu, 14 Apr 2022 12:38:36 -0700 Subject: [PATCH 17/46] Added tests for organization services. --- tests/common/db/organizations.py | 31 ++++-- tests/conftest.py | 8 ++ tests/unit/organizations/test_services.py | 121 ++++++++++++++++++++++ 3 files changed, 149 insertions(+), 11 deletions(-) create mode 100644 tests/unit/organizations/test_services.py diff --git a/tests/common/db/organizations.py b/tests/common/db/organizations.py index 9d1a8164eb16..51373eaa6c60 100644 --- a/tests/common/db/organizations.py +++ b/tests/common/db/organizations.py @@ -15,12 +15,13 @@ import factory import faker -from warehouse.rganizations.models import ( +from warehouse.organizations.models import ( Organization, + OrganizationEvent, + OrganizationInvitation, OrganizationNameCatalog, OrganizationProject, OrganizationRole, - OrganizationRoleInvitation, ) from .accounts import UserFactory @@ -35,20 +36,28 @@ class Meta: model = Organization id = factory.Faker("uuid4", cast_to=None) - name = factory.Faker("name") - normalized_name = factory.Faker("norm") - display_name = factory.Faker("display") - orgtype = factory.Faker("Community") + name = factory.Faker("word") + display_name = factory.Faker("word") + orgtype = "Community" link_url = factory.Faker("uri") - description = factory.Faker("description") + description = factory.Faker("sentence") is_active = True + is_approved = False created = factory.Faker( "date_time_between_dates", datetime_start=datetime.datetime(2020, 1, 1), datetime_end=datetime.datetime(2022, 1, 1), ) - user = factory.SubFactory(UserFactory) - project = factory.SubFactory(ProjectFactory) + date_approved = factory.Faker( + "date_time_between_dates", datetime_start=datetime.datetime(2020, 1, 1) + ) + + +class OrganizationEventFactory(WarehouseFactory): + class Meta: + model = OrganizationEvent + + organization = factory.SubFactory(OrganizationFactory) class OrganizationNameCatalogFactory(WarehouseFactory): @@ -68,9 +77,9 @@ class Meta: organization = factory.SubFactory(OrganizationFactory) -class OrganizationRoleInvitationFactory(WarehouseFactory): +class OrganizationInvitationFactory(WarehouseFactory): class Meta: - model = OrganizationRoleInvitation + model = OrganizationInvitation invite_status = "pending" token = "test_token" diff --git a/tests/conftest.py b/tests/conftest.py index d2c0c97d0614..c0e7a6837d79 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -45,6 +45,7 @@ from warehouse.email.interfaces import IEmailSender from warehouse.macaroons import services as macaroon_services from warehouse.metrics import IMetricsService +from warehouse.organizations import services as organization_services from .common.db import Session @@ -285,6 +286,13 @@ def macaroon_service(db_session): return macaroon_services.DatabaseMacaroonService(db_session) +@pytest.fixture +def organization_service(db_session, remote_addr): + return organization_services.DatabaseOrganizationService( + db_session, remote_addr=remote_addr + ) + + @pytest.fixture def token_service(app_config): return account_services.TokenService(secret="secret", salt="salt", max_age=21600) diff --git a/tests/unit/organizations/test_services.py b/tests/unit/organizations/test_services.py new file mode 100644 index 000000000000..e4c18e9d868f --- /dev/null +++ b/tests/unit/organizations/test_services.py @@ -0,0 +1,121 @@ +# 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. + +import pretend + +from zope.interface.verify import verifyClass + +from warehouse.organizations import services +from warehouse.organizations.interfaces import IOrganizationService + +from ...common.db.organizations import OrganizationFactory, UserFactory + + +def test_database_organizations_factory(): + db = pretend.stub() + remote_addr = pretend.stub() + context = pretend.stub() + request = pretend.stub(db=db, remote_addr=remote_addr) + + service = services.database_organization_factory(context, request) + assert service.db is db + assert service.remote_addr is remote_addr + + +class TestDatabaseOrganizationService: + def test_verify_service(self): + assert verifyClass(IOrganizationService, services.DatabaseOrganizationService) + + def test_service_creation(self, remote_addr): + session = pretend.stub() + service = services.DatabaseOrganizationService(session, remote_addr=remote_addr) + + assert service.db is session + assert service.remote_addr is remote_addr + + def test_get_organization(self, organization_service): + organization = OrganizationFactory.create() + assert organization_service.get_organization(organization.id) == organization + + def test_get_organization_by_name(self, organization_service): + organization = OrganizationFactory.create() + assert ( + organization_service.get_organization_by_name(organization.name) + == organization + ) + + def test_find_organizationid(self, organization_service): + organization = OrganizationFactory.create() + assert ( + organization_service.find_organizationid(organization.name) + == organization.id + ) + + def test_find_organizationid_nonexistent_org(self, organization_service): + assert organization_service.find_organizationid("a_spoon_in_the_matrix") is None + + def test_add_organization(self, organization_service): + organization = OrganizationFactory.create() + new_org = organization_service.add_organization( + name=organization.name, + display_name=organization.display_name, + orgtype=organization.orgtype, + link_url=organization.link_url, + description=organization.description, + ) + organization_service.db.flush() + org_from_db = organization_service.get_organization(new_org.id) + + assert org_from_db.name == organization.name + assert org_from_db.display_name == organization.display_name + assert org_from_db.orgtype == organization.orgtype + assert org_from_db.link_url == organization.link_url + assert org_from_db.description == organization.description + assert not org_from_db.is_active + + def test_add_catalog_entry(self, organization_service): + organization = OrganizationFactory.create() + + catalog_entry = organization_service.add_catalog_entry( + organization.name, organization.id + ) + assert catalog_entry.name == organization.name + assert catalog_entry.organization_id == organization.id + + def test_add_organization_role(self, organization_service, user_service): + user = UserFactory.create() + organization = OrganizationFactory.create() + + added_role = organization_service.add_organization_role( + "some role", user.id, organization.id + ) + assert added_role.role_name == "some role" + assert added_role.user_id == user.id + assert added_role.organization_id == organization.id + + def test_approve_organization(self, organization_service): + organization = OrganizationFactory.create() + organization_service.approve_organization(organization.id) + + assert organization.is_active is True + assert organization.is_approved is True + assert organization.date_approved is not None + + def test_decline_organization(self, organization_service): + organization = OrganizationFactory.create() + organization_service.decline_organization(organization.id) + + assert organization.is_approved is False + assert organization.date_approved is not None + + # def test_record_event(self, organization_id, *, tag, additional=None): + # raise NotImplementedError From 0b0acc56d88c5abe1850997b26e25d1ae7469baf Mon Sep 17 00:00:00 2001 From: sterbo Date: Thu, 14 Apr 2022 12:39:21 -0700 Subject: [PATCH 18/46] Added OrganizationFactory class for model. --- warehouse/organizations/models.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/warehouse/organizations/models.py b/warehouse/organizations/models.py index a5ab8d31426c..6bc8e46a7ec5 100644 --- a/warehouse/organizations/models.py +++ b/warehouse/organizations/models.py @@ -28,6 +28,7 @@ sql, ) from sqlalchemy.dialects.postgresql import JSONB, UUID +from sqlalchemy.orm.exc import NoResultFound # from sqlalchemy.orm.exc import NoResultFound from sqlalchemy_utils.types.url import URLType @@ -100,6 +101,24 @@ class OrganizationType(enum.Enum): Expired = "Company" +class OrganizationFactory: + def __init__(self, request): + self.request = request + + def __getitem__(self, organization): + try: + return ( + self.request.db.query(Organization) + .filter( + Organization.normalized_name + == func.normalize_pep426_name(organization) + ) + .one() + ) + except NoResultFound: + raise KeyError from None + + # TODO: Determine if this should also utilize SitemapMixin and TwoFactorRequireable # class Project(SitemapMixin, TwoFactorRequireable, db.Model): class Organization(db.Model): From 8d9cb8f2421fd44bc74687b12fd62727f26b0691 Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Fri, 8 Apr 2022 11:51:22 -0700 Subject: [PATCH 19/46] Blank /manage/organizations/ page --- tests/unit/test_routes.py | 1 + warehouse/manage/views.py | 11 ++++++++++ warehouse/routes.py | 1 + warehouse/templates/manage/organizations.html | 21 +++++++++++++++++++ 4 files changed, 34 insertions(+) create mode 100644 warehouse/templates/manage/organizations.html diff --git a/tests/unit/test_routes.py b/tests/unit/test_routes.py index c4aac6c899a0..a30d65152599 100644 --- a/tests/unit/test_routes.py +++ b/tests/unit/test_routes.py @@ -237,6 +237,7 @@ def add_policy(name, filename): pretend.call( "manage.account.token", "/manage/account/token/", domain=warehouse ), + pretend.call("manage.organizations", "/manage/organizations/", domain=warehouse), pretend.call("manage.projects", "/manage/projects/", domain=warehouse), pretend.call( "manage.project.settings", diff --git a/warehouse/manage/views.py b/warehouse/manage/views.py index 458ade5b97b2..09059cde4a54 100644 --- a/warehouse/manage/views.py +++ b/warehouse/manage/views.py @@ -969,6 +969,17 @@ def delete_macaroon(self): return HTTPSeeOther(redirect_to) +@view_config( + route_name="manage.organizations", + renderer="manage/organizations.html", + uses_session=True, + permission="manage:user", + has_translations=True, +) +def manage_organizations(request): + return {} + + @view_config( route_name="manage.projects", renderer="manage/projects.html", diff --git a/warehouse/routes.py b/warehouse/routes.py index 31aa0eaba377..c3e28a28f529 100644 --- a/warehouse/routes.py +++ b/warehouse/routes.py @@ -221,6 +221,7 @@ def includeme(config): domain=warehouse, ) config.add_route("manage.account.token", "/manage/account/token/", domain=warehouse) + config.add_route("manage.organizations", "/manage/organizations/", domain=warehouse) config.add_route("manage.projects", "/manage/projects/", domain=warehouse) config.add_route( "manage.project.settings", diff --git a/warehouse/templates/manage/organizations.html b/warehouse/templates/manage/organizations.html new file mode 100644 index 000000000000..889741d2b00f --- /dev/null +++ b/warehouse/templates/manage/organizations.html @@ -0,0 +1,21 @@ +{# + # 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 "manage_base.html" %} + +{% set active_tab = 'organizations' %} + +{% block title %}{% trans %}Your organizations{% endtrans %}{% endblock %} + +{% block main %} +{% endblock %} From 99975f9cddffabf6a04c2a217c9860300e6467e9 Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Tue, 12 Apr 2022 01:05:42 -0700 Subject: [PATCH 20/46] Create organization form on /manage/organizations/ - Organization account name - Organization name - Organization URL - Organization description - Organization type --- warehouse/manage/forms.py | 95 +++++++++++++++ warehouse/manage/views.py | 39 +++++- warehouse/templates/manage/organizations.html | 114 ++++++++++++++++++ 3 files changed, 245 insertions(+), 3 deletions(-) diff --git a/warehouse/manage/forms.py b/warehouse/manage/forms.py index d9aa24933d67..e74538e13731 100644 --- a/warehouse/manage/forms.py +++ b/warehouse/manage/forms.py @@ -27,6 +27,8 @@ ) from warehouse.i18n import localize as _ +# /manage/account/ forms + class RoleNameMixin: @@ -303,3 +305,96 @@ class Toggle2FARequirementForm(forms.Form): __params__ = ["two_factor_requirement_sentinel"] two_factor_requirement_sentinel = wtforms.HiddenField() + + +# /manage/organizations/ forms + + +class NewOrganizationNameMixin: + + name = wtforms.StringField( + validators=[ + wtforms.validators.DataRequired( + message="Specify organization account name" + ), + wtforms.validators.Length( + max=50, + message=_( + "Choose an organization account name with 50 characters or less." + ), + ), + # the regexp below must match the CheckConstraint + # for the name field in organizations.model.Organization + wtforms.validators.Regexp( + r"^[a-zA-Z0-9][a-zA-Z0-9._-]*[a-zA-Z0-9]$", + message=_( + "The organization account name is invalid. " + "Organization account names " + "must be composed of letters, numbers, " + "dots, hyphens and underscores. And must " + "also start and finish with a letter or number. " + "Choose a different organization account name." + ), + ), + ] + ) + + def validate_name(self, field): + if self.organization_service.find_organizationid(field.data) is not None: + raise wtforms.validators.ValidationError( + _( + "This organization account name is already being " + "used by another account. Choose a different " + "organization account name." + ) + ) + + +class CreateOrganizationForm(forms.Form, NewOrganizationNameMixin): + + __params__ = ["name", "display_name", "link_url", "description", "orgtype"] + + display_name = wtforms.StringField( + validators=[ + wtforms.validators.DataRequired(message="Specify your organization name"), + wtforms.validators.Length( + max=100, + message=_( + "The organization name is too long. " + "Choose a organization name with 100 characters or less." + ), + ), + ] + ) + link_url = wtforms.URLField( + validators=[ + wtforms.validators.DataRequired(message="Specify your organization URL"), + wtforms.validators.Length( + max=400, + message=_( + "The organization URL is too long. " + "Choose a organization URL with 400 characters or less." + ), + ), + ] + ) + description = wtforms.TextAreaField( + validators=[ + wtforms.validators.DataRequired( + message="Specify your organization description" + ), + wtforms.validators.Length( + max=400, + message=_( + "The organization description is too long. " + "Choose a organization description with 400 characters or less." + ), + ), + ] + ) + orgtype = wtforms.SelectField( + choices=[("Company", "Company"), ("Community", "Community")], + validators=[ + wtforms.validators.DataRequired(message="Select organization type"), + ], + ) diff --git a/warehouse/manage/views.py b/warehouse/manage/views.py index 09059cde4a54..458858340f1d 100644 --- a/warehouse/manage/views.py +++ b/warehouse/manage/views.py @@ -70,6 +70,7 @@ ChangeRoleForm, ConfirmPasswordForm, CreateMacaroonForm, + CreateOrganizationForm, CreateRoleForm, DeleteMacaroonForm, DeleteTOTPForm, @@ -969,15 +970,47 @@ def delete_macaroon(self): return HTTPSeeOther(redirect_to) -@view_config( +@view_defaults( route_name="manage.organizations", renderer="manage/organizations.html", uses_session=True, + require_csrf=True, + require_methods=False, permission="manage:user", has_translations=True, ) -def manage_organizations(request): - return {} +class ManageOrganizationsViews: + def __init__(self, request): + self.request = request + # TODO: self.organization_service = request.find_service(IOrganizationService, context=None) + + @property + def default_response(self): + return { + "create_organization_form": CreateOrganizationForm( + # TODO: organization_service=self.organization_service, + ), + } + + @view_config(request_method="GET") + def manage_organizations(self): + return self.default_response + + @view_config(request_method="POST", request_param=CreateOrganizationForm.__params__) + def create_organization(self): + form = CreateOrganizationForm( + self.request.POST, + # TODO: organization_service=self.organization_service, + ) + + if form.validate(): + data = form.data + # TODO: self.organization_service.create_organization(**data) + self.request.session.flash( + "Request for new organization submitted", queue="success" + ) + + return self.default_response @view_config( diff --git a/warehouse/templates/manage/organizations.html b/warehouse/templates/manage/organizations.html index 889741d2b00f..1dacd0227bca 100644 --- a/warehouse/templates/manage/organizations.html +++ b/warehouse/templates/manage/organizations.html @@ -18,4 +18,118 @@ {% block title %}{% trans %}Your organizations{% endtrans %}{% endblock %} {% block main %} +

{{ title }}

+ + {{ form_error_anchor(create_organization_form) }} +
+

{% trans %}Create new organization{% endtrans %}

+ +
+ + {{ form_errors(create_organization_form) }} +
+ + {{ create_organization_form.name(placeholder=gettext("Select an organization account name"), autocapitalize="off", autocomplete="off", spellcheck="false", required="required", class_="form-group__field", **{"aria-describedby":"name-errors"}) }} +
+ {% if create_organization_form.name.errors %} + + {% endif %} +
+

+ {% trans %}This account name will be used in URLs on PyPI.{% endtrans %}
+ {% trans %}For example{% endtrans %}: psf +

+
+ +
+ + {{ create_organization_form.display_name(placeholder=gettext("Name of your business, product, or project"), autocomplete="organization", autocapitalize="off", spellcheck="false", class_="form-group__field", **{"aria-describedby":"display-name-errors"}) }} +
+ {% if create_organization_form.display_name.errors %} + + {% endif %} +
+

+ {% trans %}For example{% endtrans %}: Python Software Foundation +

+
+ +
+ +

+ {{ create_organization_form.link_url(placeholder=gettext("URL for your business, product, or project"), autocomplete="url", autocapitalize="off", spellcheck="false", class_="form-group__field", **{"aria-describedby":"link-url-errors"}) }} +

+ +

+ {% trans %}For example{% endtrans %}: https://www.python.org/psf/ +

+
+ +
+ + {{ create_organization_form.description(placeholder=gettext("Description of your business, product, or project"), autocomplete="off", autocapitalize="off", spellcheck="true", class_="form-group__field", **{"aria-describedby":"description-errors"}) }} +
+ {% if create_organization_form.description.errors %} + + {% endif %} +
+
+ +
+ +

+ {{ create_organization_form.orgtype(class_="form-group__field", **{"aria-describedby":"orgtype-errors"}) }} +

+
+ {{ field_errors(create_organization_form.orgtype) }} +
+

+ {% trans trimmed %} + Companies can create organization accounts as a paid service while community projects are granted complimentary access. + {% endtrans %} +

+
+ + +
+
{% endblock %} From c9fb07d2ca0ae59cb37f0f016a947a594648387f Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Wed, 13 Apr 2022 22:50:10 -0700 Subject: [PATCH 21/46] Register DatabaseOrganizationService Found the droids that we're looking for. --- tests/functional/manage/test_views.py | 5 +++++ tests/unit/manage/test_views.py | 3 +++ tests/unit/test_config.py | 1 + tests/unit/test_routes.py | 4 +++- warehouse/config.py | 3 +++ warehouse/manage/forms.py | 4 ++++ warehouse/manage/views.py | 9 ++++++--- warehouse/organizations/__init__.py | 8 +++++++- 8 files changed, 32 insertions(+), 5 deletions(-) diff --git a/tests/functional/manage/test_views.py b/tests/functional/manage/test_views.py index dec2e80d2ba1..11b456755dda 100644 --- a/tests/functional/manage/test_views.py +++ b/tests/functional/manage/test_views.py @@ -16,6 +16,7 @@ from warehouse.accounts.interfaces import IPasswordBreachedService, IUserService from warehouse.manage import views +from warehouse.organizations.interfaces import IOrganizationService from ...common.db.accounts import EmailFactory, UserFactory @@ -23,10 +24,14 @@ class TestManageAccount: def test_save_account(self, pyramid_services, user_service, db_request): breach_service = pretend.stub() + organization_service = pretend.stub() pyramid_services.register_service(user_service, IUserService, None) pyramid_services.register_service( breach_service, IPasswordBreachedService, None ) + pyramid_services.register_service( + organization_service, IOrganizationService, None + ) user = UserFactory.create(name="old name") EmailFactory.create(primary=True, verified=True, public=True, user=user) db_request.user = user diff --git a/tests/unit/manage/test_views.py b/tests/unit/manage/test_views.py index 92b91be50d03..ca73170e12b8 100644 --- a/tests/unit/manage/test_views.py +++ b/tests/unit/manage/test_views.py @@ -44,6 +44,7 @@ from warehouse.manage import views from warehouse.metrics.interfaces import IMetricsService from warehouse.oidc.interfaces import TooManyOIDCRegistrations +from warehouse.organizations.interfaces import IOrganizationService from warehouse.packaging.models import ( File, JournalEntry, @@ -78,12 +79,14 @@ class TestManageAccount: def test_default_response(self, monkeypatch, public_email, expected_public_email): breach_service = pretend.stub() user_service = pretend.stub() + organization_service = pretend.stub() name = pretend.stub() user_id = pretend.stub() request = pretend.stub( find_service=lambda iface, **kw: { IPasswordBreachedService: breach_service, IUserService: user_service, + IOrganizationService: organization_service, }[iface], user=pretend.stub(name=name, id=user_id, public_email=public_email), ) diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index e194c2becfe6..7406cd0eeffc 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -352,6 +352,7 @@ def __init__(self): pretend.call(".oidc"), pretend.call(".malware"), pretend.call(".manage"), + pretend.call(".organizations"), pretend.call(".packaging"), pretend.call(".redirects"), pretend.call(".routes"), diff --git a/tests/unit/test_routes.py b/tests/unit/test_routes.py index a30d65152599..dcccf822e5d8 100644 --- a/tests/unit/test_routes.py +++ b/tests/unit/test_routes.py @@ -237,7 +237,9 @@ def add_policy(name, filename): pretend.call( "manage.account.token", "/manage/account/token/", domain=warehouse ), - pretend.call("manage.organizations", "/manage/organizations/", domain=warehouse), + pretend.call( + "manage.organizations", "/manage/organizations/", domain=warehouse + ), pretend.call("manage.projects", "/manage/projects/", domain=warehouse), pretend.call( "manage.project.settings", diff --git a/warehouse/config.py b/warehouse/config.py index 16d09981286e..8fd0866cece7 100644 --- a/warehouse/config.py +++ b/warehouse/config.py @@ -518,6 +518,9 @@ def configure(settings=None): # Register logged-in views config.include(".manage") + # Register our organization support. + config.include(".organizations") + # Allow the packaging app to register any services it has. config.include(".packaging") diff --git a/warehouse/manage/forms.py b/warehouse/manage/forms.py index e74538e13731..5f2ceceb0303 100644 --- a/warehouse/manage/forms.py +++ b/warehouse/manage/forms.py @@ -354,6 +354,10 @@ class CreateOrganizationForm(forms.Form, NewOrganizationNameMixin): __params__ = ["name", "display_name", "link_url", "description", "orgtype"] + def __init__(self, *args, organization_service, **kwargs): + super().__init__(*args, **kwargs) + self.organization_service = organization_service + display_name = wtforms.StringField( validators=[ wtforms.validators.DataRequired(message="Specify your organization name"), diff --git a/warehouse/manage/views.py b/warehouse/manage/views.py index 458858340f1d..8caefd18557f 100644 --- a/warehouse/manage/views.py +++ b/warehouse/manage/views.py @@ -84,6 +84,7 @@ from warehouse.oidc.forms import DeleteProviderForm, GitHubProviderForm from warehouse.oidc.interfaces import TooManyOIDCRegistrations from warehouse.oidc.models import GitHubProvider, OIDCProvider +from warehouse.organizations.interfaces import IOrganizationService from warehouse.packaging.models import ( File, JournalEntry, @@ -982,13 +983,15 @@ def delete_macaroon(self): class ManageOrganizationsViews: def __init__(self, request): self.request = request - # TODO: self.organization_service = request.find_service(IOrganizationService, context=None) + self.organization_service = request.find_service( + IOrganizationService, context=None + ) @property def default_response(self): return { "create_organization_form": CreateOrganizationForm( - # TODO: organization_service=self.organization_service, + organization_service=self.organization_service, ), } @@ -1000,7 +1003,7 @@ def manage_organizations(self): def create_organization(self): form = CreateOrganizationForm( self.request.POST, - # TODO: organization_service=self.organization_service, + organization_service=self.organization_service, ) if form.validate(): diff --git a/warehouse/organizations/__init__.py b/warehouse/organizations/__init__.py index 7223de0e0995..d89e3a731c2f 100644 --- a/warehouse/organizations/__init__.py +++ b/warehouse/organizations/__init__.py @@ -10,4 +10,10 @@ # See the License for the specific language governing permissions and # limitations under the License. -# TODO: Nothing to see here yet. These aren't the droids you're looking for. +from warehouse.organizations.interfaces import IOrganizationService +from warehouse.organizations.services import database_organization_factory + + +def includeme(config): + # Register our organization service + config.register_service_factory(database_organization_factory, IOrganizationService) From e33b8ca5ad3c7d2d00dccc2876e9a467ac038fd1 Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Wed, 13 Apr 2022 23:39:37 -0700 Subject: [PATCH 22/46] POST /manage/organizations/ database updates --- warehouse/manage/views.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/warehouse/manage/views.py b/warehouse/manage/views.py index 8caefd18557f..0ae51e6d2cb9 100644 --- a/warehouse/manage/views.py +++ b/warehouse/manage/views.py @@ -1008,7 +1008,13 @@ def create_organization(self): if form.validate(): data = form.data - # TODO: self.organization_service.create_organization(**data) + organization = self.organization_service.add_organization(**data) + self.organization_service.add_catalog_entry( + organization.name, organization.id + ) + self.organization_service.add_organization_role( + "Owner", self.request.user.id, organization.id + ) self.request.session.flash( "Request for new organization submitted", queue="success" ) From b2b0ec2cc0cfc788014a108b4bb50bccafa894a5 Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Thu, 14 Apr 2022 00:49:17 -0700 Subject: [PATCH 23/46] Add .get_admins method to user service --- warehouse/accounts/interfaces.py | 6 ++++++ warehouse/accounts/services.py | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/warehouse/accounts/interfaces.py b/warehouse/accounts/interfaces.py index adf68da13042..151766d30eda 100644 --- a/warehouse/accounts/interfaces.py +++ b/warehouse/accounts/interfaces.py @@ -78,6 +78,12 @@ def get_user_by_email(email): if there is no user with that email. """ + def get_admins(): + """ + Return a list of user objects corresponding with admin users, or [] + if there is no admin users. + """ + def find_userid(username): """ Find the unique user identifier for the given username or None if there diff --git a/warehouse/accounts/services.py b/warehouse/accounts/services.py index a7a14950b7f3..616aad903e1e 100644 --- a/warehouse/accounts/services.py +++ b/warehouse/accounts/services.py @@ -101,6 +101,10 @@ def get_user_by_email(self, email): user_id = self.find_userid_by_email(email) return None if user_id is None else self.get_user(user_id) + @functools.lru_cache() + def get_admins(self): + return self.db.query(User).filter(User.is_superuser.is_(True)).all() + @functools.lru_cache() def find_userid(self, username): try: From d4664d3b2ceebad55619f774ea8f805e3010345a Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Thu, 14 Apr 2022 00:49:36 -0700 Subject: [PATCH 24/46] POST /manage/organizations/ email notifications --- warehouse/manage/views.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/warehouse/manage/views.py b/warehouse/manage/views.py index 0ae51e6d2cb9..e935d8c38728 100644 --- a/warehouse/manage/views.py +++ b/warehouse/manage/views.py @@ -43,9 +43,11 @@ from warehouse.admin.flags import AdminFlagValue from warehouse.email import ( send_account_deletion_email, + send_admin_new_organization_requested_email, send_collaborator_removed_email, send_collaborator_role_changed_email, send_email_verification_email, + send_new_organization_requested_email, send_oidc_provider_added_email, send_oidc_provider_removed_email, send_password_change_email, @@ -983,6 +985,7 @@ def delete_macaroon(self): class ManageOrganizationsViews: def __init__(self, request): self.request = request + self.user_service = request.find_service(IUserService, context=None) self.organization_service = request.find_service( IOrganizationService, context=None ) @@ -1015,6 +1018,15 @@ def create_organization(self): self.organization_service.add_organization_role( "Owner", self.request.user.id, organization.id ) + send_admin_new_organization_requested_email( + self.request, + self.user_service.get_admins(), + organization_name=organization.name, + initiator_username=self.request.user.username, + ) + send_new_organization_requested_email( + self.request, self.request.user, organization_name=organization.name + ) self.request.session.flash( "Request for new organization submitted", queue="success" ) From 5d9bcc5985c3ee7131abeadc87920bcba691620f Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Thu, 14 Apr 2022 01:46:31 -0700 Subject: [PATCH 25/46] Blank /admin/organizations/approve/ page This is a placeholder so we can reference `admin.organization.approve` as a route in the admin-new-organization-requested email. --- tests/unit/admin/test_routes.py | 5 ++++ warehouse/admin/routes.py | 5 ++++ .../admin/organizations/approve.html | 21 ++++++++++++++++ warehouse/admin/views/organizations.py | 25 +++++++++++++++++++ 4 files changed, 56 insertions(+) create mode 100644 warehouse/admin/templates/admin/organizations/approve.html create mode 100644 warehouse/admin/views/organizations.py diff --git a/tests/unit/admin/test_routes.py b/tests/unit/admin/test_routes.py index da7d5d0487bd..084e3f48c7e2 100644 --- a/tests/unit/admin/test_routes.py +++ b/tests/unit/admin/test_routes.py @@ -26,6 +26,11 @@ def test_includeme(): assert config.add_route.calls == [ pretend.call("admin.dashboard", "/admin/", domain=warehouse), + pretend.call( + "admin.organization.approve", + "/admin/organizations/approve/", + domain=warehouse, + ), pretend.call("admin.user.list", "/admin/users/", domain=warehouse), pretend.call("admin.user.detail", "/admin/users/{user_id}/", domain=warehouse), pretend.call( diff --git a/warehouse/admin/routes.py b/warehouse/admin/routes.py index 8248f76290b7..ed7e8461db12 100644 --- a/warehouse/admin/routes.py +++ b/warehouse/admin/routes.py @@ -20,6 +20,11 @@ def includeme(config): # General Admin pages config.add_route("admin.dashboard", "/admin/", domain=warehouse) + # Organization related Admin pages + config.add_route( + "admin.organization.approve", "/admin/organizations/approve/", domain=warehouse + ) + # User related Admin pages config.add_route("admin.user.list", "/admin/users/", domain=warehouse) config.add_route("admin.user.detail", "/admin/users/{user_id}/", domain=warehouse) diff --git a/warehouse/admin/templates/admin/organizations/approve.html b/warehouse/admin/templates/admin/organizations/approve.html new file mode 100644 index 000000000000..86f871f7c427 --- /dev/null +++ b/warehouse/admin/templates/admin/organizations/approve.html @@ -0,0 +1,21 @@ +{# + # 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 "confirm-action.html" %} + +{% block title %} + {% trans %}Approve New Organization{% endtrans %} +{% endblock %} + +{% block main %} +{% endblock %} diff --git a/warehouse/admin/views/organizations.py b/warehouse/admin/views/organizations.py new file mode 100644 index 000000000000..c874ab3b8da4 --- /dev/null +++ b/warehouse/admin/views/organizations.py @@ -0,0 +1,25 @@ +# 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. + +from pyramid.view import view_config + + +@view_config( + route_name="admin.organization.approve", + renderer="admin/organizations/approve.html", + permission="admin", + require_methods=False, + uses_session=True, + has_translations=True, +) +def approve(request): + return {} From 88663d78d4959ae187e3649325b52954fb13a20b Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Thu, 14 Apr 2022 11:45:41 -0700 Subject: [PATCH 26/46] Test GET /manage/organizations/ - `ManageOrganizationsViews.default_response` - `ManageOrganizationsViews.manage_organizations()` --- tests/unit/manage/test_views.py | 35 +++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/unit/manage/test_views.py b/tests/unit/manage/test_views.py index ca73170e12b8..1c50274ee6bc 100644 --- a/tests/unit/manage/test_views.py +++ b/tests/unit/manage/test_views.py @@ -2306,6 +2306,41 @@ def test_delete_macaroon_records_events_for_each_project(self, monkeypatch): ] +class TestManageOrganizations: + def test_default_response(self, monkeypatch): + create_organization_obj = pretend.stub() + create_organization_cls = pretend.call_recorder( + lambda *a, **kw: create_organization_obj + ) + monkeypatch.setattr(views, "CreateOrganizationForm", create_organization_cls) + + request = pretend.stub( + user=pretend.stub(id=pretend.stub(), username=pretend.stub()), + find_service=lambda interface, **kw: { + IOrganizationService: pretend.stub(), + IUserService: pretend.stub(), + }[interface], + ) + + view = views.ManageOrganizationsViews(request) + + assert view.default_response == { + "create_organization_form": create_organization_obj, + } + + def test_manage_organizations(self, monkeypatch): + request = pretend.stub(find_service=lambda *a, **kw: pretend.stub()) + + default_response = {"default": "response"} + monkeypatch.setattr( + views.ManageOrganizationsViews, "default_response", default_response + ) + view = views.ManageOrganizationsViews(request) + result = view.manage_organizations() + + assert result == default_response + + class TestManageProjects: def test_manage_projects(self, db_request): older_release = ReleaseFactory(created=datetime.datetime(2015, 1, 1)) From f29a97de4821aa882efe1c72dd2f59945153f025 Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Thu, 14 Apr 2022 13:12:39 -0700 Subject: [PATCH 27/46] Translations for /manage/organizations/ make translations --- warehouse/locale/messages.pot | 135 ++++++++++++++++++++++++++++++---- 1 file changed, 122 insertions(+), 13 deletions(-) diff --git a/warehouse/locale/messages.pot b/warehouse/locale/messages.pot index 853ee9b6cff7..8b4ad685b226 100644 --- a/warehouse/locale/messages.pot +++ b/warehouse/locale/messages.pot @@ -65,7 +65,7 @@ msgid "" "different email." msgstr "" -#: warehouse/accounts/forms.py:258 warehouse/manage/forms.py:73 +#: warehouse/accounts/forms.py:258 warehouse/manage/forms.py:75 msgid "The name is too long. Choose a name with 100 characters or less." msgstr "" @@ -116,7 +116,7 @@ msgstr "" msgid "Successful WebAuthn assertion" msgstr "" -#: warehouse/accounts/views.py:441 warehouse/manage/views.py:814 +#: warehouse/accounts/views.py:441 warehouse/manage/views.py:818 msgid "Recovery code accepted. The supplied code cannot be used again." msgstr "" @@ -225,51 +225,91 @@ msgstr "" msgid "Banner Preview" msgstr "" -#: warehouse/manage/views.py:245 +#: warehouse/admin/templates/admin/organizations/approve.html:17 +msgid "Approve New Organization" +msgstr "" + +#: warehouse/manage/forms.py:322 +msgid "Choose an organization account name with 50 characters or less." +msgstr "" + +#: warehouse/manage/forms.py:330 +msgid "" +"The organization account name is invalid. Organization account names must" +" be composed of letters, numbers, dots, hyphens and underscores. And must" +" also start and finish with a letter or number. Choose a different " +"organization account name." +msgstr "" + +#: warehouse/manage/forms.py:345 +msgid "" +"This organization account name is already being used by another account. " +"Choose a different organization account name." +msgstr "" + +#: warehouse/manage/forms.py:366 +msgid "" +"The organization name is too long. Choose a organization name with 100 " +"characters or less." +msgstr "" + +#: warehouse/manage/forms.py:378 +msgid "" +"The organization URL is too long. Choose a organization URL with 400 " +"characters or less." +msgstr "" + +#: warehouse/manage/forms.py:392 +msgid "" +"The organization description is too long. Choose a organization " +"description with 400 characters or less." +msgstr "" + +#: warehouse/manage/views.py:249 msgid "Email ${email_address} added - check your email for a verification link" msgstr "" -#: warehouse/manage/views.py:762 +#: warehouse/manage/views.py:766 msgid "Recovery codes already generated" msgstr "" -#: warehouse/manage/views.py:763 +#: warehouse/manage/views.py:767 msgid "Generating new recovery codes will invalidate your existing codes." msgstr "" -#: warehouse/manage/views.py:1191 +#: warehouse/manage/views.py:1256 msgid "" "There have been too many attempted OpenID Connect registrations. Try " "again later." msgstr "" -#: warehouse/manage/views.py:1872 +#: warehouse/manage/views.py:1937 msgid "User '${username}' already has ${role_name} role for project" msgstr "" -#: warehouse/manage/views.py:1883 +#: warehouse/manage/views.py:1948 msgid "" "User '${username}' does not have a verified primary email address and " "cannot be added as a ${role_name} for project" msgstr "" -#: warehouse/manage/views.py:1896 +#: warehouse/manage/views.py:1961 msgid "User '${username}' already has an active invite. Please try again later." msgstr "" -#: warehouse/manage/views.py:1954 +#: warehouse/manage/views.py:2019 msgid "Invitation sent to '${username}'" msgstr "" -#: warehouse/manage/views.py:2001 +#: warehouse/manage/views.py:2066 msgid "Could not find role invitation." msgstr "" -#: warehouse/manage/views.py:2012 +#: warehouse/manage/views.py:2077 msgid "Invitation already expired." msgstr "" -#: warehouse/manage/views.py:2036 +#: warehouse/manage/views.py:2101 msgid "Invitation revoked from '${username}'." msgstr "" @@ -896,6 +936,11 @@ msgstr "" #: warehouse/templates/manage/account/recovery_codes-burn.html:70 #: warehouse/templates/manage/account/totp-provision.html:69 #: warehouse/templates/manage/account/webauthn-provision.html:44 +#: warehouse/templates/manage/organizations.html:34 +#: warehouse/templates/manage/organizations.html:57 +#: warehouse/templates/manage/organizations.html:79 +#: warehouse/templates/manage/organizations.html:97 +#: warehouse/templates/manage/organizations.html:116 #: warehouse/templates/manage/publishing.html:85 #: warehouse/templates/manage/publishing.html:97 #: warehouse/templates/manage/publishing.html:109 @@ -2929,6 +2974,70 @@ msgstr "" msgid "Back to projects" msgstr "" +#: warehouse/templates/manage/organizations.html:18 +msgid "Your organizations" +msgstr "" + +#: warehouse/templates/manage/organizations.html:25 +msgid "Create new organization" +msgstr "" + +#: warehouse/templates/manage/organizations.html:32 +msgid "Organization account name" +msgstr "" + +#: warehouse/templates/manage/organizations.html:37 +msgid "Select an organization account name" +msgstr "" + +#: warehouse/templates/manage/organizations.html:48 +msgid "This account name will be used in URLs on PyPI." +msgstr "" + +#: warehouse/templates/manage/organizations.html:49 +#: warehouse/templates/manage/organizations.html:71 +#: warehouse/templates/manage/organizations.html:89 +msgid "For example" +msgstr "" + +#: warehouse/templates/manage/organizations.html:55 +msgid "Organization name" +msgstr "" + +#: warehouse/templates/manage/organizations.html:60 +msgid "Name of your business, product, or project" +msgstr "" + +#: warehouse/templates/manage/organizations.html:77 +msgid "️Organization URL" +msgstr "" + +#: warehouse/templates/manage/organizations.html:83 +msgid "URL for your business, product, or project" +msgstr "" + +#: warehouse/templates/manage/organizations.html:95 +msgid "Organization description" +msgstr "" + +#: warehouse/templates/manage/organizations.html:100 +msgid "Description of your business, product, or project" +msgstr "" + +#: warehouse/templates/manage/organizations.html:114 +msgid "️Organization type" +msgstr "" + +#: warehouse/templates/manage/organizations.html:126 +msgid "" +"Companies can create organization accounts as a paid service while " +"community projects are granted complimentary access." +msgstr "" + +#: warehouse/templates/manage/organizations.html:132 +msgid "Create" +msgstr "" + #: warehouse/templates/manage/projects.html:22 #, python-format msgid "Pending invitations (%(project_count)s)" From e15e2800bf51494f412b8b641e0ddcf91a902dd7 Mon Sep 17 00:00:00 2001 From: sterbo Date: Thu, 14 Apr 2022 15:53:38 -0700 Subject: [PATCH 28/46] Fixed OrganizationType enum. --- warehouse/organizations/models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/warehouse/organizations/models.py b/warehouse/organizations/models.py index 6bc8e46a7ec5..dd87023f044d 100644 --- a/warehouse/organizations/models.py +++ b/warehouse/organizations/models.py @@ -97,8 +97,8 @@ class OrganizationProject(db.Model): class OrganizationType(enum.Enum): - Pending = "Community" - Expired = "Company" + Community = "Community" + Company = "Company" class OrganizationFactory: From 67d3a2837e0fb640c29ebb3f08b9908c91ce6caa Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Thu, 14 Apr 2022 12:43:53 -0700 Subject: [PATCH 29/46] Test POST /manage/organizations/ - `ManageOrganizationsViews.create_organization()` --- tests/unit/manage/test_views.py | 174 ++++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+) diff --git a/tests/unit/manage/test_views.py b/tests/unit/manage/test_views.py index 1c50274ee6bc..e2862f4aa600 100644 --- a/tests/unit/manage/test_views.py +++ b/tests/unit/manage/test_views.py @@ -2340,6 +2340,180 @@ def test_manage_organizations(self, monkeypatch): assert result == default_response + def test_create_organization(self, monkeypatch): + admins = [] + user_service = pretend.stub( + get_admins=pretend.call_recorder(lambda *a, **kw: admins) + ) + + organization = pretend.stub( + id=pretend.stub(), + name="psf", + display_name="Python Software Foundation", + orgtype="Community", + link_url="https://www.python.org/psf/", + description=( + "To promote, protect, and advance the Python programming " + "language, and to support and facilitate the growth of a " + "diverse and international community of Python programmers" + ), + is_active=False, + is_approved=None, + ) + catalog_entry = pretend.stub() + role = pretend.stub() + organization_service = pretend.stub( + add_organization=pretend.call_recorder(lambda *a, **kw: organization), + add_catalog_entry=pretend.call_recorder(lambda *a, **kw: catalog_entry), + add_organization_role=pretend.call_recorder(lambda *a, **kw: role), + ) + + request = pretend.stub( + POST={ + "name": organization.name, + "display_name": organization.display_name, + "orgtype": organization.orgtype, + "link_url": organization.link_url, + "description": organization.description, + }, + domain=pretend.stub(), + user=pretend.stub( + id=pretend.stub(), + username=pretend.stub(), + has_primary_verified_email=True, + ), + session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)), + find_service=lambda interface, **kw: { + IUserService: user_service, + IOrganizationService: organization_service, + }[interface], + remote_addr="0.0.0.0", + ) + + create_organization_obj = pretend.stub(validate=lambda: True, data=request.POST) + create_organization_cls = pretend.call_recorder( + lambda *a, **kw: create_organization_obj + ) + monkeypatch.setattr(views, "CreateOrganizationForm", create_organization_cls) + + send_email = pretend.call_recorder(lambda *a, **kw: None) + monkeypatch.setattr( + views, "send_admin_new_organization_requested_email", send_email + ) + monkeypatch.setattr(views, "send_new_organization_requested_email", send_email) + + default_response = {"default": "response"} + monkeypatch.setattr( + views.ManageOrganizationsViews, "default_response", default_response + ) + + view = views.ManageOrganizationsViews(request) + result = view.create_organization() + + assert user_service.get_admins.calls == [pretend.call()] + assert organization_service.add_organization.calls == [ + pretend.call( + name=organization.name, + display_name=organization.display_name, + orgtype=organization.orgtype, + link_url=organization.link_url, + description=organization.description, + ) + ] + assert organization_service.add_catalog_entry.calls == [ + pretend.call( + organization.name, + organization.id, + ) + ] + assert organization_service.add_organization_role.calls == [ + pretend.call( + "Owner", + request.user.id, + organization.id, + ) + ] + assert send_email.calls == [ + pretend.call( + request, + admins, + organization_name=organization.name, + initiator_username=request.user.username, + ), + pretend.call( + request, + request.user, + organization_name=organization.name, + ), + ] + assert result == default_response + + def test_create_organization_validation_fails(self, monkeypatch): + admins = [] + user_service = pretend.stub( + get_admins=pretend.call_recorder(lambda *a, **kw: admins) + ) + + organization = pretend.stub() + catalog_entry = pretend.stub() + role = pretend.stub() + organization_service = pretend.stub( + add_organization=pretend.call_recorder(lambda *a, **kw: organization), + add_catalog_entry=pretend.call_recorder(lambda *a, **kw: catalog_entry), + add_organization_role=pretend.call_recorder(lambda *a, **kw: role), + ) + + request = pretend.stub( + POST={ + "name": None, + "display_name": None, + "orgtype": None, + "link_url": None, + "description": None, + }, + domain=pretend.stub(), + user=pretend.stub( + id=pretend.stub(), + username=pretend.stub(), + has_primary_verified_email=True, + ), + session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)), + find_service=lambda interface, **kw: { + IUserService: user_service, + IOrganizationService: organization_service, + }[interface], + remote_addr="0.0.0.0", + ) + + create_organization_obj = pretend.stub( + validate=lambda: False, data=request.POST + ) + create_organization_cls = pretend.call_recorder( + lambda *a, **kw: create_organization_obj + ) + monkeypatch.setattr(views, "CreateOrganizationForm", create_organization_cls) + + send_email = pretend.call_recorder(lambda *a, **kw: None) + monkeypatch.setattr( + views, "send_admin_new_organization_requested_email", send_email + ) + monkeypatch.setattr(views, "send_new_organization_requested_email", send_email) + + default_response = {"default": "response"} + monkeypatch.setattr( + views.ManageOrganizationsViews, "default_response", default_response + ) + + view = views.ManageOrganizationsViews(request) + result = view.create_organization() + + assert user_service.get_admins.calls == [] + assert organization_service.add_organization.calls == [] + assert organization_service.add_catalog_entry.calls == [] + assert organization_service.add_organization_role.calls == [] + assert send_email.calls == [] + assert result == default_response + class TestManageProjects: def test_manage_projects(self, db_request): From acbfac6695c5b932a7041ef53f67ea45a7ccc20f Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Fri, 15 Apr 2022 00:36:56 -0700 Subject: [PATCH 30/46] Test CreateOrganizationForm --- tests/unit/manage/test_forms.py | 38 +++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/unit/manage/test_forms.py b/tests/unit/manage/test_forms.py index bde69e26c7e6..2ab8ed35db95 100644 --- a/tests/unit/manage/test_forms.py +++ b/tests/unit/manage/test_forms.py @@ -501,6 +501,44 @@ def test_validate_macaroon_id(self): assert form.validate() +class TestCreateOrganizationForm: + def test_creation(self): + organization_service = pretend.stub() + form = forms.CreateOrganizationForm( + organization_service=organization_service, + ) + + assert form.organization_service is organization_service + + def test_validate_name_with_no_organization(self): + organization_service = pretend.stub( + find_organizationid=pretend.call_recorder(lambda name: None) + ) + form = forms.CreateOrganizationForm(organization_service=organization_service) + field = pretend.stub(data="my_organization_name") + forms._ = lambda string: string + + form.validate_name(field) + + assert organization_service.find_organizationid.calls == [ + pretend.call("my_organization_name") + ] + + def test_validate_name_with_organization(self): + organization_service = pretend.stub( + find_organizationid=pretend.call_recorder(lambda name: 1) + ) + form = forms.CreateOrganizationForm(organization_service=organization_service) + field = pretend.stub(data="my_organization_name") + + with pytest.raises(wtforms.validators.ValidationError): + form.validate_name(field) + + assert organization_service.find_organizationid.calls == [ + pretend.call("my_organization_name") + ] + + class TestSaveAccountForm: def test_public_email_verified(self): email = pretend.stub(verified=True, public=False, email="foo@example.com") From 71d2ce0202441c19769106b0c07ccf18ddfe606b Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Fri, 15 Apr 2022 00:45:25 -0700 Subject: [PATCH 31/46] Placeholder to test /admin/organizations/approve/ Provides code coverage for the blank page added in 2c70616c. --- tests/unit/admin/views/test_organizations.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 tests/unit/admin/views/test_organizations.py diff --git a/tests/unit/admin/views/test_organizations.py b/tests/unit/admin/views/test_organizations.py new file mode 100644 index 000000000000..ae3a2639bf37 --- /dev/null +++ b/tests/unit/admin/views/test_organizations.py @@ -0,0 +1,20 @@ +# 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. + +import pretend + +from warehouse.admin.views import organizations as views + + +class TestOrganizations: + def test_approve(self): + assert views.approve(pretend.stub()) == {} From b4aa6ece8cdc2015d97844692f1a2f5e08a15314 Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Fri, 15 Apr 2022 00:58:17 -0700 Subject: [PATCH 32/46] Test `DatabaseUserService.get_admins()` --- tests/unit/accounts/test_services.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/unit/accounts/test_services.py b/tests/unit/accounts/test_services.py index 42ccebb7e4d0..ee68dc42958b 100644 --- a/tests/unit/accounts/test_services.py +++ b/tests/unit/accounts/test_services.py @@ -408,6 +408,14 @@ def test_get_user_by_email_failure(self, user_service): assert found_user is None + def test_get_admins(self, user_service): + admin = UserFactory.create(is_superuser=True) + user = UserFactory.create(is_superuser=False) + admins = user_service.get_admins() + + assert admin in admins + assert user not in admins + def test_disable_password(self, user_service): user = UserFactory.create() From 232f6386180657a453be1f38b5fbef3f4753ca4e Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Fri, 15 Apr 2022 10:17:46 -0700 Subject: [PATCH 33/46] NFC: Add comments about intentionally blank page --- warehouse/admin/templates/admin/organizations/approve.html | 5 +++++ warehouse/admin/views/organizations.py | 3 +++ warehouse/locale/messages.pot | 2 +- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/warehouse/admin/templates/admin/organizations/approve.html b/warehouse/admin/templates/admin/organizations/approve.html index 86f871f7c427..ce105ad83fdf 100644 --- a/warehouse/admin/templates/admin/organizations/approve.html +++ b/warehouse/admin/templates/admin/organizations/approve.html @@ -13,9 +13,14 @@ -#} {% extends "confirm-action.html" %} +{# This is a placeholder so we can reference `admin.organization.approve` + # as a route in the admin-new-organization-requested email. +-#} + {% block title %} {% trans %}Approve New Organization{% endtrans %} {% endblock %} {% block main %} + {% endblock %} diff --git a/warehouse/admin/views/organizations.py b/warehouse/admin/views/organizations.py index c874ab3b8da4..2750a5d6e4f7 100644 --- a/warehouse/admin/views/organizations.py +++ b/warehouse/admin/views/organizations.py @@ -13,6 +13,8 @@ from pyramid.view import view_config +# This is a placeholder so we can reference `admin.organization.approve` +# as a route in the admin-new-organization-requested email. @view_config( route_name="admin.organization.approve", renderer="admin/organizations/approve.html", @@ -22,4 +24,5 @@ has_translations=True, ) def approve(request): + # TODO return {} diff --git a/warehouse/locale/messages.pot b/warehouse/locale/messages.pot index 8b4ad685b226..0a70ed2c831a 100644 --- a/warehouse/locale/messages.pot +++ b/warehouse/locale/messages.pot @@ -225,7 +225,7 @@ msgstr "" msgid "Banner Preview" msgstr "" -#: warehouse/admin/templates/admin/organizations/approve.html:17 +#: warehouse/admin/templates/admin/organizations/approve.html:21 msgid "Approve New Organization" msgstr "" From 3b373b452345aa4a289bb334bfa6f73378899516 Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Fri, 15 Apr 2022 11:57:53 -0700 Subject: [PATCH 34/46] Record events for POST /manage/organizations/ Co-Authored-By: sterbo --- warehouse/locale/messages.pot | 16 ++++++------- warehouse/manage/views.py | 37 +++++++++++++++++++++++++++++++ warehouse/organizations/models.py | 10 ++++----- 3 files changed, 50 insertions(+), 13 deletions(-) diff --git a/warehouse/locale/messages.pot b/warehouse/locale/messages.pot index 0a70ed2c831a..72f7c3d3b8ac 100644 --- a/warehouse/locale/messages.pot +++ b/warehouse/locale/messages.pot @@ -277,39 +277,39 @@ msgstr "" msgid "Generating new recovery codes will invalidate your existing codes." msgstr "" -#: warehouse/manage/views.py:1256 +#: warehouse/manage/views.py:1293 msgid "" "There have been too many attempted OpenID Connect registrations. Try " "again later." msgstr "" -#: warehouse/manage/views.py:1937 +#: warehouse/manage/views.py:1974 msgid "User '${username}' already has ${role_name} role for project" msgstr "" -#: warehouse/manage/views.py:1948 +#: warehouse/manage/views.py:1985 msgid "" "User '${username}' does not have a verified primary email address and " "cannot be added as a ${role_name} for project" msgstr "" -#: warehouse/manage/views.py:1961 +#: warehouse/manage/views.py:1998 msgid "User '${username}' already has an active invite. Please try again later." msgstr "" -#: warehouse/manage/views.py:2019 +#: warehouse/manage/views.py:2056 msgid "Invitation sent to '${username}'" msgstr "" -#: warehouse/manage/views.py:2066 +#: warehouse/manage/views.py:2103 msgid "Could not find role invitation." msgstr "" -#: warehouse/manage/views.py:2077 +#: warehouse/manage/views.py:2114 msgid "Invitation already expired." msgstr "" -#: warehouse/manage/views.py:2101 +#: warehouse/manage/views.py:2138 msgid "Invitation revoked from '${username}'." msgstr "" diff --git a/warehouse/manage/views.py b/warehouse/manage/views.py index e935d8c38728..9a27f46ace11 100644 --- a/warehouse/manage/views.py +++ b/warehouse/manage/views.py @@ -1012,12 +1012,49 @@ def create_organization(self): if form.validate(): data = form.data organization = self.organization_service.add_organization(**data) + self.organization_service.record_event( + organization.id, + tag="organization:create", + additional={"created_by": self.request.user.username}, + ) self.organization_service.add_catalog_entry( organization.name, organization.id ) + self.organization_service.record_event( + organization.id, + tag="organization:catalog_entry:add", + additional={"submitted_by": self.request.user.username}, + ) self.organization_service.add_organization_role( "Owner", self.request.user.id, organization.id ) + self.organization_service.record_event( + organization.id, + tag="organization:organization_role:invite", + additional={ + "submitted_by": self.request.user.username, + "role_name": "Owner", + "target_user": self.request.user.username, + }, + ) + self.organization_service.record_event( + organization.id, + tag="organization:organization_role:accepted", + additional={ + "submitted_by": self.request.user.username, + "role_name": "Owner", + "target_user": self.request.user.username, + }, + ) + self.user_service.record_event( + self.request.user.id, + tag="account:organization_role:accepted", + additional={ + "submitted_by": self.request.user.username, + "organization_name": organization.name, + "role_name": "Owner", + }, + ) send_admin_new_organization_requested_email( self.request, self.user_service.get_admins(), diff --git a/warehouse/organizations/models.py b/warehouse/organizations/models.py index dd87023f044d..65eeb9191106 100644 --- a/warehouse/organizations/models.py +++ b/warehouse/organizations/models.py @@ -165,7 +165,7 @@ class Organization(db.Model): events = orm.relationship( "OrganizationEvent", - backref="organizations", + backref="organization", cascade="all, delete-orphan", lazy=True, ) @@ -175,13 +175,13 @@ class Organization(db.Model): def record_event(self, *, tag, ip_address, additional): session = orm.object_session(self) - organization = OrganizationEvent( - user=self, tag=tag, ip_address=ip_address, additional=additional + event = OrganizationEvent( + organization=self, tag=tag, ip_address=ip_address, additional=additional ) - session.add(organization) + session.add(event) session.flush() - return organization + return event class OrganizationEvent(db.Model): From d9de8b3c08d05fd38d12de9d6330d5576d7f11f5 Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Fri, 15 Apr 2022 13:22:12 -0700 Subject: [PATCH 35/46] Test record events for POST /manage/organizations/ --- tests/unit/manage/test_views.py | 50 +++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/tests/unit/manage/test_views.py b/tests/unit/manage/test_views.py index e2862f4aa600..1d2230af6a2c 100644 --- a/tests/unit/manage/test_views.py +++ b/tests/unit/manage/test_views.py @@ -2343,7 +2343,8 @@ def test_manage_organizations(self, monkeypatch): def test_create_organization(self, monkeypatch): admins = [] user_service = pretend.stub( - get_admins=pretend.call_recorder(lambda *a, **kw: admins) + get_admins=pretend.call_recorder(lambda *a, **kw: admins), + record_event=pretend.call_recorder(lambda *a, **kw: None), ) organization = pretend.stub( @@ -2366,6 +2367,7 @@ def test_create_organization(self, monkeypatch): add_organization=pretend.call_recorder(lambda *a, **kw: organization), add_catalog_entry=pretend.call_recorder(lambda *a, **kw: catalog_entry), add_organization_role=pretend.call_recorder(lambda *a, **kw: role), + record_event=pretend.call_recorder(lambda *a, **kw: None), ) request = pretend.stub( @@ -2433,6 +2435,47 @@ def test_create_organization(self, monkeypatch): organization.id, ) ] + assert organization_service.record_event.calls == [ + pretend.call( + organization.id, + tag="organization:create", + additional={"created_by": request.user.username}, + ), + pretend.call( + organization.id, + tag="organization:catalog_entry:add", + additional={"submitted_by": request.user.username}, + ), + pretend.call( + organization.id, + tag="organization:organization_role:invite", + additional={ + "submitted_by": request.user.username, + "role_name": "Owner", + "target_user": request.user.username, + }, + ), + pretend.call( + organization.id, + tag="organization:organization_role:accepted", + additional={ + "submitted_by": request.user.username, + "role_name": "Owner", + "target_user": request.user.username, + }, + ), + ] + assert user_service.record_event.calls == [ + pretend.call( + request.user.id, + tag="account:organization_role:accepted", + additional={ + "submitted_by": request.user.username, + "organization_name": organization.name, + "role_name": "Owner", + }, + ), + ] assert send_email.calls == [ pretend.call( request, @@ -2451,7 +2494,8 @@ def test_create_organization(self, monkeypatch): def test_create_organization_validation_fails(self, monkeypatch): admins = [] user_service = pretend.stub( - get_admins=pretend.call_recorder(lambda *a, **kw: admins) + get_admins=pretend.call_recorder(lambda *a, **kw: admins), + record_event=pretend.call_recorder(lambda *a, **kw: None), ) organization = pretend.stub() @@ -2461,6 +2505,7 @@ def test_create_organization_validation_fails(self, monkeypatch): add_organization=pretend.call_recorder(lambda *a, **kw: organization), add_catalog_entry=pretend.call_recorder(lambda *a, **kw: catalog_entry), add_organization_role=pretend.call_recorder(lambda *a, **kw: role), + record_event=pretend.call_recorder(lambda *a, **kw: None), ) request = pretend.stub( @@ -2511,6 +2556,7 @@ def test_create_organization_validation_fails(self, monkeypatch): assert organization_service.add_organization.calls == [] assert organization_service.add_catalog_entry.calls == [] assert organization_service.add_organization_role.calls == [] + assert organization_service.record_event.calls == [] assert send_email.calls == [] assert result == default_response From 1f40800dd5ac0d98eb8b5dbb9af55e178a6bc317 Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Fri, 15 Apr 2022 16:34:52 -0700 Subject: [PATCH 36/46] Functional test for POST /manage/organizations/ --- tests/functional/manage/test_views.py | 49 +++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/functional/manage/test_views.py b/tests/functional/manage/test_views.py index 11b456755dda..fb0a18a804cb 100644 --- a/tests/functional/manage/test_views.py +++ b/tests/functional/manage/test_views.py @@ -17,6 +17,7 @@ from warehouse.accounts.interfaces import IPasswordBreachedService, IUserService from warehouse.manage import views from warehouse.organizations.interfaces import IOrganizationService +from warehouse.organizations.models import OrganizationType from ...common.db.accounts import EmailFactory, UserFactory @@ -43,3 +44,51 @@ def test_save_account(self, pyramid_services, user_service, db_request): user = user_service.get_user(user.id) assert user.name == "new name" assert user.public_email is None + + +class TestManageOrganizations: + def test_create_organization( + self, + pyramid_services, + user_service, + organization_service, + db_request, + monkeypatch, + ): + pyramid_services.register_service(user_service, IUserService, None) + pyramid_services.register_service( + organization_service, IOrganizationService, None + ) + user = UserFactory.create(name="old name") + EmailFactory.create(primary=True, verified=True, public=True, user=user) + db_request.user = user + db_request.method = "POST" + db_request.path = "/manage/organizations/" + db_request.POST = MultiDict( + { + "name": "psf", + "display_name": "Python Software Foundation", + "orgtype": "Community", + "link_url": "https://www.python.org/psf/", + "description": ( + "To promote, protect, and advance the Python programming " + "language, and to support and facilitate the growth of a " + "diverse and international community of Python programmers" + ), + } + ) + send_email = pretend.call_recorder(lambda *a, **kw: None) + monkeypatch.setattr( + views, "send_admin_new_organization_requested_email", send_email + ) + monkeypatch.setattr(views, "send_new_organization_requested_email", send_email) + views.ManageOrganizationsViews(db_request).create_organization() + + organization = organization_service.get_organization_by_name( + db_request.POST["name"] + ) + assert organization.name == db_request.POST["name"] + assert organization.display_name == db_request.POST["display_name"] + assert organization.orgtype == OrganizationType[db_request.POST["orgtype"]] + assert organization.link_url == db_request.POST["link_url"] + assert organization.description == db_request.POST["description"] From faa73f4e8f6a7698d6dc40c408b90604ec09057f Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Fri, 15 Apr 2022 20:39:28 -0700 Subject: [PATCH 37/46] Comment out `OrganizationFactory` for future use Co-Authored-By: sterbo --- warehouse/organizations/models.py | 34 +++++++++++++++---------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/warehouse/organizations/models.py b/warehouse/organizations/models.py index 65eeb9191106..85352a988ed2 100644 --- a/warehouse/organizations/models.py +++ b/warehouse/organizations/models.py @@ -28,7 +28,6 @@ sql, ) from sqlalchemy.dialects.postgresql import JSONB, UUID -from sqlalchemy.orm.exc import NoResultFound # from sqlalchemy.orm.exc import NoResultFound from sqlalchemy_utils.types.url import URLType @@ -101,22 +100,23 @@ class OrganizationType(enum.Enum): Company = "Company" -class OrganizationFactory: - def __init__(self, request): - self.request = request - - def __getitem__(self, organization): - try: - return ( - self.request.db.query(Organization) - .filter( - Organization.normalized_name - == func.normalize_pep426_name(organization) - ) - .one() - ) - except NoResultFound: - raise KeyError from None +# TODO: For future use +# class OrganizationFactory: +# def __init__(self, request): +# self.request = request +# +# def __getitem__(self, organization): +# try: +# return ( +# self.request.db.query(Organization) +# .filter( +# Organization.normalized_name +# == func.normalize_pep426_name(organization) +# ) +# .one() +# ) +# except NoResultFound: +# raise KeyError from None # TODO: Determine if this should also utilize SitemapMixin and TwoFactorRequireable From 1163b5ee808a3645d65ca8d345162488e2b90bbb Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Sat, 16 Apr 2022 14:54:26 -0700 Subject: [PATCH 38/46] NFC: Fix camel case for class names --- tests/unit/email/test_init.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/email/test_init.py b/tests/unit/email/test_init.py index 2f30796e50db..3fda4908bdbd 100644 --- a/tests/unit/email/test_init.py +++ b/tests/unit/email/test_init.py @@ -413,7 +413,7 @@ def retry(exc): assert task.retry.calls == [pretend.call(exc=exc)] -class TestSendAdminNewOrganizationrequestedEmail: +class TestSendAdminNewOrganizationRequestedEmail: def test_send_admin_new_organization_requested_email( self, pyramid_request, pyramid_config, monkeypatch ): @@ -1355,7 +1355,7 @@ def test_primary_email_change_email_unverified( assert send_email.delay.calls == [] -class TestSendNewOrganizationrequestedEmail: +class TestSendNewOrganizationRequestedEmail: def test_send_new_organization_requested_email( self, pyramid_request, pyramid_config, monkeypatch ): From eecf5de6e6c42d5b091d6966b5416ab92e15e116 Mon Sep 17 00:00:00 2001 From: sterbo Date: Mon, 18 Apr 2022 11:17:31 -0700 Subject: [PATCH 39/46] Converted OrganizationRoleType to SQLAlchemy Enum --- tests/unit/organizations/test_services.py | 5 +++-- warehouse/organizations/models.py | 13 ++++++++++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/tests/unit/organizations/test_services.py b/tests/unit/organizations/test_services.py index e4c18e9d868f..3e0d86adb06b 100644 --- a/tests/unit/organizations/test_services.py +++ b/tests/unit/organizations/test_services.py @@ -16,6 +16,7 @@ from warehouse.organizations import services from warehouse.organizations.interfaces import IOrganizationService +from warehouse.organizations.models import OrganizationRoleType from ...common.db.organizations import OrganizationFactory, UserFactory @@ -96,9 +97,9 @@ def test_add_organization_role(self, organization_service, user_service): organization = OrganizationFactory.create() added_role = organization_service.add_organization_role( - "some role", user.id, organization.id + OrganizationRoleType.Owner.value, user.id, organization.id ) - assert added_role.role_name == "some role" + assert added_role.role_name == OrganizationRoleType.Owner.value assert added_role.user_id == user.id assert added_role.organization_id == organization.id diff --git a/warehouse/organizations/models.py b/warehouse/organizations/models.py index 85352a988ed2..ea9c398d7668 100644 --- a/warehouse/organizations/models.py +++ b/warehouse/organizations/models.py @@ -37,6 +37,14 @@ from warehouse.utils.attrs import make_repr +class OrganizationRoleType(enum.Enum): + + BillingManager = "Billing Manager" + Manager = "Manager" + Member = "Member" + Owner = "Owner" + + class OrganizationRole(db.Model): __tablename__ = "organization_roles" @@ -52,7 +60,10 @@ class OrganizationRole(db.Model): __repr__ = make_repr("role_name") - role_name = Column(Text, nullable=False) + role_name = Column( + Enum(OrganizationRoleType, values_callable=lambda x: [e.value for e in x]), + nullable=False, + ) user_id = Column( ForeignKey("users.id", onupdate="CASCADE", ondelete="CASCADE"), nullable=False ) From 5034ac01f521a8336b266eed161cd21614b34647 Mon Sep 17 00:00:00 2001 From: sterbo Date: Mon, 18 Apr 2022 11:17:48 -0700 Subject: [PATCH 40/46] Add disable-organizations global admin flag --- ...99509d92_add_disable_organizations_flag.py | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 warehouse/migrations/versions/9f0f99509d92_add_disable_organizations_flag.py diff --git a/warehouse/migrations/versions/9f0f99509d92_add_disable_organizations_flag.py b/warehouse/migrations/versions/9f0f99509d92_add_disable_organizations_flag.py new file mode 100644 index 000000000000..98bb0f79da5c --- /dev/null +++ b/warehouse/migrations/versions/9f0f99509d92_add_disable_organizations_flag.py @@ -0,0 +1,55 @@ +# 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. +""" +Add disable-organizations AdminFlag + +Revision ID: 9f0f99509d92 +Revises: 4a985d158c3c +Create Date: 2022-04-18 02:04:40.318843 +""" + +from alembic import op + +revision = "9f0f99509d92" +down_revision = "4a985d158c3c" + +# Note: It is VERY important to ensure that a migration does not lock for a +# long period of time and to ensure that each individual migration does +# not break compatibility with the *previous* version of the code base. +# This is because the migrations will be ran automatically as part of the +# deployment process, but while the previous version of the code is still +# up and running. Thus backwards incompatible changes must be broken up +# over multiple migrations inside of multiple pull requests in order to +# phase them in over multiple deploys. + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.execute( + """ + INSERT INTO admin_flags(id, description, enabled, notify) + VALUES ( + 'disable-organizations', + 'Disallow ALL functionality for Organizations', + TRUE, + FALSE + ) + """ + ) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.execute("DELETE FROM admin_flags WHERE id = 'disable-organizations'") + + # ### end Alembic commands ### From 161ab199ddc7ef19c067c2de6539aa7574262015 Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Mon, 18 Apr 2022 01:02:31 -0700 Subject: [PATCH 41/46] Add `AdminFlagValue.DISABLE_ORGANIZATIONS` --- tests/functional/manage/test_views.py | 15 ++++++++-- tests/unit/manage/test_views.py | 42 ++++++++++++++++++++++++++- warehouse/admin/flags.py | 1 + warehouse/locale/messages.pot | 16 +++++----- warehouse/manage/views.py | 6 ++++ 5 files changed, 69 insertions(+), 11 deletions(-) diff --git a/tests/functional/manage/test_views.py b/tests/functional/manage/test_views.py index fb0a18a804cb..4c5da434d14e 100644 --- a/tests/functional/manage/test_views.py +++ b/tests/functional/manage/test_views.py @@ -15,6 +15,7 @@ from webob.multidict import MultiDict from warehouse.accounts.interfaces import IPasswordBreachedService, IUserService +from warehouse.admin.flags import AdminFlagValue from warehouse.manage import views from warehouse.organizations.interfaces import IOrganizationService from warehouse.organizations.models import OrganizationType @@ -39,9 +40,10 @@ def test_save_account(self, pyramid_services, user_service, db_request): db_request.method = "POST" db_request.path = "/manage/accounts/" db_request.POST = MultiDict({"name": "new name", "public_email": ""}) - views.ManageAccountViews(db_request).save_account() + views.ManageAccountViews(db_request).save_account() user = user_service.get_user(user.id) + assert user.name == "new name" assert user.public_email is None @@ -77,16 +79,25 @@ def test_create_organization( ), } ) + monkeypatch.setattr( + db_request, + "flags", + pretend.stub(enabled=pretend.call_recorder(lambda *a: False)), + ) send_email = pretend.call_recorder(lambda *a, **kw: None) monkeypatch.setattr( views, "send_admin_new_organization_requested_email", send_email ) monkeypatch.setattr(views, "send_new_organization_requested_email", send_email) - views.ManageOrganizationsViews(db_request).create_organization() + views.ManageOrganizationsViews(db_request).create_organization() organization = organization_service.get_organization_by_name( db_request.POST["name"] ) + + assert db_request.flags.enabled.calls == [ + pretend.call(AdminFlagValue.DISABLE_ORGANIZATIONS), + ] assert organization.name == db_request.POST["name"] assert organization.display_name == db_request.POST["display_name"] assert organization.orgtype == OrganizationType[db_request.POST["orgtype"]] diff --git a/tests/unit/manage/test_views.py b/tests/unit/manage/test_views.py index 1d2230af6a2c..c040bdc5d001 100644 --- a/tests/unit/manage/test_views.py +++ b/tests/unit/manage/test_views.py @@ -2329,7 +2329,10 @@ def test_default_response(self, monkeypatch): } def test_manage_organizations(self, monkeypatch): - request = pretend.stub(find_service=lambda *a, **kw: pretend.stub()) + request = pretend.stub( + find_service=lambda *a, **kw: pretend.stub(), + flags=pretend.stub(enabled=pretend.call_recorder(lambda *a: False)), + ) default_response = {"default": "response"} monkeypatch.setattr( @@ -2338,8 +2341,24 @@ def test_manage_organizations(self, monkeypatch): view = views.ManageOrganizationsViews(request) result = view.manage_organizations() + assert request.flags.enabled.calls == [ + pretend.call(AdminFlagValue.DISABLE_ORGANIZATIONS), + ] assert result == default_response + def test_manage_organizations_disallow_organizations(self, monkeypatch): + request = pretend.stub( + find_service=lambda *a, **kw: pretend.stub(), + flags=pretend.stub(enabled=pretend.call_recorder(lambda *a: True)), + ) + + view = views.ManageOrganizationsViews(request) + with pytest.raises(HTTPNotFound): + view.manage_organizations() + assert request.flags.enabled.calls == [ + pretend.call(AdminFlagValue.DISABLE_ORGANIZATIONS), + ] + def test_create_organization(self, monkeypatch): admins = [] user_service = pretend.stub( @@ -2389,6 +2408,7 @@ def test_create_organization(self, monkeypatch): IUserService: user_service, IOrganizationService: organization_service, }[interface], + flags=pretend.stub(enabled=pretend.call_recorder(lambda *a: False)), remote_addr="0.0.0.0", ) @@ -2412,6 +2432,9 @@ def test_create_organization(self, monkeypatch): view = views.ManageOrganizationsViews(request) result = view.create_organization() + assert request.flags.enabled.calls == [ + pretend.call(AdminFlagValue.DISABLE_ORGANIZATIONS), + ] assert user_service.get_admins.calls == [pretend.call()] assert organization_service.add_organization.calls == [ pretend.call( @@ -2527,6 +2550,7 @@ def test_create_organization_validation_fails(self, monkeypatch): IUserService: user_service, IOrganizationService: organization_service, }[interface], + flags=pretend.stub(enabled=pretend.call_recorder(lambda *a: False)), remote_addr="0.0.0.0", ) @@ -2552,6 +2576,9 @@ def test_create_organization_validation_fails(self, monkeypatch): view = views.ManageOrganizationsViews(request) result = view.create_organization() + assert request.flags.enabled.calls == [ + pretend.call(AdminFlagValue.DISABLE_ORGANIZATIONS), + ] assert user_service.get_admins.calls == [] assert organization_service.add_organization.calls == [] assert organization_service.add_catalog_entry.calls == [] @@ -2560,6 +2587,19 @@ def test_create_organization_validation_fails(self, monkeypatch): assert send_email.calls == [] assert result == default_response + def test_create_organizations_disallow_organizations(self, monkeypatch): + request = pretend.stub( + find_service=lambda *a, **kw: pretend.stub(), + flags=pretend.stub(enabled=pretend.call_recorder(lambda *a: True)), + ) + + view = views.ManageOrganizationsViews(request) + with pytest.raises(HTTPNotFound): + view.create_organization() + assert request.flags.enabled.calls == [ + pretend.call(AdminFlagValue.DISABLE_ORGANIZATIONS), + ] + class TestManageProjects: def test_manage_projects(self, db_request): diff --git a/warehouse/admin/flags.py b/warehouse/admin/flags.py index 03082747c611..408b1f9d8e3a 100644 --- a/warehouse/admin/flags.py +++ b/warehouse/admin/flags.py @@ -18,6 +18,7 @@ class AdminFlagValue(enum.Enum): + DISABLE_ORGANIZATIONS = "disable-organizations" DISALLOW_DELETION = "disallow-deletion" DISALLOW_NEW_PROJECT_REGISTRATION = "disallow-new-project-registration" DISALLOW_NEW_UPLOAD = "disallow-new-upload" diff --git a/warehouse/locale/messages.pot b/warehouse/locale/messages.pot index 72f7c3d3b8ac..a8c62bf97fea 100644 --- a/warehouse/locale/messages.pot +++ b/warehouse/locale/messages.pot @@ -277,39 +277,39 @@ msgstr "" msgid "Generating new recovery codes will invalidate your existing codes." msgstr "" -#: warehouse/manage/views.py:1293 +#: warehouse/manage/views.py:1299 msgid "" "There have been too many attempted OpenID Connect registrations. Try " "again later." msgstr "" -#: warehouse/manage/views.py:1974 +#: warehouse/manage/views.py:1980 msgid "User '${username}' already has ${role_name} role for project" msgstr "" -#: warehouse/manage/views.py:1985 +#: warehouse/manage/views.py:1991 msgid "" "User '${username}' does not have a verified primary email address and " "cannot be added as a ${role_name} for project" msgstr "" -#: warehouse/manage/views.py:1998 +#: warehouse/manage/views.py:2004 msgid "User '${username}' already has an active invite. Please try again later." msgstr "" -#: warehouse/manage/views.py:2056 +#: warehouse/manage/views.py:2062 msgid "Invitation sent to '${username}'" msgstr "" -#: warehouse/manage/views.py:2103 +#: warehouse/manage/views.py:2109 msgid "Could not find role invitation." msgstr "" -#: warehouse/manage/views.py:2114 +#: warehouse/manage/views.py:2120 msgid "Invitation already expired." msgstr "" -#: warehouse/manage/views.py:2138 +#: warehouse/manage/views.py:2144 msgid "Invitation revoked from '${username}'." msgstr "" diff --git a/warehouse/manage/views.py b/warehouse/manage/views.py index 9a27f46ace11..70b9e46b2cb0 100644 --- a/warehouse/manage/views.py +++ b/warehouse/manage/views.py @@ -1000,10 +1000,16 @@ def default_response(self): @view_config(request_method="GET") def manage_organizations(self): + if self.request.flags.enabled(AdminFlagValue.DISABLE_ORGANIZATIONS): + raise HTTPNotFound + return self.default_response @view_config(request_method="POST", request_param=CreateOrganizationForm.__params__) def create_organization(self): + if self.request.flags.enabled(AdminFlagValue.DISABLE_ORGANIZATIONS): + raise HTTPNotFound + form = CreateOrganizationForm( self.request.POST, organization_service=self.organization_service, From 0cb26b03e56935bab169050ea5addaa24576d384 Mon Sep 17 00:00:00 2001 From: sterbo Date: Tue, 19 Apr 2022 11:02:47 -0700 Subject: [PATCH 42/46] Modified org name catalog to store normalized name --- tests/common/db/organizations.py | 1 + tests/unit/organizations/test_services.py | 4 ++-- .../614a7fcb40ed_create_organization_models.py | 10 +++++----- warehouse/organizations/models.py | 10 +++++----- warehouse/organizations/services.py | 10 ++++++---- 5 files changed, 19 insertions(+), 16 deletions(-) diff --git a/tests/common/db/organizations.py b/tests/common/db/organizations.py index 51373eaa6c60..c677257f3909 100644 --- a/tests/common/db/organizations.py +++ b/tests/common/db/organizations.py @@ -37,6 +37,7 @@ class Meta: id = factory.Faker("uuid4", cast_to=None) name = factory.Faker("word") + normalized_name = factory.Faker("word") display_name = factory.Faker("word") orgtype = "Community" link_url = factory.Faker("uri") diff --git a/tests/unit/organizations/test_services.py b/tests/unit/organizations/test_services.py index 3e0d86adb06b..e2809a64ff67 100644 --- a/tests/unit/organizations/test_services.py +++ b/tests/unit/organizations/test_services.py @@ -87,9 +87,9 @@ def test_add_catalog_entry(self, organization_service): organization = OrganizationFactory.create() catalog_entry = organization_service.add_catalog_entry( - organization.name, organization.id + organization.normalized_name, organization.id ) - assert catalog_entry.name == organization.name + assert catalog_entry.normalized_name == organization.normalized_name assert catalog_entry.organization_id == organization.id def test_add_organization_role(self, organization_service, user_service): diff --git a/warehouse/migrations/versions/614a7fcb40ed_create_organization_models.py b/warehouse/migrations/versions/614a7fcb40ed_create_organization_models.py index 2ee6db3cf4d7..3ad793d8f692 100644 --- a/warehouse/migrations/versions/614a7fcb40ed_create_organization_models.py +++ b/warehouse/migrations/versions/614a7fcb40ed_create_organization_models.py @@ -122,7 +122,7 @@ def upgrade(): server_default=sa.text("gen_random_uuid()"), nullable=False, ), - sa.Column("name", sa.Text(), nullable=False), + sa.Column("normalized_name", sa.Text(), nullable=False), sa.Column("organization_id", postgresql.UUID(as_uuid=True), nullable=False), sa.ForeignKeyConstraint( ["organization_id"], @@ -132,15 +132,15 @@ def upgrade(): ), sa.PrimaryKeyConstraint("id"), sa.UniqueConstraint( - "name", + "normalized_name", "organization_id", - name="_organization_name_catalog_name_organization_uc", + name="_organization_name_catalog_normalized_name_organization_uc", ), ) op.create_index( - "organization_name_catalog_name_idx", + "organization_name_catalog_normalized_name_idx", "organization_name_catalog", - ["name"], + ["normalized_name"], unique=False, ) op.create_index( diff --git a/warehouse/organizations/models.py b/warehouse/organizations/models.py index ea9c398d7668..0c893a5451a8 100644 --- a/warehouse/organizations/models.py +++ b/warehouse/organizations/models.py @@ -214,18 +214,18 @@ class OrganizationNameCatalog(db.Model): __tablename__ = "organization_name_catalog" __table_args__ = ( - Index("organization_name_catalog_name_idx", "name"), + Index("organization_name_catalog_normalized_name_idx", "normalized_name"), Index("organization_name_catalog_organization_id_idx", "organization_id"), UniqueConstraint( - "name", + "normalized_name", "organization_id", - name="_organization_name_catalog_name_organization_uc", + name="_organization_name_catalog_normalized_name_organization_uc", ), ) - __repr__ = make_repr("name", "organization_id") + __repr__ = make_repr("normalized_name", "organization_id") - name = Column(Text, nullable=False) + normalized_name = Column(Text, nullable=False) organization_id = Column( ForeignKey("organizations.id", onupdate="CASCADE", ondelete="CASCADE"), nullable=False, diff --git a/warehouse/organizations/services.py b/warehouse/organizations/services.py index 43fbaf42c342..0393e56fc3cc 100644 --- a/warehouse/organizations/services.py +++ b/warehouse/organizations/services.py @@ -48,12 +48,14 @@ def get_organization_by_name(self, name): def find_organizationid(self, name): """ - Find the unique organization identifier for the given name or None if there - is no organization with the given name. + Find the unique organization identifier for the given normalized name or None + if there is no organization with the given name. """ try: organization = ( - self.db.query(Organization.id).filter(Organization.name == name).one() + self.db.query(Organization.id) + .filter(Organization.normalized_name == name) + .one() ) except NoResultFound: return @@ -83,7 +85,7 @@ def add_catalog_entry(self, name, organization_id): """ organization = self.get_organization(organization_id) catalog_entry = OrganizationNameCatalog( - name=name, organization_id=organization.id + normalized_name=name, organization_id=organization.id ) self.db.add(catalog_entry) From c73674c54a81eff0c916a9f491b68a54981dd48a Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Wed, 20 Apr 2022 05:51:19 -0700 Subject: [PATCH 43/46] {OrganizationEvents => Organization.Events} - Remove `OrganizationEvents` class - Add `HasEvents` mixing to `Organization` - Update references {OrganizationEvents => Organization.Events} - Update database migration {organization_id => source_id} --- tests/common/db/organizations.py | 5 +-- ...85d158c3c_add_organization_events_table.py | 10 ++--- warehouse/organizations/interfaces.py | 2 +- warehouse/organizations/models.py | 39 ++----------------- warehouse/organizations/services.py | 2 +- 5 files changed, 12 insertions(+), 46 deletions(-) diff --git a/tests/common/db/organizations.py b/tests/common/db/organizations.py index c677257f3909..f0d8b78e67d6 100644 --- a/tests/common/db/organizations.py +++ b/tests/common/db/organizations.py @@ -17,7 +17,6 @@ from warehouse.organizations.models import ( Organization, - OrganizationEvent, OrganizationInvitation, OrganizationNameCatalog, OrganizationProject, @@ -56,9 +55,9 @@ class Meta: class OrganizationEventFactory(WarehouseFactory): class Meta: - model = OrganizationEvent + model = Organization.Event - organization = factory.SubFactory(OrganizationFactory) + source = factory.SubFactory(OrganizationFactory) class OrganizationNameCatalogFactory(WarehouseFactory): diff --git a/warehouse/migrations/versions/4a985d158c3c_add_organization_events_table.py b/warehouse/migrations/versions/4a985d158c3c_add_organization_events_table.py index 02b22b512a09..76c39bd2153e 100644 --- a/warehouse/migrations/versions/4a985d158c3c_add_organization_events_table.py +++ b/warehouse/migrations/versions/4a985d158c3c_add_organization_events_table.py @@ -45,7 +45,7 @@ def upgrade(): server_default=sa.text("gen_random_uuid()"), nullable=False, ), - sa.Column("organization_id", postgresql.UUID(as_uuid=True), nullable=False), + sa.Column("source_id", postgresql.UUID(as_uuid=True), nullable=False), sa.Column("tag", sa.String(), nullable=False), sa.Column( "time", sa.DateTime(), server_default=sa.text("now()"), nullable=False @@ -53,7 +53,7 @@ def upgrade(): sa.Column("ip_address", sa.String(), nullable=False), sa.Column("additional", postgresql.JSONB(astext_type=sa.Text()), nullable=True), sa.ForeignKeyConstraint( - ["organization_id"], + ["source_id"], ["organizations.id"], initially="DEFERRED", deferrable=True, @@ -61,9 +61,9 @@ def upgrade(): sa.PrimaryKeyConstraint("id"), ) op.create_index( - op.f("ix_organization_events_organization_id"), + op.f("ix_organization_events_source_id"), "organization_events", - ["organization_id"], + ["source_id"], unique=False, ) # ### end Alembic commands ### @@ -72,7 +72,7 @@ def upgrade(): def downgrade(): # ### commands auto generated by Alembic - please adjust! ### op.drop_index( - op.f("ix_organization_events_organization_id"), table_name="organization_events" + op.f("ix_organization_events_source_id"), table_name="organization_events" ) op.drop_table("organization_events") # ### end Alembic commands ### diff --git a/warehouse/organizations/interfaces.py b/warehouse/organizations/interfaces.py index 258b1e0e9004..2e72d7bcadc0 100644 --- a/warehouse/organizations/interfaces.py +++ b/warehouse/organizations/interfaces.py @@ -60,7 +60,7 @@ def decline_organization(organization_id): def record_event(organization_id, *, tag, additional=None): """ - Creates a new OrganizationEvent for the given organization with the given + Creates a new Organization.Event for the given organization with the given tag, IP address, and additional metadata. Returns the event. diff --git a/warehouse/organizations/models.py b/warehouse/organizations/models.py index 0c893a5451a8..e094488d47d3 100644 --- a/warehouse/organizations/models.py +++ b/warehouse/organizations/models.py @@ -20,20 +20,19 @@ Enum, ForeignKey, Index, - String, Text, UniqueConstraint, func, orm, sql, ) -from sqlalchemy.dialects.postgresql import JSONB, UUID # from sqlalchemy.orm.exc import NoResultFound from sqlalchemy_utils.types.url import URLType from warehouse import db from warehouse.accounts.models import User +from warehouse.events.models import HasEvents from warehouse.utils.attrs import make_repr @@ -131,8 +130,8 @@ class OrganizationType(enum.Enum): # TODO: Determine if this should also utilize SitemapMixin and TwoFactorRequireable -# class Project(SitemapMixin, TwoFactorRequireable, db.Model): -class Organization(db.Model): +# class Organization(SitemapMixin, TwoFactorRequireable, HasEvents, db.Model): +class Organization(HasEvents, db.Model): __tablename__ = "organizations" __table_args__ = ( CheckConstraint( @@ -174,41 +173,9 @@ class Organization(db.Model): "Project", secondary=OrganizationProject.__table__, backref="organizations" # type: ignore # noqa ) - events = orm.relationship( - "OrganizationEvent", - backref="organization", - cascade="all, delete-orphan", - lazy=True, - ) - # TODO: # def __acl__(self): - def record_event(self, *, tag, ip_address, additional): - session = orm.object_session(self) - event = OrganizationEvent( - organization=self, tag=tag, ip_address=ip_address, additional=additional - ) - session.add(event) - session.flush() - - return event - - -class OrganizationEvent(db.Model): - __tablename__ = "organization_events" - - organization_id = Column( - UUID(as_uuid=True), - ForeignKey("organizations.id", deferrable=True, initially="DEFERRED"), - nullable=False, - index=True, - ) - tag = Column(String, nullable=False) - time = Column(DateTime, nullable=False, server_default=sql.func.now()) - ip_address = Column(String, nullable=False) - additional = Column(JSONB, nullable=True) - class OrganizationNameCatalog(db.Model): diff --git a/warehouse/organizations/services.py b/warehouse/organizations/services.py index 0393e56fc3cc..394fbcb86981 100644 --- a/warehouse/organizations/services.py +++ b/warehouse/organizations/services.py @@ -132,7 +132,7 @@ def decline_organization(self, organization_id): def record_event(self, organization_id, *, tag, additional=None): """ - Creates a new OrganizationEvent for the given organization with the given + Creates a new Organization.Event for the given organization with the given tag, IP address, and additional metadata. Returns the event. From db0ec20ab3e83d78934b0a6ae3e58b65006eec92 Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Wed, 20 Apr 2022 07:46:15 -0700 Subject: [PATCH 44/46] Store id instead of username in new events `Organization.Event` with tag: - organization:create - organization:catalog_entry:add - organization:organization_role:invite - organization:organization_role:accepted `User.Event` with tag: - account:organization_role:accepted --- tests/unit/manage/test_views.py | 14 +++++++------- warehouse/manage/views.py | 14 +++++++------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/unit/manage/test_views.py b/tests/unit/manage/test_views.py index 3a3c61cd4705..636a5ba9e9b6 100644 --- a/tests/unit/manage/test_views.py +++ b/tests/unit/manage/test_views.py @@ -2461,29 +2461,29 @@ def test_create_organization(self, monkeypatch): pretend.call( organization.id, tag="organization:create", - additional={"created_by": request.user.username}, + additional={"created_by_id": str(request.user.id)}, ), pretend.call( organization.id, tag="organization:catalog_entry:add", - additional={"submitted_by": request.user.username}, + additional={"submitted_by_id": str(request.user.id)}, ), pretend.call( organization.id, tag="organization:organization_role:invite", additional={ - "submitted_by": request.user.username, + "submitted_by_id": str(request.user.id), "role_name": "Owner", - "target_user": request.user.username, + "target_user_id": str(request.user.id), }, ), pretend.call( organization.id, tag="organization:organization_role:accepted", additional={ - "submitted_by": request.user.username, + "submitted_by_id": str(request.user.id), "role_name": "Owner", - "target_user": request.user.username, + "target_user_id": str(request.user.id), }, ), ] @@ -2492,7 +2492,7 @@ def test_create_organization(self, monkeypatch): request.user.id, tag="account:organization_role:accepted", additional={ - "submitted_by": request.user.username, + "submitted_by_id": str(request.user.id), "organization_name": organization.name, "role_name": "Owner", }, diff --git a/warehouse/manage/views.py b/warehouse/manage/views.py index f5736b17f5b3..fc8caf89aba8 100644 --- a/warehouse/manage/views.py +++ b/warehouse/manage/views.py @@ -1020,7 +1020,7 @@ def create_organization(self): self.organization_service.record_event( organization.id, tag="organization:create", - additional={"created_by": self.request.user.username}, + additional={"created_by_id": str(self.request.user.id)}, ) self.organization_service.add_catalog_entry( organization.name, organization.id @@ -1028,7 +1028,7 @@ def create_organization(self): self.organization_service.record_event( organization.id, tag="organization:catalog_entry:add", - additional={"submitted_by": self.request.user.username}, + additional={"submitted_by_id": str(self.request.user.id)}, ) self.organization_service.add_organization_role( "Owner", self.request.user.id, organization.id @@ -1037,25 +1037,25 @@ def create_organization(self): organization.id, tag="organization:organization_role:invite", additional={ - "submitted_by": self.request.user.username, + "submitted_by_id": str(self.request.user.id), "role_name": "Owner", - "target_user": self.request.user.username, + "target_user_id": str(self.request.user.id), }, ) self.organization_service.record_event( organization.id, tag="organization:organization_role:accepted", additional={ - "submitted_by": self.request.user.username, + "submitted_by_id": str(self.request.user.id), "role_name": "Owner", - "target_user": self.request.user.username, + "target_user_id": str(self.request.user.id), }, ) self.user_service.record_event( self.request.user.id, tag="account:organization_role:accepted", additional={ - "submitted_by": self.request.user.username, + "submitted_by_id": str(self.request.user.id), "organization_name": organization.name, "role_name": "Owner", }, From 49fafeecbf2ef5c768d016aa22cf34b30a0e3a5b Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Wed, 20 Apr 2022 08:43:24 -0700 Subject: [PATCH 45/46] Display CreateOrganizationForm errors If there is a validation error, return the existing invalid form instead of a new blank form so user can actually see that validation error. --- tests/unit/manage/test_views.py | 7 +------ warehouse/locale/messages.pot | 16 ++++++++-------- warehouse/manage/views.py | 2 ++ 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/tests/unit/manage/test_views.py b/tests/unit/manage/test_views.py index 636a5ba9e9b6..8ffb8c41da3b 100644 --- a/tests/unit/manage/test_views.py +++ b/tests/unit/manage/test_views.py @@ -2567,11 +2567,6 @@ def test_create_organization_validation_fails(self, monkeypatch): ) monkeypatch.setattr(views, "send_new_organization_requested_email", send_email) - default_response = {"default": "response"} - monkeypatch.setattr( - views.ManageOrganizationsViews, "default_response", default_response - ) - view = views.ManageOrganizationsViews(request) result = view.create_organization() @@ -2584,7 +2579,7 @@ def test_create_organization_validation_fails(self, monkeypatch): assert organization_service.add_organization_role.calls == [] assert organization_service.record_event.calls == [] assert send_email.calls == [] - assert result == default_response + assert result == {"create_organization_form": create_organization_obj} def test_create_organizations_disallow_organizations(self, monkeypatch): request = pretend.stub( diff --git a/warehouse/locale/messages.pot b/warehouse/locale/messages.pot index 4e809c5e4ca1..33de5fb6767f 100644 --- a/warehouse/locale/messages.pot +++ b/warehouse/locale/messages.pot @@ -277,39 +277,39 @@ msgstr "" msgid "Generating new recovery codes will invalidate your existing codes." msgstr "" -#: warehouse/manage/views.py:1298 +#: warehouse/manage/views.py:1300 msgid "" "There have been too many attempted OpenID Connect registrations. Try " "again later." msgstr "" -#: warehouse/manage/views.py:1979 +#: warehouse/manage/views.py:1981 msgid "User '${username}' already has ${role_name} role for project" msgstr "" -#: warehouse/manage/views.py:1990 +#: warehouse/manage/views.py:1992 msgid "" "User '${username}' does not have a verified primary email address and " "cannot be added as a ${role_name} for project" msgstr "" -#: warehouse/manage/views.py:2003 +#: warehouse/manage/views.py:2005 msgid "User '${username}' already has an active invite. Please try again later." msgstr "" -#: warehouse/manage/views.py:2061 +#: warehouse/manage/views.py:2063 msgid "Invitation sent to '${username}'" msgstr "" -#: warehouse/manage/views.py:2108 +#: warehouse/manage/views.py:2110 msgid "Could not find role invitation." msgstr "" -#: warehouse/manage/views.py:2119 +#: warehouse/manage/views.py:2121 msgid "Invitation already expired." msgstr "" -#: warehouse/manage/views.py:2143 +#: warehouse/manage/views.py:2145 msgid "Invitation revoked from '${username}'." msgstr "" diff --git a/warehouse/manage/views.py b/warehouse/manage/views.py index fc8caf89aba8..932248a92649 100644 --- a/warehouse/manage/views.py +++ b/warehouse/manage/views.py @@ -1072,6 +1072,8 @@ def create_organization(self): self.request.session.flash( "Request for new organization submitted", queue="success" ) + else: + return {"create_organization_form": form} return self.default_response From c37c786d73d99b0138e46507511f072b4f58807e Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Wed, 20 Apr 2022 10:01:15 -0700 Subject: [PATCH 46/46] Tweak naming in events data {*_id => *_user_id} - {created_by_id => created_by_user_id} - {submitted_by_id => submitted_by_user_id} Discussed with @ewdurbin. Using `*_user_id` seems more clear. --- tests/unit/manage/test_views.py | 10 +++++----- warehouse/manage/views.py | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/unit/manage/test_views.py b/tests/unit/manage/test_views.py index 8ffb8c41da3b..6c97740d84f8 100644 --- a/tests/unit/manage/test_views.py +++ b/tests/unit/manage/test_views.py @@ -2461,18 +2461,18 @@ def test_create_organization(self, monkeypatch): pretend.call( organization.id, tag="organization:create", - additional={"created_by_id": str(request.user.id)}, + additional={"created_by_user_id": str(request.user.id)}, ), pretend.call( organization.id, tag="organization:catalog_entry:add", - additional={"submitted_by_id": str(request.user.id)}, + additional={"submitted_by_user_id": str(request.user.id)}, ), pretend.call( organization.id, tag="organization:organization_role:invite", additional={ - "submitted_by_id": str(request.user.id), + "submitted_by_user_id": str(request.user.id), "role_name": "Owner", "target_user_id": str(request.user.id), }, @@ -2481,7 +2481,7 @@ def test_create_organization(self, monkeypatch): organization.id, tag="organization:organization_role:accepted", additional={ - "submitted_by_id": str(request.user.id), + "submitted_by_user_id": str(request.user.id), "role_name": "Owner", "target_user_id": str(request.user.id), }, @@ -2492,7 +2492,7 @@ def test_create_organization(self, monkeypatch): request.user.id, tag="account:organization_role:accepted", additional={ - "submitted_by_id": str(request.user.id), + "submitted_by_user_id": str(request.user.id), "organization_name": organization.name, "role_name": "Owner", }, diff --git a/warehouse/manage/views.py b/warehouse/manage/views.py index 932248a92649..423318ecb924 100644 --- a/warehouse/manage/views.py +++ b/warehouse/manage/views.py @@ -1020,7 +1020,7 @@ def create_organization(self): self.organization_service.record_event( organization.id, tag="organization:create", - additional={"created_by_id": str(self.request.user.id)}, + additional={"created_by_user_id": str(self.request.user.id)}, ) self.organization_service.add_catalog_entry( organization.name, organization.id @@ -1028,7 +1028,7 @@ def create_organization(self): self.organization_service.record_event( organization.id, tag="organization:catalog_entry:add", - additional={"submitted_by_id": str(self.request.user.id)}, + additional={"submitted_by_user_id": str(self.request.user.id)}, ) self.organization_service.add_organization_role( "Owner", self.request.user.id, organization.id @@ -1037,7 +1037,7 @@ def create_organization(self): organization.id, tag="organization:organization_role:invite", additional={ - "submitted_by_id": str(self.request.user.id), + "submitted_by_user_id": str(self.request.user.id), "role_name": "Owner", "target_user_id": str(self.request.user.id), }, @@ -1046,7 +1046,7 @@ def create_organization(self): organization.id, tag="organization:organization_role:accepted", additional={ - "submitted_by_id": str(self.request.user.id), + "submitted_by_user_id": str(self.request.user.id), "role_name": "Owner", "target_user_id": str(self.request.user.id), }, @@ -1055,7 +1055,7 @@ def create_organization(self): self.request.user.id, tag="account:organization_role:accepted", additional={ - "submitted_by_id": str(self.request.user.id), + "submitted_by_user_id": str(self.request.user.id), "organization_name": organization.name, "role_name": "Owner", },