This is an automated email from the ASF dual-hosted git repository.
sbp pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tooling-trusted-releases.git
The following commit(s) were added to refs/heads/main by this push:
new fc3f43b Make admin forms to delete keys and releases more type safe
fc3f43b is described below
commit fc3f43b4725837214206d9c7f1b3efb20b267748
Author: Sean B. Palmer <[email protected]>
AuthorDate: Sun Nov 16 20:38:14 2025 +0000
Make admin forms to delete keys and releases more type safe
---
atr/admin/__init__.py | 215 +++++++++++++++++--------------
atr/admin/templates/delete-release.html | 36 +-----
atr/blueprints/admin.py | 10 +-
atr/post/finish.py | 5 +-
atr/templates/delete-committee-keys.html | 22 +---
playwright/test.py | 16 +--
scripts/docs_check.py | 3 +-
7 files changed, 139 insertions(+), 168 deletions(-)
diff --git a/atr/admin/__init__.py b/atr/admin/__init__.py
index 9153f6d..30bfde7 100644
--- a/atr/admin/__init__.py
+++ b/atr/admin/__init__.py
@@ -30,6 +30,8 @@ import aiohttp
import asfquart
import asfquart.base as base
import asfquart.session
+import htpy
+import pydantic
import quart
import sqlalchemy.orm as orm
@@ -41,6 +43,7 @@ import atr.db.interaction as interaction
import atr.form as form
import atr.forms as forms
import atr.get as get
+import atr.htm as htm
import atr.ldap as ldap
import atr.log as log
import atr.mapping as mapping
@@ -62,26 +65,35 @@ class BrowseAsUserForm(form.Form):
uid: str = form.label("ASF UID", "Enter the ASF UID to browse as.")
-class DeleteCommitteeKeysForm(forms.Typed):
- committee_name = forms.select("Committee")
- confirm_delete = forms.string(
- "Confirmation",
- validators=forms.constant("DELETE KEYS"),
- placeholder="DELETE KEYS",
- )
- submit = forms.submit("Delete all keys for selected committee")
+class DeleteCommitteeKeysForm(form.Form):
+ committee_name: str = form.label("Committee", widget=form.Widget.SELECT)
+ confirm_delete: str = form.label("Confirmation", "Type DELETE KEYS to
confirm")
+ @pydantic.field_validator("confirm_delete")
+ @classmethod
+ def validate_confirm_delete(cls, v: str) -> str:
+ if v != "DELETE KEYS":
+ raise ValueError("You must type DELETE KEYS exactly to confirm
deletion")
+ return v
-class DeleteReleaseForm(forms.Typed):
- """Form for deleting releases."""
- confirm_delete = forms.string(
- "Confirmation",
- validators=forms.constant("DELETE"),
- placeholder="DELETE",
- description="Please type DELETE exactly to confirm deletion.",
- )
- submit = forms.submit("Delete selected releases permanently")
+class DeleteReleaseForm(form.Form):
+ releases_to_delete: form.StrList = form.label("Select releases to delete",
widget=form.Widget.CUSTOM)
+ confirm_delete: str = form.label("Confirmation", "Please type DELETE
exactly to confirm deletion.")
+
+ @pydantic.field_validator("confirm_delete")
+ @classmethod
+ def validate_confirm_delete(cls, v: str) -> str:
+ if v != "DELETE":
+ raise ValueError("You must type DELETE exactly to confirm
deletion")
+ return v
+
+ @pydantic.field_validator("releases_to_delete")
+ @classmethod
+ def validate_releases_to_delete(cls, v: form.StrList) -> form.StrList:
+ if len(v) == 0:
+ raise ValueError("You must select at least one release to delete")
+ return v
class LdapLookupForm(forms.Typed):
@@ -283,7 +295,7 @@ async def _data(session: web.Committer, model: str =
"Committee") -> str:
@admin.get("/delete-test-openpgp-keys")
async def delete_test_openpgp_keys_get(session: web.Committer) -> web.Response:
- """Display form to delete test user OpenPGP keys."""
+ """Display the form to delete test user OpenPGP keys."""
if not config.get().ALLOW_TESTS:
raise base.ASFQuartException("Test operations are disabled in this
environment", errorcode=403)
@@ -323,102 +335,114 @@ async def delete_test_openpgp_keys_post(session:
web.Committer) -> web.Response:
@admin.get("/delete-committee-keys")
async def delete_committee_keys_get(session: web.Committer) -> str |
web.WerkzeugResponse:
- return await _delete_committee_keys(session)
-
-
[email protected]("/delete-committee-keys")
-async def delete_committee_keys_post(session: web.Committer) -> str |
web.WerkzeugResponse:
- return await _delete_committee_keys(session)
-
-
-async def _delete_committee_keys(session: web.Committer) -> str |
web.WerkzeugResponse:
- form = await DeleteCommitteeKeysForm.create_form()
+ """Display the form to delete committee keys."""
async with db.session() as data:
all_committees = await
data.committee(_public_signing_keys=True).order_by(sql.Committee.name).all()
committees_with_keys = [c for c in all_committees if
c.public_signing_keys]
- form.committee_name.choices = [(c.name, c.display_name) for c in
committees_with_keys]
-
- if await form.validate_on_submit():
- committee_name = form.committee_name.data
- async with db.session() as data:
- committee_query = data.committee(name=committee_name)
- via = sql.validate_instrumented_attribute
- committee_query.query = committee_query.query.options(
-
orm.selectinload(via(sql.Committee.public_signing_keys)).selectinload(
- via(sql.PublicSigningKey.committees)
- )
- )
- committee = await committee_query.get()
- if not committee:
- await quart.flash(f"Committee '{committee_name}' not found.",
"error")
- return await session.redirect(delete_committee_keys_get)
+ committee_choices = [(c.name, c.display_name) for c in
committees_with_keys]
- keys_to_check = list(committee.public_signing_keys)
- if not keys_to_check:
- await quart.flash(f"Committee '{committee_name}' has no
keys.", "info")
- return await session.redirect(delete_committee_keys_get)
+ rendered_form = form.render(
+ model_cls=DeleteCommitteeKeysForm,
+ submit_label="Delete all keys for selected committee",
+ defaults={"committee_name": committee_choices},
+ )
+ return await template.render("delete-committee-keys.html",
form=rendered_form)
- num_removed = len(committee.public_signing_keys)
- committee.public_signing_keys.clear()
- await data.flush()
- unused_deleted = 0
- for key_obj in keys_to_check:
- if not key_obj.committees:
- await data.delete(key_obj)
- unused_deleted += 1
[email protected]("/delete-committee-keys")
[email protected](DeleteCommitteeKeysForm)
+async def delete_committee_keys_post(
+ session: web.Committer, delete_form: DeleteCommitteeKeysForm
+) -> str | web.WerkzeugResponse:
+ """Delete all keys for selected committee."""
+ committee_name = delete_form.committee_name
- await data.commit()
- await quart.flash(
- f"Removed {num_removed} key links for '{committee_name}'.
Deleted {unused_deleted} unused keys.",
- "success",
- )
- return await session.redirect(delete_committee_keys_get)
+ async with db.session() as data:
+ committee_query = data.committee(name=committee_name)
+ via = sql.validate_instrumented_attribute
+ committee_query.query = committee_query.query.options(
+
orm.selectinload(via(sql.Committee.public_signing_keys)).selectinload(via(sql.PublicSigningKey.committees))
+ )
+ committee = await committee_query.get()
- elif quart.request.method == "POST":
- await quart.flash("Form validation failed. Select committee and type
DELETE KEYS.", "warning")
+ if not committee:
+ await quart.flash(f"Committee '{committee_name}' not found.",
"error")
+ return await session.redirect(delete_committee_keys_get)
- return await template.render("delete-committee-keys.html", form=form)
+ keys_to_check = list(committee.public_signing_keys)
+ if not keys_to_check:
+ await quart.flash(f"Committee '{committee_name}' has no keys.",
"info")
+ return await session.redirect(delete_committee_keys_get)
+ num_removed = len(committee.public_signing_keys)
+ committee.public_signing_keys.clear()
+ await data.flush()
[email protected]("/delete-release")
-async def delete_release_get(session: web.Committer) -> str |
web.WerkzeugResponse:
- return await _delete_release(session)
+ unused_deleted = 0
+ for key_obj in keys_to_check:
+ if not key_obj.committees:
+ await data.delete(key_obj)
+ unused_deleted += 1
+ await data.commit()
+ await quart.flash(
+ f"Removed {num_removed} key links for '{committee_name}'. Deleted
{unused_deleted} unused keys.",
+ "success",
+ )
[email protected]("/delete-release")
-async def delete_release_post(session: web.Committer) -> str |
web.WerkzeugResponse:
- return await _delete_release(session)
+ return await session.redirect(delete_committee_keys_get)
-async def _delete_release(session: web.Committer) -> str |
web.WerkzeugResponse:
- """Page to delete selected releases and their associated data and files."""
- form = await DeleteReleaseForm.create_form()
[email protected]("/delete-release")
+async def delete_release_get(session: web.Committer) -> str |
web.WerkzeugResponse:
+ """Display the form to delete releases."""
+ async with db.session() as data:
+ releases = await
data.release(_project=True).order_by(sql.Release.name).all()
- if quart.request.method == "POST":
- if await form.validate_on_submit():
- form_data = await quart.request.form
- releases_to_delete = form_data.getlist("releases_to_delete")
+ if releases:
+ releases_widget = htpy.div[
+ [
+ htpy.div(".form-check")[
+ htpy.input(
+ class_="form-check-input",
+ type="checkbox",
+ name="releases_to_delete",
+ value=release.name,
+ id=f"release_{release.name}",
+ ),
+ htpy.label(".form-check-label",
for_=f"release_{release.name}")[
+ htpy.strong[release.name],
+ f" ({release.project.display_name},
{release.phase.value.upper()})",
+ ],
+ ]
+ for release in releases
+ ]
+ ]
+ block = htm.Block(htm.div)
+ block.append(releases_widget)
+ block.div(".form-text.mt-1")["Select one or more releases to delete
permanently."]
+ releases_widget_with_help = block.collect()
+ else:
+ releases_widget_with_help = htpy.p(".text-muted")["No releases found
in the database."]
- if not releases_to_delete:
- await quart.flash("No releases selected for deletion.",
"warning")
- return await session.redirect(delete_release_get)
+ rendered_form = form.render(
+ model_cls=DeleteReleaseForm,
+ submit_label="Delete selected releases permanently",
+ submit_classes="btn-danger",
+ custom={"releases_to_delete": releases_widget_with_help},
+ )
- await _delete_releases(session, releases_to_delete)
+ return await template.render("delete-release.html", form=rendered_form)
- # Redirecting back to the deletion page will refresh the list of
releases too
- return await session.redirect(delete_release_get)
- # It's unlikely that form validation failed due to spurious release
names
- # Therefore we assume that the user forgot to type DELETE to confirm
- await quart.flash("Form validation failed. Please type DELETE to
confirm.", "warning")
- # Fall through to the combined GET and failed form validation handling
below
[email protected]("/delete-release")
[email protected](DeleteReleaseForm)
+async def delete_release_post(session: web.Committer, delete_form:
DeleteReleaseForm) -> str | web.WerkzeugResponse:
+ """Delete selected releases and their associated data and files."""
+ await _delete_releases(session, delete_form.releases_to_delete)
- # For GET request or failed form validation
- async with db.session() as data:
- releases = await
data.release(_project=True).order_by(sql.Release.name).all()
- return await template.render("delete-release.html", form=form,
releases=releases, stats=None)
+ return await session.redirect(delete_release_get)
@admin.get("/env")
@@ -454,7 +478,7 @@ async def keys_check_post(session: web.Committer) ->
web.QuartResponse:
@admin.get("/keys/regenerate-all")
async def keys_regenerate_all_get(session: web.Committer) -> web.QuartResponse:
- """Display form to regenerate KEYS files."""
+ """Display the form to regenerate KEYS files."""
rendered_form = form.render(
model_cls=form.Empty,
submit_label="Regenerate all KEYS files",
@@ -865,11 +889,12 @@ async def _delete_releases(session: web.Committer,
releases_to_delete: list[str]
fail_count += 1
error_messages.append(f"{release_name}: Unexpected error ({e})")
+ releases = "release" if (success_count == 1) else "releases"
if success_count > 0:
- await quart.flash(f"Successfully deleted {success_count} release(s).",
"success")
+ await quart.flash(f"Successfully deleted {success_count} {releases}.",
"success")
if fail_count > 0:
errors_str = "\n".join(error_messages)
- await quart.flash(f"Failed to delete {fail_count}
release(s):\n{errors_str}", "error")
+ await quart.flash(f"Failed to delete {fail_count}
{releases}:\n{errors_str}", "error")
async def _get_filesystem_dirs() -> list[str]:
diff --git a/atr/admin/templates/delete-release.html
b/atr/admin/templates/delete-release.html
index b732642..2e721a8 100644
--- a/atr/admin/templates/delete-release.html
+++ b/atr/admin/templates/delete-release.html
@@ -15,40 +15,6 @@
<strong>Warning:</strong> This action is irreversible. Deleting a release
will permanently remove its database records, including tasks and check
results, and its associated files from the filesystem.
</div>
- <form method="post" novalidate>
- {{ form.hidden_tag() }}
-
- <div class="mb-3">
- <label class="form-label">Select releases to delete:</label>
- {% if releases %}
- <div class="list-group overflow-y-auto border rounded">
- {% for release in releases %}
- <label class="list-group-item list-group-item-action d-flex gap-3">
- <input class="form-check-input flex-shrink-0"
- type="checkbox"
- name="releases_to_delete"
- value="{{ release.name }}" />
- <span>
- <strong>{{ release.name }}</strong> ({{
release.project.display_name }}, Phase: {{ release.phase.value.upper() }})
- </span>
- </label>
- {% endfor %}
- </div>
- <div class="form-text">Select one or more releases to delete
permanently.</div>
- {% else %}
- <p class="text-muted">No releases found in the database.</p>
- {% endif %}
- </div>
-
- <div class="mb-3">
- {{ forms.label(form.confirm_delete) }}
- {{ forms.widget(form.confirm_delete) }}
- {{ forms.errors(form.confirm_delete) }}
- {{ forms.description(form.confirm_delete) }}
- </div>
-
- {{ form.submit(class="btn btn-danger") }}
-
- </form>
+ {{ form }}
{% endblock content %}
diff --git a/atr/blueprints/admin.py b/atr/blueprints/admin.py
index 88eeeab..c3318d3 100644
--- a/atr/blueprints/admin.py
+++ b/atr/blueprints/admin.py
@@ -48,14 +48,14 @@ async def _check_admin_access() -> None:
def empty() -> Callable[[Callable[..., Awaitable[Any]]], Callable[...,
Awaitable[Any]]]:
def decorator(func: Callable[..., Awaitable[Any]]) -> Callable[...,
Awaitable[Any]]:
async def wrapper(session: web.Committer, *args: Any, **kwargs: Any)
-> Any:
- form_data = await quart.request.form
+ form_data = await atr.form.quart_request()
try:
context = {
"args": args,
"kwargs": kwargs,
"session": session,
}
- atr.form.validate(atr.form.Empty, dict(form_data),
context=context)
+ atr.form.validate(atr.form.Empty, form_data, context=context)
return await func(session, *args, **kwargs)
except pydantic.ValidationError:
msg = "Sorry, there was an empty form validation error. Please
try again."
@@ -76,20 +76,20 @@ def form(
) -> Callable[[Callable[..., Awaitable[Any]]], Callable[..., Awaitable[Any]]]:
def decorator(func: Callable[..., Awaitable[Any]]) -> Callable[...,
Awaitable[Any]]:
async def wrapper(session: web.Committer, *args: Any, **kwargs: Any)
-> Any:
- form_data = await quart.request.form
+ form_data = await atr.form.quart_request()
try:
context = {
"args": args,
"kwargs": kwargs,
"session": session,
}
- validated_form = atr.form.validate(form_cls, dict(form_data),
context=context)
+ validated_form = atr.form.validate(form_cls, form_data,
context=context)
return await func(session, validated_form, *args, **kwargs)
except pydantic.ValidationError as e:
errors = e.errors()
if len(errors) == 0:
raise RuntimeError("Validation failed, but no errors were
reported")
- flash_data = atr.form.flash_error_data(form_cls, errors,
dict(form_data))
+ flash_data = atr.form.flash_error_data(form_cls, errors,
form_data)
summary = atr.form.flash_error_summary(errors, flash_data)
await quart.flash(summary, category="error")
diff --git a/atr/post/finish.py b/atr/post/finish.py
index 03c5ca6..7df9a76 100644
--- a/atr/post/finish.py
+++ b/atr/post/finish.py
@@ -124,15 +124,16 @@ async def _remove_rc_tags(
if creation_error is not None:
return await respond(409, creation_error)
+ items = "item" if (renamed_count == 1) else "items"
if error_messages:
status_ok = renamed_count > 0
# TODO: Ideally HTTP would have a general mixed status, like 207
but for anything
http_status = 200 if status_ok else 500
- msg = f"RC tags removed for {renamed_count} item(s) with some
errors: {'; '.join(error_messages)}"
+ msg = f"RC tags removed for {renamed_count} {items} with some
errors: {'; '.join(error_messages)}"
return await respond(http_status, msg)
if renamed_count > 0:
- return await respond(200, f"Successfully removed RC tags from
{renamed_count} item(s).")
+ return await respond(200, f"Successfully removed RC tags from
{renamed_count} {items}.")
return await respond(200, "No items required RC tag removal or no
changes were made.")
diff --git a/atr/templates/delete-committee-keys.html
b/atr/templates/delete-committee-keys.html
index 61e7611..2114c86 100644
--- a/atr/templates/delete-committee-keys.html
+++ b/atr/templates/delete-committee-keys.html
@@ -10,27 +10,7 @@
<div class="container mx-auto p-4">
<h1 class="mb-4">Delete all keys for a committee</h1>
- <form method="post"
- action="{{ as_url(admin.delete_committee_keys_post) }}"
- class="atr-canary py-4 px-5 border rounded">
- {{ form.csrf_token }}
-
- <div class="mb-3">
- {{ forms.label(form.committee_name) }}
- {{ forms.widget(form.committee_name, classes="form-select",
id=form.committee_name.id) }}
- {{ forms.errors(form.committee_name) }}
- {{ forms.description(form.committee_name) }}
- </div>
-
- <div class="mb-3">
- {{ forms.label(form.confirm_delete) }}
- {{ forms.widget(form.confirm_delete, classes="form-control",
id=form.confirm_delete.id) }}
- {{ forms.errors(form.confirm_delete) }}
- {{ forms.description(form.confirm_delete) }}
- </div>
-
- <div class="mt-4">{{ form.submit(class_="btn btn-danger") }}</div>
- </form>
+ {{ form }}
</div>
{% endblock content %}
diff --git a/playwright/test.py b/playwright/test.py
index a7bd112..df51509 100755
--- a/playwright/test.py
+++ b/playwright/test.py
@@ -226,7 +226,7 @@ def lifecycle_05_resolve_vote(page: sync_api.Page,
credentials: Credentials, ver
tabulate_form_locator =
page.locator(f'form[action="/resolve/{TEST_PROJECT}/{version_name}"]')
sync_api.expect(tabulate_form_locator).to_be_visible()
- tabulate_button_locator =
tabulate_form_locator.locator('button[type="submit"]:has-text("Resolve vote")')
+ tabulate_button_locator = tabulate_form_locator.get_by_role("button",
name="Resolve vote")
sync_api.expect(tabulate_button_locator).to_be_enabled()
logging.info("Clicking 'Tabulate votes' button")
tabulate_button_locator.click()
@@ -406,7 +406,7 @@ def release_remove(page: sync_api.Page, release_name: str)
-> None:
page.locator("#confirm_delete").fill("DELETE")
logging.info(f"Submitting deletion form for {release_name}")
- submit_button_locator =
page.locator('input[type="submit"][value="Delete selected releases
permanently"]')
+ submit_button_locator = page.get_by_role("button", name="Delete
selected releases permanently")
sync_api.expect(submit_button_locator).to_be_enabled()
submit_button_locator.click()
@@ -417,7 +417,7 @@ def release_remove(page: sync_api.Page, release_name: str)
-> None:
logging.info(f"Checking for success flash message for {release_name}")
flash_message_locator = page.locator("div.flash-success")
sync_api.expect(flash_message_locator).to_be_visible()
- sync_api.expect(flash_message_locator).to_contain_text("Successfully
deleted 1 release(s)")
+ sync_api.expect(flash_message_locator).to_contain_text("Successfully
deleted 1 release")
logging.info(f"Deletion successful for {release_name}")
else:
logging.info(f"Could not find the {release_name} release, no deletion
needed")
@@ -882,7 +882,7 @@ def test_login(page: sync_api.Page, credentials:
Credentials) -> None:
page.locator('input[name="password"]').fill(credentials.password)
logging.info("Submitting the login form")
- submit_button_locator =
page.locator('input[type="submit"][value="Authenticate"]')
+ submit_button_locator = page.get_by_role("button", name="Authenticate")
sync_api.expect(submit_button_locator).to_be_enabled()
submit_button_locator.click()
@@ -977,7 +977,7 @@ def test_projects_03_add_project(page: sync_api.Page,
credentials: Credentials)
page.locator('input[name="label"]').fill(project_label)
logging.info("Submitting the add derived project form")
- submit_button_locator = page.locator('input[type="submit"][value="Add
project"]')
+ submit_button_locator = page.get_by_role("button", name="Add project")
sync_api.expect(submit_button_locator).to_be_enabled()
submit_button_locator.click()
@@ -1145,7 +1145,7 @@ def test_tidy_up_openpgp_keys(page: sync_api.Page) ->
None:
# Navigate to the delete route and submit the form
go_to_path(page, "/admin/delete-test-openpgp-keys")
- delete_button = page.locator('button[type="submit"]')
+ delete_button = page.get_by_role("button", name="Delete all OpenPGP keys
for test user")
if delete_button.is_visible():
delete_button.click()
page.wait_for_load_state()
@@ -1200,9 +1200,7 @@ def test_tidy_up_openpgp_keys_continued(page:
sync_api.Page, fingerprints_to_del
logging.info(f"Locating delete form for fingerprint: {fingerprint}")
# Locate again by fingerprint for robustness
row_to_delete_locator =
page.locator(f'tr:has(a[href="/keys/details/{fingerprint}"])')
- delete_button_locator = row_to_delete_locator.locator(
- 'form[action="/keys"] input[type="submit"][value="Delete key"]'
- )
+ delete_button_locator = row_to_delete_locator.get_by_role("button",
name="Delete key")
if delete_button_locator.is_visible():
logging.info(f"Delete button found for {fingerprint}, proceeding
with deletion")
diff --git a/scripts/docs_check.py b/scripts/docs_check.py
index 3c9b906..a289302 100755
--- a/scripts/docs_check.py
+++ b/scripts/docs_check.py
@@ -191,7 +191,8 @@ def main() -> None:
print("Documentation link validation errors:\n", file=sys.stderr)
for error in errors:
print(error, file=sys.stderr)
- print(f"\nFound {len(errors)} error(s)", file=sys.stderr)
+ suffix = "s" if (len(errors) > 1) else ""
+ print(f"\nFound {len(errors)} error{suffix}", file=sys.stderr)
sys.exit(1)
print(f"Validated {len(all_links)} links across
{len(list(docs_dir.glob('*.md')))} files")
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
