add enum support to custom forms

[sinisaos]

An alternative approach to #434 . It uses Piccolo choices without complex parsing definitions in the Vue frontend.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.24%. Comparing base (1157c12) to head (3cb1f9a).
⚠️ Report is 47 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #436      +/-   ##
==========================================
+ Coverage   93.42%   94.24%   +0.81%     
==========================================
  Files           5        6       +1     
  Lines         365      417      +52     
==========================================
+ Hits          341      393      +52     
  Misses         24       24              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[sinisaos]

@Skelmis I can't add you as a reviewer, but I'm interested in your thoughts on this approach of using Python enums in custom forms. Thanks in advance.

[Skelmis]

It looks like a much simpler implementation which I love. I'll try test it out within a week

[sinisaos]

@Skelmis Great. Thanks

[Skelmis]

Tested and it works as is, my only issue is that using the example we are accessing an untyped field.

I.e.
data: NewStaffModel yet we need data.permissions which doesnt exist

Users could instead do something like this:

class NewStaffFormModel(BaseModel):
    username: str
    email: EmailStr
    superuser: bool

class NewStaffModel(NewStaffFormModel):
    permissions: Permission

def new_staff_endpoint(request: Request, data: NewStaffModel) -> str:
    print(type(data.permissions))
    return "A new staff member has been successfully created."


FORM = FormConfig(
    name="Enum form",
    pydantic_model=NewStaffFormModel,
    endpoint=new_staff_endpoint,
    description="Make a enum form.",
    choices={"permissions": Permission},
    form_group="Text forms",
)

But even in that case type(data.permissions) is str instead of Permissions.

Do you think it'd be possible to try return Enum items as the relevant enum in this case? Or should users do the enum conversion themselves

[sinisaos]

@Skelmis Thank you for your time to try this out.

Do you think it'd be possible to try return Enum items as the relevant enum in this case? Or should users do the enum conversion themselves

Yes, you are right. I think the easiest way, as you say, is for users to do the enum conversion themselves. Something like this

def new_staff_endpoint(request: Request, data: NewStaffModel) -> str:
    # data.permissions = Permission(int(data.permissions)) # for int enum
    data.permissions = Permission(data.permissions) # for str enum
    print(type(data.permissions)) # prints correct type <enum 'Permission'>
    return "A new staff member has been successfully created."

[Skelmis]

Seems reasonable for me. Thoughts @dantownsend ?

[Skelmis]

Hrmm perhaps just realised we should likely add docs haha. Will see if I find time sometime to try write some up

[sinisaos]

@Skelmis Yes, you are right, but I don't see the point of writing docs because PR is not reviewed or accepted. We can add a usage example to the docs later, if PR is accepted.

[Skelmis]

Fair enough

[Skelmis]

@dantownsend what's your view on this pr? Docs before pr, or pr then docs on approved content?

Either way keen to get this moving, would love this feature

[github-actions]

This PR has been marked as stale because it has been open for 30 days with no activity. Are there any blockers, or should this be closed?

[Skelmis]

@dantownsend what's your view on this pr? Docs before pr, or pr then docs on approved content?

Either way keen to get this moving, would love this feature

@dantownsend waiting on your response to move this PR forward

[dantownsend]

Sorry for the delay on this. I was torn about which approach to take:

1. Should we make our front end understand the refs in the OpenAPI schema.
2. Should we add the extra attributes (like this in this PR).

There was also some discussion about not using OpenAPI schemas any more to drive the front end (because Litestar doesn't allow the extra attributes).

This PR might be the best approach for now though.

[dantownsend]

Rather than passing the choices in, can't we just look at the enum value on the pydantic model itself (i.e. if the default value for the field is an enum then use it to generate the choices).

[sinisaos]

Do you think something like this?

Code changes

def __init__(
      self,
      name: str,
      pydantic_model: t.Type[PydanticModel],
      endpoint: t.Callable[
          [Request, PydanticModel],
          t.Union[FormResponse, t.Coroutine[None, None, FormResponse]],
      ],
      description: t.Optional[str] = None,
      form_group: t.Optional[str] = None,
  ):
      self.name = name
      self.pydantic_model = pydantic_model
      self.endpoint = endpoint
      self.description = description
      self.form_group = form_group
      self.slug = self.name.replace(" ", "-").lower()
      for (
          field_name,
          field_value,
      ) in self.pydantic_model.model_fields.items():
          if isinstance(field_value.annotation, enum.EnumType):
              # update model_fields, field annotation and
              # rebuild the model for the changes to take effect
              pydantic_model.model_fields[field_name] = Field(
                  json_schema_extra={
                      "extra": {
                          "choices": convert_enum_to_choices(
                              field_value.annotation
                          )
                      }
                  },
              )
              pydantic_model.model_fields[field_name].annotation = str
              pydantic_model.model_rebuild(force=True)

Usage would be

Usage example

# An example of using Python enum in custom forms
class Permission(str, enum.Enum):
    admissions = "admissions"
    gallery = "gallery"
    notices = "notices"
    uploads = "uploads"


class NewStaffModel(BaseModel):
    username: str
    email: EmailStr
    superuser: bool
    permissions: Permission


def new_staff_endpoint(request: Request, data: NewStaffModel) -> str:
    # We need to do the enum type conversion ourselves like this:
    # data.permissions = Permission(int(data.permissions)) # for int enum
    # data.permissions = Permission(data.permissions)  # for str enum
    print(data)
    return "A new staff member has been successfully created."


FORM = FormConfig(
    name="Enum form",
    pydantic_model=NewStaffModel,
    endpoint=new_staff_endpoint,
    description="Make a enum form.",
    form_group="Text forms",
)

[dantownsend]

Here can you use issubclass(field_value.annotation, enum.Enum) instead?

[sinisaos]

Note that the field value annotation is of type EnumType (before Python 3.11 it was EnumMeta), not of type enum.Enum and we need to compare the object with ENUMTYPE not enum.Enum. It's practically the same to use issubclass(type(field_value.annotation), ENUMTYPE) or if isinstance(field_value.annotation, ENUMTYPE), but I find the isinstance option a bit cleaner.

[dantownsend]

OK, I see what you're saying.

I think my preference is still this:

inspect.isclass(field_value.annotation) and issubclass(field_value.annotation, enum.Enum)

I find it a bit more intuitive, but now I understand yours I can see that it works fine.

[sinisaos]

Yes, it works. Great.

[dantownsend]

Can't we just leave the annotation as what it was originally?

[sinisaos]

We need to reassign the annotation type because if we don't, that annotation is None and that causes a form validation error. I'm using the string type because if we reassign annotation to the Enum, frontend don't work (working with strings because choices display_value and value are strings, and choices does not work with Enum). In the whole PR I tried to use Piccolo choices so that there would be as few changes as possible on the frontend. If you don't like the whole concept, feel free to delete this PR.

[dantownsend]

Ah, yes - I forgot that the front end would get confused by the open api schema is we leave the enum in there.

[dantownsend]

Thanks for fixing these - I'm going to pull it out into a separate PR.

[github-actions]

This PR has been marked as stale because it has been open for 30 days with no activity. Are there any blockers, or should this be closed?

[github-actions]

This PR has been marked as stale because it has been open for 30 days with no activity. Are there any blockers, or should this be closed?

[Skelmis]

@dantownsend do you have any updates regarding this PR? Is it blocked as a result of technical basis or more so just personal time constraints

[github-actions]

This PR has been marked as stale because it has been open for 30 days with no activity. Are there any blockers, or should this be closed?