Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

warehouse, tests: pick DB changes from #11122 #11157

Merged
merged 7 commits into from
Apr 12, 2022

Conversation

woodruffw
Copy link
Member

@woodruffw woodruffw commented Apr 11, 2022

This is a breakout of #11122 , containing just the DB migration and associated changes.

Main changes:

  • The macaroon DB model's caveats are renamed to permissions_caveat. No pre-existing data is dropped, and the format of the serialized data is preserved.
  • Caveats in general are restructured to conform to the macaroon data-model -- the top level type is a list instead of dictionary, and we handle multiple caveats correctly (although we still only supply one for the time being)
  • Event rendering for events that contain caveats (account:api_token:added) is refactored to handle multiple caveats correctly, via the new caveat_detail macro.

@woodruffw woodruffw requested a review from di April 11, 2022 16:55
@woodruffw woodruffw self-assigned this Apr 11, 2022
@woodruffw woodruffw mentioned this pull request Apr 11, 2022
{% else %}
{% trans project_name=caveat.permissions.projects[0] %}Token scope: Project {{ project_name }}{% endtrans %}
{% endif %}
{% elif "exp" in caveat %}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

N.B.: This check is a no-op until #11122. I can remove it from this PR and introduce it there instead, if you'd like.

warehouse/manage/views.py Outdated Show resolved Hide resolved
warehouse/macaroons/models.py Outdated Show resolved Hide resolved
Emphasizes that this is the entire caveat, and not just the permissions body.
@woodruffw woodruffw requested a review from di April 11, 2022 18:46
Prior to these changes, the `caveats` field in API token events was
a dictionary, not a list.
@di di requested a review from ewdurbin April 11, 2022 22:57
@di
Copy link
Member

di commented Apr 11, 2022

LGTM. @ewdurbin, would appreciate your review but I'll probably merge in ~24 hours or so if you are unable.

@di di merged commit 9a84e62 into pypi:main Apr 12, 2022
@di di deleted the tob-simplify-macaroon-db-model branch April 12, 2022 13:40
Comment on lines +26 to +27
def upgrade():
op.alter_column("macaroons", "caveats", new_column_name="permissions_caveat")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had a brief outage here between the time when the migration was applied and when the code change was deployed. In the future we should make sure that any rename like this happens in two phases: a migration to duplicate the old column to the new column goes with the code change, and then a followup migration removes the old column.

divbzero added a commit to divbzero/warehouse that referenced this pull request Apr 13, 2022
    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 (pypi#11157).
divbzero added a commit to divbzero/warehouse that referenced this pull request Apr 13, 2022
    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 (pypi#11157).
divbzero added a commit to divbzero/warehouse that referenced this pull request Apr 15, 2022
    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 (pypi#11157).
ewdurbin pushed a commit that referenced this pull request Apr 20, 2022
* admin-new-organization-requested email template

* new-orgnization-requested email template

* Test admin-new-organization-requested email

* Test new-organization-requested email

* Translate *-organization-requested consistently

* `make translations` for *-organization-requested

* Remove translations from admin-* emails

On second thought, we probably don't want localization for admin emails.

* Initial cut at db model architecture

* Ran code thru make reformat and lint (pypa#11070)

* Added tests for new db models (pypa#11070)

* Numerous tweaks to db models

* Require value for is_active and ran reformat.

* 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).

* Numerous tweaks to alembic scripts

* Initial implementation of organization service.

* Implementing organization events functionality.

* Added tests for organization services.

* Added OrganizationFactory class for model.

* Blank /manage/organizations/ page

* Create organization form on /manage/organizations/

- Organization account name
- Organization name
- Organization URL
- Organization description
- Organization type

* Register DatabaseOrganizationService

Found the droids that we're looking for.

* POST /manage/organizations/ database updates

* Add .get_admins method to user service

* POST /manage/organizations/ email notifications

* 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.

* Test GET /manage/organizations/

- `ManageOrganizationsViews.default_response`
- `ManageOrganizationsViews.manage_organizations()`

* Translations for /manage/organizations/

    make translations

* Fixed OrganizationType enum.

* Test POST /manage/organizations/

- `ManageOrganizationsViews.create_organization()`

* Test CreateOrganizationForm

* Placeholder to test /admin/organizations/approve/

Provides code coverage for the blank page added in 2c70616.

* Test `DatabaseUserService.get_admins()`

* NFC: Add comments about intentionally blank page

* Record events for POST /manage/organizations/

Co-Authored-By: sterbo <matt.sterba@gmail.com>

* Test record events for POST /manage/organizations/

* Functional test for POST /manage/organizations/

* Comment out `OrganizationFactory` for future use

Co-Authored-By: sterbo <matt.sterba@gmail.com>

* NFC: Fix camel case for class names

* Converted OrganizationRoleType to SQLAlchemy Enum

* Add disable-organizations global admin flag

* Add `AdminFlagValue.DISABLE_ORGANIZATIONS`

* Modified org name catalog to store normalized name

* {OrganizationEvents => Organization.Events}

- Remove `OrganizationEvents` class
- Add `HasEvents` mixing to `Organization`
- Update references {OrganizationEvents => Organization.Events}
- Update database migration {organization_id => source_id}

* 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

* 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.

* 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.

Co-authored-by: sterbo <matt.sterba@gmail.com>
domdfcoding pushed a commit to domdfcoding/warehouse that referenced this pull request Jun 7, 2022
* warehouse, tests: pick DB changes from pypi#11122

* warehouse: `make translations`

* manage/views: remove outdated note

* warehouse, tests: `Macaroon.permissions -> Macaroon.permissions_caveat`

Emphasizes that this is the entire caveat, and not just the permissions body.

* warehouse: `make translations`

* warehouse/templates: handle stale event caveats

Prior to these changes, the `caveats` field in API token events was
a dictionary, not a list.

* warehouse: `make translations`
domdfcoding pushed a commit to domdfcoding/warehouse that referenced this pull request Jun 7, 2022
* admin-new-organization-requested email template

* new-orgnization-requested email template

* Test admin-new-organization-requested email

* Test new-organization-requested email

* Translate *-organization-requested consistently

* `make translations` for *-organization-requested

* Remove translations from admin-* emails

On second thought, we probably don't want localization for admin emails.

* Initial cut at db model architecture

* Ran code thru make reformat and lint (pypa#11070)

* Added tests for new db models (pypa#11070)

* Numerous tweaks to db models

* Require value for is_active and ran reformat.

* 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 (pypi#11157).

* Numerous tweaks to alembic scripts

* Initial implementation of organization service.

* Implementing organization events functionality.

* Added tests for organization services.

* Added OrganizationFactory class for model.

* Blank /manage/organizations/ page

* Create organization form on /manage/organizations/

- Organization account name
- Organization name
- Organization URL
- Organization description
- Organization type

* Register DatabaseOrganizationService

Found the droids that we're looking for.

* POST /manage/organizations/ database updates

* Add .get_admins method to user service

* POST /manage/organizations/ email notifications

* 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.

* Test GET /manage/organizations/

- `ManageOrganizationsViews.default_response`
- `ManageOrganizationsViews.manage_organizations()`

* Translations for /manage/organizations/

    make translations

* Fixed OrganizationType enum.

* Test POST /manage/organizations/

- `ManageOrganizationsViews.create_organization()`

* Test CreateOrganizationForm

* Placeholder to test /admin/organizations/approve/

Provides code coverage for the blank page added in 2c70616.

* Test `DatabaseUserService.get_admins()`

* NFC: Add comments about intentionally blank page

* Record events for POST /manage/organizations/

Co-Authored-By: sterbo <matt.sterba@gmail.com>

* Test record events for POST /manage/organizations/

* Functional test for POST /manage/organizations/

* Comment out `OrganizationFactory` for future use

Co-Authored-By: sterbo <matt.sterba@gmail.com>

* NFC: Fix camel case for class names

* Converted OrganizationRoleType to SQLAlchemy Enum

* Add disable-organizations global admin flag

* Add `AdminFlagValue.DISABLE_ORGANIZATIONS`

* Modified org name catalog to store normalized name

* {OrganizationEvents => Organization.Events}

- Remove `OrganizationEvents` class
- Add `HasEvents` mixing to `Organization`
- Update references {OrganizationEvents => Organization.Events}
- Update database migration {organization_id => source_id}

* 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

* 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.

* 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.

Co-authored-by: sterbo <matt.sterba@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants