Manage organization "Projects" #11473
Conversation
divbzero
commented
May 28, 2022
divbzero
commented
- Closes Transfer a project #11085.
- Closes Create a project #11086.
I really mean it this time.
Moving the code from warehouse/forklift/legacy.py to a separate function `warehouse.utils.project.add_project` will let us reuse it elsewhere.
- {CreateOrganizationProjectForm => AddOrganizationProjectForm}
- {create_organization_project_form => add_organization_project_form}
- "organization-project-added" - "organization-project-removed"
- `send_organization_project_added` - `send_organization_project_removed`
`@pytest.mark.parametrize` to reuse the same `test_validate` code.
Project must have at least one individual owner before it can be removed from an organization.
Don't show the "Transfer project" button if the user odes not own any organizations that the project can be transferred to.
- Move Owner role creation out of the function - Move validation code to separate `validate_project_name` function
There was a problem hiding this comment.
In review, the nuts and bolts of this look great. A couple UX things and points of clarification for transfers:
UX:
- When viewing projects owned by an organization either by transfer or creation, as an Owner of the Organization I am able to directly hit the manage page, but the buttons and badges on the page are not accurate. Specifically the "Manage" button is greyed out. ACL is good though since I can navigate to the project manage page directly. I think the buttons/badges are determined based on https://github.com/pypa/warehouse/blob/028dffea1db70ca6f884cf9b81cedb3a12fc49a2/warehouse/manage/views.py#L128-L181 so we need to see some updates there, I'm sure this will be confusing during user testing.
- "Sole Owner" starts to lose meaning for Organization accounts but could be similarly determined by calculating the total number of Users with either a direct Owner Role or ownership via an Organization.
- I would suspect that people will be confused not seeing Organization Projects they have access to in their main project view. Perhaps this should come later? But again I'd suspect this would be flagged during user testing.
Clarification on transfers:
- After transferring a project, my User still holds an Owner Role on the project directly, and I cannot remove that role myself. I'm not positive what people would expect in this scenario, but my gut says "If I transfer this project to an Organization, I'm relinquishing my owner bit" so I think when transferring either from the project management or org management sections the Owner Role should be removed.
As @ewdurbin noted when reviewing pypi#11473, I had not adjusted the queries for a user's projects to account for projects managed by organizations owned by the user. Fixed by adding `UNION` clauses to those queries.
Projects in organizations owned by the user will be displayed in the user's main projects list.
|
@ewdurbin I’ve addressed the UX things in the most recent 3 commits, please take a look to see if they seem like the right fix. For the clarification on transfers, would we want every individual project Owner to relinquish their ownership when the project is transferred to an organization? Would we also disallow the invitation of an individual project Owner if the project belongs to an organization? |
@divbzero I think that is reasonable. If actions on a given project require a specific permission that matches the "owner" role permission level now that should be granted via roles within orgs or projects. |
|
These revisions are 👍🏼. As far as I can tell, once the outstanding question on Owner handling is settled this is ready to go. One note is that the Sole Owner badge currently seems to get confused. If I was a sole owner on a non-org project and transfer it to an org with multiple owners... I still get the badge. Hence the "I think once the outstanding question is handled" above! It should be resolved. |
| {% if project.organizations %} | ||
| <a href="{{ request.route_path('manage.organization.projects', organization_name=project.organizations[0].normalized_name) }}" class="badge package-snippet__badge"> | ||
| <i class="fa fa-users" aria-hidden="true"></i> | ||
| {{ project.organizations[0].name }} |
There was a problem hiding this comment.
Interesting note... Hmmm wondering if we should enforce single org ownership for projects?
There was a problem hiding this comment.
The relationship looks because we are using an organization_project association table, but there will only ever be a single organization for each project.
There was a problem hiding this comment.
To avoid confusion in the future, I’ve changed the Project.organizations relationship to Project.organization with uselist=False.
@ewdurbin pointed out that the previous version of the query would fail if a project with one individual owner is also owned by an organization.
Refactor relationship via association table to be one-to-many to avoid confusion in the future.
When a user transfers a project to an organization, remove that user as an individual owner of the project. As an owner of the organization, the user will still have full permissions over the project.
|
I’ve also fixed up the sole owner query and removed the owner role when that owner transfers project to an organization. |
|
Everything looks great! Thanks for being so diligent in numerous review rounds. |
3 participants
