Skip to content

Commit

Permalink
resolve new errors introduced in #17405
Browse files Browse the repository at this point in the history
changing from return value to exception was not caught due to stubs.

it's unclear to me how the changes in #17405 were expected to work, or if they were intended to be surfaced in the UI.
  • Loading branch information
ewdurbin committed Feb 6, 2025
1 parent 4775901 commit b2a286e
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 63 deletions.
4 changes: 2 additions & 2 deletions tests/unit/oidc/forms/test_activestate.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def test_validate_project_name_already_in_use_owner(self, pyramid_config):
owners = [user]

def check_project_name(name):
return ProjectNameUnavailableExistingError(
raise ProjectNameUnavailableExistingError(
existing_project=pretend.stub(owners=owners)
)

Expand Down Expand Up @@ -98,7 +98,7 @@ def test_validate_project_name_already_in_use_not_owner(self, pyramid_config):
owners = []

def check_project_name(name):
return ProjectNameUnavailableExistingError(
raise ProjectNameUnavailableExistingError(
existing_project=pretend.stub(owners=owners)
)

Expand Down
23 changes: 16 additions & 7 deletions tests/unit/oidc/forms/test_github.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,15 @@ def test_validate_project_name_already_in_use_owner(self, pyramid_config):
user = pretend.stub()
owners = [user]

def check_project_name(name):
raise ProjectNameUnavailableExistingError(
existing_project=pretend.stub(owners=owners)
)

form = github.PendingGitHubPublisherForm(
api_token="fake-token",
route_url=route_url,
check_project_name=lambda name: ProjectNameUnavailableExistingError(
existing_project=pretend.stub(owners=owners)
),
check_project_name=check_project_name,
user=user,
)

Expand All @@ -93,12 +96,15 @@ def test_validate_project_name_already_in_use_not_owner(self, pyramid_config):
user = pretend.stub()
owners = []

def check_project_name(name):
raise ProjectNameUnavailableExistingError(
existing_project=pretend.stub(owners=owners)
)

form = github.PendingGitHubPublisherForm(
api_token="fake-token",
route_url=route_url,
check_project_name=lambda name: ProjectNameUnavailableExistingError(
existing_project=pretend.stub(owners=owners)
),
check_project_name=check_project_name,
user=user,
)

Expand All @@ -119,10 +125,13 @@ def test_validate_project_name_already_in_use_not_owner(self, pyramid_config):
],
)
def test_validate_project_name_unavailable(self, reason, pyramid_config):
def check_project_name(name):
raise reason

form = github.PendingGitHubPublisherForm(
api_token="fake-token",
route_url=pretend.call_recorder(lambda *args, **kwargs: ""),
check_project_name=lambda name: reason,
check_project_name=check_project_name,
user=pretend.stub(),
)

Expand Down
18 changes: 12 additions & 6 deletions tests/unit/oidc/forms/test_gitlab.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,14 @@ def test_validate_project_name_already_in_use_owner(self, pyramid_config):
owners = [user]
route_url = pretend.call_recorder(lambda *args, **kwargs: "my_url")

def check_project_name(name):
raise ProjectNameUnavailableExistingError(
existing_project=pretend.stub(owners=owners)
)

form = gitlab.PendingGitLabPublisherForm(
route_url=route_url,
check_project_name=lambda name: ProjectNameUnavailableExistingError(
existing_project=pretend.stub(owners=owners)
),
check_project_name=check_project_name,
user=user,
)

Expand All @@ -81,11 +84,14 @@ def test_validate_project_name_already_in_use_not_owner(self, pyramid_config):
owners = []
route_url = pretend.call_recorder(lambda *args, **kwargs: "my_url")

def check_project_name(name):
raise ProjectNameUnavailableExistingError(
existing_project=pretend.stub(owners=owners)
)

form = gitlab.PendingGitLabPublisherForm(
route_url=route_url,
check_project_name=lambda name: ProjectNameUnavailableExistingError(
existing_project=pretend.stub(owners=owners)
),
check_project_name=check_project_name,
user=user,
)

Expand Down
20 changes: 14 additions & 6 deletions tests/unit/oidc/forms/test_google.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,15 @@ def test_validate_project_name_already_in_use_owner(self, pyramid_config):
user = pretend.stub()
owners = [user]
route_url = pretend.call_recorder(lambda *args, **kwargs: "my_url")

def check_project_name(name):
raise ProjectNameUnavailableExistingError(
existing_project=pretend.stub(owners=owners)
)

form = google.PendingGooglePublisherForm(
route_url=route_url,
check_project_name=lambda name: ProjectNameUnavailableExistingError(
existing_project=pretend.stub(owners=owners)
),
check_project_name=check_project_name,
user=user,
)

Expand All @@ -77,11 +81,15 @@ def test_validate_project_name_already_in_use_not_owner(self, pyramid_config):
user = pretend.stub()
owners = []
route_url = pretend.call_recorder(lambda *args, **kwargs: "my_url")

def check_project_name(name):
raise ProjectNameUnavailableExistingError(
existing_project=pretend.stub(owners=owners)
)

form = google.PendingGooglePublisherForm(
route_url=route_url,
check_project_name=lambda name: ProjectNameUnavailableExistingError(
existing_project=pretend.stub(owners=owners)
),
check_project_name=check_project_name,
user=user,
)

Expand Down
83 changes: 41 additions & 42 deletions warehouse/oidc/forms/_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,55 +36,54 @@ class PendingPublisherMixin:
def validate_project_name(self, field):
project_name = field.data

match self._check_project_name(project_name):
case ProjectNameUnavailableInvalidError():
raise wtforms.validators.ValidationError(_("Invalid project name"))
case ProjectNameUnavailableExistingError(existing_project=existing_project):
# If the user owns the existing project, the error message includes a
# link to the project settings that the user can modify.
if self._user in existing_project.owners:
url_params = {
name: value for name, value in self.data.items() if value
}
url_params["provider"] = {self.provider}
url = self._route_url(
"manage.project.settings.publishing",
project_name=project_name,
_query=url_params,
)
try:
self._check_project_name(project_name)
except ProjectNameUnavailableInvalidError:
raise wtforms.validators.ValidationError(_("Invalid project name"))
except ProjectNameUnavailableExistingError as e:
# If the user owns the existing project, the error message includes a
# link to the project settings that the user can modify.
if self._user in e.existing_project.owners:
url_params = {name: value for name, value in self.data.items() if value}
url_params["provider"] = {self.provider}
url = self._route_url(
"manage.project.settings.publishing",
project_name=project_name,
_query=url_params,
)

# We mark the error message as safe, so that the HTML hyperlink is
# not escaped by Jinja
raise wtforms.validators.ValidationError(
markupsafe.Markup(
_(
"This project already exists: use the project's "
"publishing settings <a href='${url}'>here</a> to "
"create a Trusted Publisher for it.",
mapping={"url": url},
)
# We mark the error message as safe, so that the HTML hyperlink is
# not escaped by Jinja
raise wtforms.validators.ValidationError(
markupsafe.Markup(
_(
"This project already exists: use the project's "
"publishing settings <a href='${url}'>here</a> to "
"create a Trusted Publisher for it.",
mapping={"url": url},
)
)
else:
raise wtforms.validators.ValidationError(
_("This project already exists.")
)

case ProjectNameUnavailableProhibitedError():
raise wtforms.validators.ValidationError(
_("This project name isn't allowed")
)
case ProjectNameUnavailableSimilarError():
else:
raise wtforms.validators.ValidationError(
_("This project name is too similar to an existing project")
_("This project already exists.")
)
case ProjectNameUnavailableStdlibError():
raise wtforms.validators.ValidationError(
_(
"This project name isn't allowed (conflict with the Python"
" standard library module name)"
)

except ProjectNameUnavailableProhibitedError:
raise wtforms.validators.ValidationError(
_("This project name isn't allowed")
)
except ProjectNameUnavailableSimilarError:
raise wtforms.validators.ValidationError(
_("This project name is too similar to an existing project")
)
except ProjectNameUnavailableStdlibError:
raise wtforms.validators.ValidationError(
_(
"This project name isn't allowed (conflict with the Python"
" standard library module name)"
)
)

@property
def provider(self) -> str: # pragma: no cover
Expand Down

0 comments on commit b2a286e

Please sign in to comment.