sbp commented on code in PR #343: URL: https://github.com/apache/tooling-trusted-releases/pull/343#discussion_r2557420740
########## atr/docs/user-interface.md: ########## @@ -46,30 +46,104 @@ Template rendering happens in a thread pool to avoid blocking the async event lo ## Forms -HTML forms in ATR are handled by [WTForms](https://wtforms.readthedocs.io/), accessed through our [`forms`](/ref/atr/forms.py) module. Each form is a class that inherits from [`forms.Typed`](/ref/atr/forms.py:Typed), which itself inherits from `QuartForm` in [Quart-WTF](https://quart-wtf.readthedocs.io/). Form fields are class attributes created using helper functions from the `forms` module. - -Here is a typical form definition from [`shared/keys.py`](/ref/atr/shared/keys.py:AddOpenPGPKeyForm): +HTML forms in ATR are handled by [Pydantic](https://docs.pydantic.dev/latest/) models accessed through our [`form`](/ref/atr/form.py) module. Each form is a class that inherits from [`form.Form`](/ref/atr/form.py:Form), which itself inherits from `pydantic.BaseModel`. Form fields are defined as class attributes using Pydantic type annotations, with the [`form.label`](/ref/atr/form.py:label) function providing field metadata like labels and documentation. +Here is a typical form definition from [`shared/keys.py`](/ref/atr/shared/keys.py): ```python -class AddOpenPGPKeyForm(forms.Typed): - public_key = forms.textarea( +class AddOpenPGPKeyForm(form.Form): + public_key: str = form.label( "Public OpenPGP key", - placeholder="Paste your ASCII-armored public OpenPGP key here...", - description="Your public key should be in ASCII-armored format, starting with" - ' "-----BEGIN PGP PUBLIC KEY BLOCK-----"', + 'Your public key should be in ASCII-armored format, starting with "-----BEGIN PGP PUBLIC KEY BLOCK-----"', + widget=form.Widget.TEXTAREA, ) - selected_committees = forms.checkboxes( + selected_committees: form.StrList = form.label( "Associate key with committees", - description="Select the committees with which to associate your key.", + "Select the committees with which to associate your key.", ) - submit = forms.submit("Add OpenPGP key") + + @pydantic.model_validator(mode="after") + def validate_at_least_one_committee(self) -> "AddOpenPGPKeyForm": + if not self.selected_committees: + raise ValueError("You must select at least one committee to associate with this key") + return self ``` -The helper functions like [`forms.textarea`](/ref/atr/forms.py:textarea), [`forms.checkboxes`](/ref/atr/forms.py:checkboxes), and [`forms.submit`](/ref/atr/forms.py:submit) create WTForms field objects with appropriate validators. The first argument is always the label text. Optional fields take `optional=True`, and you can provide placeholders, descriptions, and other field-specific options. If you do not pass `optional=True`, the field is required by default. The [`forms.string`](/ref/atr/forms.py:string) function adds `REQUIRED` to the validators, while [`forms.optional`](/ref/atr/forms.py:optional) adds `OPTIONAL`. +### Field Types and Labels + +The [`form.label`](/ref/atr/form.py:label) function is used to add metadata to Pydantic fields. The first argument is the label text, the second (optional) argument is documentation text that appears below the field, and you can pass additional keyword arguments like `widget=form.Widget.TEXTAREA` to specify the HTML widget type. + +Fields use Pydantic type annotations to define their data type: +- `str` - text input (default widget: `Widget.TEXT`) +- `form.Email` - email input with validation +- `form.URL` - URL input with validation +- `form.Bool` - checkbox +- `form.Int` - number input +- `form.StrList` - multiple checkboxes that collect strings +- `form.File` - single file upload +- `form.FileList` - multiple file upload +- `form.Enum[EnumType]` - dropdown select from enum values +- `form.Set[EnumType]` - multiple checkboxes from enum values -To use a form in a route, create it with `await FormClass.create_form()`. For POST requests, pass `data=await quart.request.form` to populate it with the submitted data. Then validate with `await form.validate_on_submit()`. If validation passes, you extract data from `form.field_name.data` and proceed. If validation fails, re-render the template with the form object, which will then display error messages. +Fields are required by default. To make a field optional, use Python's standard `Optional` or union syntax: `field_name: str | None = None`. Review Comment: It's not true that fields are required by default. In the checkbox cases, we can receive zero values from the browser and Pydantic allows this, because the type wraps a list. Also we allow empty values by default in most cases, which is relevant; URL is an exception, and we ought to decide whether we want to reflect its exceptional position in its name. An aim when designing `form.py` is that we should never have to allow `None`, but there was one case where it would have made more sense to use that, so perhaps we should at least allow it. We [don't](https://release-test.apache.org/docs/code-conventions), however, allow the use of `Optional`. ########## atr/docs/user-interface.md: ########## @@ -46,30 +46,104 @@ Template rendering happens in a thread pool to avoid blocking the async event lo ## Forms -HTML forms in ATR are handled by [WTForms](https://wtforms.readthedocs.io/), accessed through our [`forms`](/ref/atr/forms.py) module. Each form is a class that inherits from [`forms.Typed`](/ref/atr/forms.py:Typed), which itself inherits from `QuartForm` in [Quart-WTF](https://quart-wtf.readthedocs.io/). Form fields are class attributes created using helper functions from the `forms` module. - -Here is a typical form definition from [`shared/keys.py`](/ref/atr/shared/keys.py:AddOpenPGPKeyForm): +HTML forms in ATR are handled by [Pydantic](https://docs.pydantic.dev/latest/) models accessed through our [`form`](/ref/atr/form.py) module. Each form is a class that inherits from [`form.Form`](/ref/atr/form.py:Form), which itself inherits from `pydantic.BaseModel`. Form fields are defined as class attributes using Pydantic type annotations, with the [`form.label`](/ref/atr/form.py:label) function providing field metadata like labels and documentation. +Here is a typical form definition from [`shared/keys.py`](/ref/atr/shared/keys.py): ```python -class AddOpenPGPKeyForm(forms.Typed): - public_key = forms.textarea( +class AddOpenPGPKeyForm(form.Form): + public_key: str = form.label( "Public OpenPGP key", - placeholder="Paste your ASCII-armored public OpenPGP key here...", - description="Your public key should be in ASCII-armored format, starting with" - ' "-----BEGIN PGP PUBLIC KEY BLOCK-----"', + 'Your public key should be in ASCII-armored format, starting with "-----BEGIN PGP PUBLIC KEY BLOCK-----"', + widget=form.Widget.TEXTAREA, ) - selected_committees = forms.checkboxes( + selected_committees: form.StrList = form.label( "Associate key with committees", - description="Select the committees with which to associate your key.", + "Select the committees with which to associate your key.", ) - submit = forms.submit("Add OpenPGP key") + + @pydantic.model_validator(mode="after") + def validate_at_least_one_committee(self) -> "AddOpenPGPKeyForm": + if not self.selected_committees: + raise ValueError("You must select at least one committee to associate with this key") + return self ``` -The helper functions like [`forms.textarea`](/ref/atr/forms.py:textarea), [`forms.checkboxes`](/ref/atr/forms.py:checkboxes), and [`forms.submit`](/ref/atr/forms.py:submit) create WTForms field objects with appropriate validators. The first argument is always the label text. Optional fields take `optional=True`, and you can provide placeholders, descriptions, and other field-specific options. If you do not pass `optional=True`, the field is required by default. The [`forms.string`](/ref/atr/forms.py:string) function adds `REQUIRED` to the validators, while [`forms.optional`](/ref/atr/forms.py:optional) adds `OPTIONAL`. +### Field Types and Labels + +The [`form.label`](/ref/atr/form.py:label) function is used to add metadata to Pydantic fields. The first argument is the label text, the second (optional) argument is documentation text that appears below the field, and you can pass additional keyword arguments like `widget=form.Widget.TEXTAREA` to specify the HTML widget type. + +Fields use Pydantic type annotations to define their data type: +- `str` - text input (default widget: `Widget.TEXT`) +- `form.Email` - email input with validation +- `form.URL` - URL input with validation +- `form.Bool` - checkbox +- `form.Int` - number input +- `form.StrList` - multiple checkboxes that collect strings +- `form.File` - single file upload +- `form.FileList` - multiple file upload +- `form.Enum[EnumType]` - dropdown select from enum values +- `form.Set[EnumType]` - multiple checkboxes from enum values -To use a form in a route, create it with `await FormClass.create_form()`. For POST requests, pass `data=await quart.request.form` to populate it with the submitted data. Then validate with `await form.validate_on_submit()`. If validation passes, you extract data from `form.field_name.data` and proceed. If validation fails, re-render the template with the form object, which will then display error messages. +Fields are required by default. To make a field optional, use Python's standard `Optional` or union syntax: `field_name: str | None = None`. -The [`forms`](/ref/atr/forms.py) module also provides rendering functions that generate Bootstrap-styled HTML. The function [`forms.render_columns`](/ref/atr/forms.py:render_columns) creates a two-column layout with labels on the left and inputs on the right. The function [`forms.render_simple`](/ref/atr/forms.py:render_simple) creates a simpler vertical layout. The function [`forms.render_table`](/ref/atr/forms.py:render_table) puts the form inside a table. All three functions return htpy elements, which you can embed in templates or return directly from route handlers. +The `widget` parameter in [`form.label`](/ref/atr/form.py:label) lets you override the default widget for a field type. Available widgets include: `TEXTAREA`, `CHECKBOXES`, `SELECT`, `RADIO`, `HIDDEN`, and others from the `form.Widget` enum. Review Comment: We should probably mention the cases when you need to do this override. It's fairly limited; the defaults should work in most cases, and the enum values are there for internal tracking. ########## atr/docs/user-interface.md: ########## @@ -115,43 +189,67 @@ The block class also adds a `data-src` attribute to elements, which records whic ## How a route renders UI -A typical route that renders UI first authenticates the user, loads data from the database, creates and validates a form if necessary, and renders a template with the data and form. Here is a simplified example from [`get/keys.py`](/ref/atr/get/keys.py:add): +A typical route that renders UI first authenticates the user, loads data from the database, builds HTML using htpy, and renders it using a template. GET and POST requests are now handled by separate routes, with form validation automatically handled by the [`@post.form()`](/ref/atr/blueprints/post.py:form) decorator. Here is a simplified example from [`get/keys.py`](/ref/atr/get/keys.py:add): Review Comment: On "are now", the documentation should arguably always reflect the existing state of the code, not changes made to the code. (Similarly for inline code comments.) ########## atr/docs/user-interface.md: ########## @@ -46,30 +46,104 @@ Template rendering happens in a thread pool to avoid blocking the async event lo ## Forms -HTML forms in ATR are handled by [WTForms](https://wtforms.readthedocs.io/), accessed through our [`forms`](/ref/atr/forms.py) module. Each form is a class that inherits from [`forms.Typed`](/ref/atr/forms.py:Typed), which itself inherits from `QuartForm` in [Quart-WTF](https://quart-wtf.readthedocs.io/). Form fields are class attributes created using helper functions from the `forms` module. - -Here is a typical form definition from [`shared/keys.py`](/ref/atr/shared/keys.py:AddOpenPGPKeyForm): +HTML forms in ATR are handled by [Pydantic](https://docs.pydantic.dev/latest/) models accessed through our [`form`](/ref/atr/form.py) module. Each form is a class that inherits from [`form.Form`](/ref/atr/form.py:Form), which itself inherits from `pydantic.BaseModel`. Form fields are defined as class attributes using Pydantic type annotations, with the [`form.label`](/ref/atr/form.py:label) function providing field metadata like labels and documentation. +Here is a typical form definition from [`shared/keys.py`](/ref/atr/shared/keys.py): ```python -class AddOpenPGPKeyForm(forms.Typed): - public_key = forms.textarea( +class AddOpenPGPKeyForm(form.Form): + public_key: str = form.label( "Public OpenPGP key", - placeholder="Paste your ASCII-armored public OpenPGP key here...", - description="Your public key should be in ASCII-armored format, starting with" - ' "-----BEGIN PGP PUBLIC KEY BLOCK-----"', + 'Your public key should be in ASCII-armored format, starting with "-----BEGIN PGP PUBLIC KEY BLOCK-----"', + widget=form.Widget.TEXTAREA, ) - selected_committees = forms.checkboxes( + selected_committees: form.StrList = form.label( "Associate key with committees", - description="Select the committees with which to associate your key.", + "Select the committees with which to associate your key.", ) - submit = forms.submit("Add OpenPGP key") + + @pydantic.model_validator(mode="after") + def validate_at_least_one_committee(self) -> "AddOpenPGPKeyForm": + if not self.selected_committees: + raise ValueError("You must select at least one committee to associate with this key") + return self ``` -The helper functions like [`forms.textarea`](/ref/atr/forms.py:textarea), [`forms.checkboxes`](/ref/atr/forms.py:checkboxes), and [`forms.submit`](/ref/atr/forms.py:submit) create WTForms field objects with appropriate validators. The first argument is always the label text. Optional fields take `optional=True`, and you can provide placeholders, descriptions, and other field-specific options. If you do not pass `optional=True`, the field is required by default. The [`forms.string`](/ref/atr/forms.py:string) function adds `REQUIRED` to the validators, while [`forms.optional`](/ref/atr/forms.py:optional) adds `OPTIONAL`. +### Field Types and Labels + +The [`form.label`](/ref/atr/form.py:label) function is used to add metadata to Pydantic fields. The first argument is the label text, the second (optional) argument is documentation text that appears below the field, and you can pass additional keyword arguments like `widget=form.Widget.TEXTAREA` to specify the HTML widget type. + +Fields use Pydantic type annotations to define their data type: +- `str` - text input (default widget: `Widget.TEXT`) +- `form.Email` - email input with validation +- `form.URL` - URL input with validation +- `form.Bool` - checkbox +- `form.Int` - number input +- `form.StrList` - multiple checkboxes that collect strings +- `form.File` - single file upload +- `form.FileList` - multiple file upload +- `form.Enum[EnumType]` - dropdown select from enum values +- `form.Set[EnumType]` - multiple checkboxes from enum values -To use a form in a route, create it with `await FormClass.create_form()`. For POST requests, pass `data=await quart.request.form` to populate it with the submitted data. Then validate with `await form.validate_on_submit()`. If validation passes, you extract data from `form.field_name.data` and proceed. If validation fails, re-render the template with the form object, which will then display error messages. +Fields are required by default. To make a field optional, use Python's standard `Optional` or union syntax: `field_name: str | None = None`. -The [`forms`](/ref/atr/forms.py) module also provides rendering functions that generate Bootstrap-styled HTML. The function [`forms.render_columns`](/ref/atr/forms.py:render_columns) creates a two-column layout with labels on the left and inputs on the right. The function [`forms.render_simple`](/ref/atr/forms.py:render_simple) creates a simpler vertical layout. The function [`forms.render_table`](/ref/atr/forms.py:render_table) puts the form inside a table. All three functions return htpy elements, which you can embed in templates or return directly from route handlers. +The `widget` parameter in [`form.label`](/ref/atr/form.py:label) lets you override the default widget for a field type. Available widgets include: `TEXTAREA`, `CHECKBOXES`, `SELECT`, `RADIO`, `HIDDEN`, and others from the `form.Widget` enum. + +### Using Forms in Routes + +To use a form in a route, validate the form data from a POST request using [`form.validate`](/ref/atr/form.py:validate): Review Comment: The example code below this doesn't use `form.validate`. Also `form.validate` should not be used directly except in certain conditions such as when the request was made by JavaScript. ########## atr/docs/user-interface.md: ########## @@ -46,30 +46,104 @@ Template rendering happens in a thread pool to avoid blocking the async event lo ## Forms -HTML forms in ATR are handled by [WTForms](https://wtforms.readthedocs.io/), accessed through our [`forms`](/ref/atr/forms.py) module. Each form is a class that inherits from [`forms.Typed`](/ref/atr/forms.py:Typed), which itself inherits from `QuartForm` in [Quart-WTF](https://quart-wtf.readthedocs.io/). Form fields are class attributes created using helper functions from the `forms` module. - -Here is a typical form definition from [`shared/keys.py`](/ref/atr/shared/keys.py:AddOpenPGPKeyForm): +HTML forms in ATR are handled by [Pydantic](https://docs.pydantic.dev/latest/) models accessed through our [`form`](/ref/atr/form.py) module. Each form is a class that inherits from [`form.Form`](/ref/atr/form.py:Form), which itself inherits from `pydantic.BaseModel`. Form fields are defined as class attributes using Pydantic type annotations, with the [`form.label`](/ref/atr/form.py:label) function providing field metadata like labels and documentation. +Here is a typical form definition from [`shared/keys.py`](/ref/atr/shared/keys.py): ```python -class AddOpenPGPKeyForm(forms.Typed): - public_key = forms.textarea( +class AddOpenPGPKeyForm(form.Form): + public_key: str = form.label( "Public OpenPGP key", - placeholder="Paste your ASCII-armored public OpenPGP key here...", - description="Your public key should be in ASCII-armored format, starting with" - ' "-----BEGIN PGP PUBLIC KEY BLOCK-----"', + 'Your public key should be in ASCII-armored format, starting with "-----BEGIN PGP PUBLIC KEY BLOCK-----"', + widget=form.Widget.TEXTAREA, ) - selected_committees = forms.checkboxes( + selected_committees: form.StrList = form.label( "Associate key with committees", - description="Select the committees with which to associate your key.", + "Select the committees with which to associate your key.", ) - submit = forms.submit("Add OpenPGP key") + + @pydantic.model_validator(mode="after") + def validate_at_least_one_committee(self) -> "AddOpenPGPKeyForm": + if not self.selected_committees: + raise ValueError("You must select at least one committee to associate with this key") + return self ``` -The helper functions like [`forms.textarea`](/ref/atr/forms.py:textarea), [`forms.checkboxes`](/ref/atr/forms.py:checkboxes), and [`forms.submit`](/ref/atr/forms.py:submit) create WTForms field objects with appropriate validators. The first argument is always the label text. Optional fields take `optional=True`, and you can provide placeholders, descriptions, and other field-specific options. If you do not pass `optional=True`, the field is required by default. The [`forms.string`](/ref/atr/forms.py:string) function adds `REQUIRED` to the validators, while [`forms.optional`](/ref/atr/forms.py:optional) adds `OPTIONAL`. +### Field Types and Labels + +The [`form.label`](/ref/atr/form.py:label) function is used to add metadata to Pydantic fields. The first argument is the label text, the second (optional) argument is documentation text that appears below the field, and you can pass additional keyword arguments like `widget=form.Widget.TEXTAREA` to specify the HTML widget type. + +Fields use Pydantic type annotations to define their data type: +- `str` - text input (default widget: `Widget.TEXT`) +- `form.Email` - email input with validation +- `form.URL` - URL input with validation +- `form.Bool` - checkbox +- `form.Int` - number input +- `form.StrList` - multiple checkboxes that collect strings +- `form.File` - single file upload +- `form.FileList` - multiple file upload +- `form.Enum[EnumType]` - dropdown select from enum values +- `form.Set[EnumType]` - multiple checkboxes from enum values -To use a form in a route, create it with `await FormClass.create_form()`. For POST requests, pass `data=await quart.request.form` to populate it with the submitted data. Then validate with `await form.validate_on_submit()`. If validation passes, you extract data from `form.field_name.data` and proceed. If validation fails, re-render the template with the form object, which will then display error messages. +Fields are required by default. To make a field optional, use Python's standard `Optional` or union syntax: `field_name: str | None = None`. -The [`forms`](/ref/atr/forms.py) module also provides rendering functions that generate Bootstrap-styled HTML. The function [`forms.render_columns`](/ref/atr/forms.py:render_columns) creates a two-column layout with labels on the left and inputs on the right. The function [`forms.render_simple`](/ref/atr/forms.py:render_simple) creates a simpler vertical layout. The function [`forms.render_table`](/ref/atr/forms.py:render_table) puts the form inside a table. All three functions return htpy elements, which you can embed in templates or return directly from route handlers. +The `widget` parameter in [`form.label`](/ref/atr/form.py:label) lets you override the default widget for a field type. Available widgets include: `TEXTAREA`, `CHECKBOXES`, `SELECT`, `RADIO`, `HIDDEN`, and others from the `form.Widget` enum. + +### Using Forms in Routes + +To use a form in a route, validate the form data from a POST request using [`form.validate`](/ref/atr/form.py:validate): +```python [email protected]("/keys/add") [email protected](shared.keys.AddOpenPGPKeyForm) +async def add(session: web.Committer, add_openpgp_key_form: shared.keys.AddOpenPGPKeyForm) -> web.WerkzeugResponse: + """Add a new public signing key to the user's account.""" + try: + key_text = add_openpgp_key_form.public_key + selected_committee_names = add_openpgp_key_form.selected_committees + + # Process the validated form data... + async with storage.write() as write: + # ... + + await quart.flash("OpenPGP key added successfully.", "success") + except web.FlashError as e: + await quart.flash(str(e), "error") + except Exception as e: + log.exception("Error adding OpenPGP key:") + await quart.flash(f"An unexpected error occurred: {e!s}", "error") + + return await session.redirect(get.keys.keys) +``` + +The [`form.validate`](/ref/atr/form.py:validate) function takes the form class, the form data dictionary, and an optional context dictionary. If validation succeeds, it returns an instance of your form class with validated data. If validation fails, it raises a `pydantic.ValidationError`. Review Comment: This is true, but as noted above, `form.validate` is not used in the sample code and should be avoided except in certain circumstances. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
