Skip to content
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

Support Enum types within Forms #434

Closed

Conversation

Skelmis
Copy link
Contributor

@Skelmis Skelmis commented Jan 16, 2025

My attempt at supporting Enum's within Forms.

Note this is my first time working with Vue and as such I'm trying to figure things out and just get them working. I'd love feedback and improvements.

#430

@Skelmis
Copy link
Contributor Author

Skelmis commented Jan 16, 2025

So PoC form:

from enum import Enum

from pydantic import BaseModel
from starlette.requests import Request

from piccolo_admin.endpoints import FormConfig


class Test(Enum):
    ONE = "ONE"
    TWO = "TWO"
    THREE = "THREE"

class CalculatorModel(BaseModel):
    number_1: int
    number_2: int
    enum: Test


def calculator(request: Request, data: CalculatorModel):
    """
    A very simple example of a form which adds numbers together.
    """
    return f"The answer is {data.number_1 + data.number_2} with enum {data.enum}."


FORM = FormConfig(
    name="Calculator",
    pydantic_model=CalculatorModel,
    endpoint=calculator,
    description=("Adds two numbers together."),
)

Produces:
image

@sinisaos
Copy link
Member

@Skelmis There was a PR to enable Python Enums in custom forms. I used Piccolo choices because Piccolo Admin and Piccolo API doesn't understand references through "$ref". Everything worked (video here or video here) and was easy for the user to use, but I'm closing that PR because it was decided that we need to move to a native OpenAPI approach (that would be best). Piccolo Admin and Piccolo API do not currently use complex references and to work natively we need to make changes to both repos

@Skelmis
Copy link
Contributor Author

Skelmis commented Jan 16, 2025

What changes are required within the API? Possibly I've overlooked something but I don't feel like it may require changes

@sinisaos
Copy link
Member

@Skelmis create_pydantic_model does not use definitions and $ref (as in OpenAPI for enums). Here and here are possible ideas of changes that should be made, but you have to wait @dantownsend to see how it should be done.

@Skelmis
Copy link
Contributor Author

Skelmis commented Jan 16, 2025

Interesting, I wonder then how $refs are currently propgrating then given this draft is essentially working. I might take a poke later

@sinisaos
Copy link
Member

Ah my mistake, sorry. I forgot that custom forms using Pydantic BaseModel do not create_pydantic_model (like table forms) and the $ref exists in the json schema. After that you parse the properties (I tried pretty much the same thing in a closed PR, but I used Piccolo choices). Sorry again.

@dantownsend
Copy link
Member

@Skelmis Thanks for taking a look at this, looks like some great progress!

I don't mind us changing create_pydantic_model so it returns choices as $ref. At the moment we add it as an extra attribute to the Pydantic JSON schema:

https://github.com/piccolo-orm/piccolo/blob/965c5b3ef2311a0ecb39a09341d5a6ebabc25b5d/piccolo/utils/pydantic.py#L280C37-L280C53

@Skelmis
Copy link
Contributor Author

Skelmis commented Jan 17, 2025

I actually prefer how it currently works because then you get a display value and the actual value versus $ref which at face value is just the raw value

@Skelmis
Copy link
Contributor Author

Skelmis commented Jan 17, 2025

I'm currently getting the following error, could either of you give me a hand resolving it?

src/components/NewForm.vue:13:37 - error TS2722: Cannot invoke an object which is possibly 'undefined'.

13                     v-bind:choices="etc(schema['$defs'][Number(property['$ref']?.split(/[//]+/).pop())])"
                                       ~~~


Found 1 error in src/components/NewForm.vue:

<div
v-if="Object.keys(property).includes('$ref')"
>
<label>{{ schema["$defs"][Number(property['$ref']?.split(/[//]+/).pop())].title }}</label>

Check warning

Code scanning / CodeQL

Duplicate character in character class Warning

Character '/' is
repeated in the same character class
.
@Skelmis
Copy link
Contributor Author

Skelmis commented Feb 7, 2025

Closing in favor of #436

@Skelmis Skelmis closed this Feb 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants