Skip to content

surface conflicting prefixes when reviewing organizations #18036

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 30 additions & 4 deletions tests/unit/admin/views/test_organizations.py
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,7 @@ def test_detail(self, db_request):
assert result["user"] == organization_application.submitted_by
assert result["form"].name.data == organization_application.name
assert result["conflicting_applications"] == []
assert result["conflicting_namespace_prefixes"] == []
assert result["organization_application"] == organization_application

@pytest.mark.usefixtures("_enable_organizations")
Expand Down Expand Up @@ -669,6 +670,7 @@ def test_detail_edit_invalid(self, db_request):
assert result["form"].name.data == existing_organization.name
assert result["form"].name.errors != []
assert result["conflicting_applications"] == []
assert result["conflicting_namespace_prefixes"] == []
assert result["organization_application"] == organization_application

@pytest.mark.usefixtures("_enable_organizations")
Expand All @@ -683,6 +685,7 @@ def test_detail_is_approved_true(self, db_request):
assert result["user"] == organization_application.submitted_by
assert result["form"].name.data == organization_application.name
assert result["conflicting_applications"] == []
assert result["conflicting_namespace_prefixes"] == []
assert result["organization_application"] == organization_application

@pytest.mark.usefixtures("_enable_organizations")
Expand All @@ -697,17 +700,25 @@ def test_detail_is_approved_false(self, db_request):
assert result["user"] == organization_application.submitted_by
assert result["form"].name.data == organization_application.name
assert result["conflicting_applications"] == []
assert result["conflicting_namespace_prefixes"] == []
assert result["organization_application"] == organization_application

@pytest.mark.usefixtures("_enable_organizations")
@pytest.mark.parametrize(
("name", "conflicts"),
("name", "conflicts", "conflicting_prefixes", "not_conflicting"),
[
("pypi", ["PyPI", "pypi"]),
("py-pi", ["Py-PI", "PY-PI"]),
(
"pypi",
["PyPI", "pypi"],
["pypi-common", "PyPi_rocks", "pypi-team-garbage"],
["py-pi"],
),
("py-pi", ["Py-PI", "PY-PI"], ["py", "py-pi_dot-com"], ["pypi"]),
],
)
def test_detail_conflicting_applications(self, db_request, name, conflicts):
def test_detail_conflicting_applications(
self, db_request, name, conflicts, conflicting_prefixes, not_conflicting
):
organization_application = OrganizationApplicationFactory.create(
name=name, status=OrganizationApplicationStatus.Declined
)
Expand All @@ -718,13 +729,28 @@ def test_detail_conflicting_applications(self, db_request, name, conflicts):
],
key=lambda o: o.submitted,
)
conflicting_prefix_applications = sorted(
[
OrganizationApplicationFactory.create(name=conflict)
for conflict in conflicting_prefixes
],
key=lambda o: o.submitted,
)
[OrganizationApplicationFactory.create(name=name) for name in not_conflicting]
db_request.matchdict["organization_application_id"] = (
organization_application.id
)
result = views.organization_application_detail(db_request)
assert result["user"] == organization_application.submitted_by
assert result["form"].name.data == organization_application.name
assert result["conflicting_applications"] == conflicting_applications
assert result["conflicting_namespace_prefixes"] == [
conflict
for conflict in sorted(
conflicting_applications + conflicting_prefix_applications,
key=lambda o: o.submitted,
)
]
assert result["organization_application"] == organization_application

@pytest.mark.usefixtures("_enable_organizations")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -388,10 +388,31 @@ <h3 class="card-title">Organization Request{% if information_requests %}{% if ou
</label>
<div class="col-12">
<table class="table">
<tr><th>Application</th><th>Submitted</th><th>Requestor</th></tr>
<tr><th>Application</th><th>Status</th><th>Submitted</th><th>Requestor</th></tr>
{% for application in conflicting_applications %}
<tr>
<td><a target="_blank" href="{{ request.route_url('admin.organization_application.detail', organization_application_id=application.id) }}">{{ application.name }}</a></td>
<td>{{ application.status }}</td>
<td>{{ application.submitted|format_date() }}</td>
<td><a target="_blank" href="{{ request.route_url('admin.user.detail', username=application.submitted_by.username) }}">{{ application.submitted_by.username }}</a></td>
</tr>
{% endfor %}
</table>
</div>
</div>
{% endif %}
{% if conflicting_namespace_prefixes %}
<div class="form-group">
<label class="col-12 control-label">
Conflicting Prefixes <i class="fa fa-clipboard-question text-orange"></i>
</label>
<div class="col-12">
<table class="table">
<tr><th>Application</th><th>Status</th><th>Submitted</th><th>Requestor</th></tr>
{% for application in conflicting_namespace_prefixes %}
<tr>
<td><a target="_blank" href="{{ request.route_url('admin.organization_application.detail', organization_application_id=application.id) }}">{{ application.name }}</a></td>
<td>{{ application.status }}</td>
Comment on lines +404 to +415
Copy link
Member

Choose a reason for hiding this comment

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

I think we could probably just replace the 'conflicting applications' section with this rather than potentially duplicating it here, it should be clear enough which are exact matches & which are prefixes?

<td>{{ application.submitted|format_date() }}</td>
<td><a target="_blank" href="{{ request.route_url('admin.user.detail', username=application.submitted_by.username) }}">{{ application.submitted_by.username }}</a></td>
</tr>
Expand Down
27 changes: 27 additions & 0 deletions warehouse/admin/views/organizations.py
Original file line number Diff line number Diff line change
Expand Up @@ -372,12 +372,39 @@ def organization_application_detail(request):
.all()
)

parts = organization_application.normalized_name.split("-")
conflicting_namespace_prefixes = (
request.db.query(OrganizationApplication)
.filter(
or_(
*(
[
OrganizationApplication.normalized_name == parts[0],
OrganizationApplication.normalized_name.startswith(
parts[0] + "-"
),
]
+ [
OrganizationApplication.normalized_name.startswith(
"-".join(parts[: i + 1])
)
for i in range(1, len(parts))
]
)
)
)
.filter(OrganizationApplication.id != organization_application.id)
.order_by(OrganizationApplication.submitted)
.all()
)

user = user_service.get_user(organization_application.submitted_by_id)

return {
"organization_application": organization_application,
"form": form,
"conflicting_applications": conflicting_applications,
"conflicting_namespace_prefixes": conflicting_namespace_prefixes,
"user": user,
}

Expand Down