Skip to content

Conversation

PawelSuwinski
Copy link
Contributor

My proposition for enum input guesser.

Closes #477

@alanpoulain alanpoulain changed the title enum input guesser feat: enum input guesser Oct 4, 2022
@alanpoulain
Copy link
Member

I've added a commit with my changes, WDYT?

@PawelSuwinski
Copy link
Contributor Author

PawelSuwinski commented Oct 5, 2022

I've added a commit with my changes, WDYT?

I think that now it looks better :). Putting humanizing to api-doc-parser allows to use an inflector, that is already present in deps and not to try to do it manually by some hardcoded regexp especially that it is not trivial thing, there are a lot of unusual cases.

Anyway at the end IMHO parser+guesser should cover humanizing out of the box two kinds of typically used enums: (1) CONSTANT_CASE - it is already done here (and even more thanks to using inflector) by openapi2/3 api-doc-parser and (2) schema.org likes enumerations - usually its camel case (or rather camel caps) prefixed with namespace, for example:

'https://schema.org/AudiobookFormat' => {"Audiobook format":  "https://schema.org/AudiobookFormat" }

So with this approach (humanize in api-doc-parser) second case is for future hydra api-doc-parser implementation.

For now last thing to do I think is to take advantage of humanized enums or transformEnum extra prop in FieldGuesser.

@PawelSuwinski
Copy link
Contributor Author

For now last thing to do I think is to take advantage of humanized enums or transformEnum extra prop in FieldGuesser.

I think that it could be done this way, untested draft:

// FieldGuesser
(...)
  if (field.enum) {
    const renderEnum =
      (props as FunctionFieldProps).render ??
      ((record) =>
        Object.entries(field.enum ?? {}).find(
          ([, v]) => v === props.source,
        )?.[0] ?? (props.source ? record[props.source] : null));

    return (
      <FunctionField {...(props as FunctionFieldProps)} render={renderEnum} />
    );
  }

And similar (primaryText prop) in case of ArrayFiled.

@alanpoulain
Copy link
Member

Anyway at the end IMHO parser+guesser should cover humanizing out of the box two kinds of typically used enums: (1) CONSTANT_CASE - it is already done here (and even more thanks to using inflector) by openapi2/3 api-doc-parser and (2) schema.org likes enumerations - usually its camel case (or rather camel caps) prefixed with namespace, for example:

'https://schema.org/AudiobookFormat' => {"Audiobook format":  "https://schema.org/AudiobookFormat" }

So with this approach (humanize in api-doc-parser) second case is for future hydra api-doc-parser implementation.

Do you have an example where schema.org values need to be used for enums?

Why not for the FieldGuesser 🙂 You should create an EnumField dedicated component though.

@PawelSuwinski
Copy link
Contributor Author

PawelSuwinski commented Oct 5, 2022

Do you have an example where schema.org values need to be used for enums?

API Platform at that moment supports only schema.org Enumeration as enums AFAIK, so to have complete solution (at least on api level) one needs to use schema.org built-in types for a field range (for example https://schema.org/BookFormatType) or use own dictionary (my case) with classes that extends https://schema.org/Enumeration:

https://github.com/api-platform/schema-generator/blob/ab36e6dadc7e316ad10afcc99a2b15e0dbb43552/src/Schema/Model/Class_.php#L23

In that case generator generate Assert\Choice constraint for such field with enums values (full name with the namespace prefix):
https://github.com/api-platform/schema-generator/blob/ab36e6dadc7e316ad10afcc99a2b15e0dbb43552/src/AttributeGenerator/ConstraintAttributeGenerator.php#L74

And at the and as you wrote last it goes as enum in api doc:
#477 (comment)

I am using here my custom decorator to do it and suppress namespace but without it enum list will contain fully qualified values like 'https://schema.org/EBook'.

Why not for the FieldGuesser slightly_smiling_face You should create an EnumField dedicated component though.

FunctionField seems to have all that is needed for job to be done, is dedicated EnumField than really needed?

I meant something like this:

diff --git a/src/FieldGuesser.tsx b/src/FieldGuesser.tsx
index dd5b159..c6b93ea 100644
--- a/src/FieldGuesser.tsx
+++ b/src/FieldGuesser.tsx
@@ -6,6 +6,7 @@ import {
   ChipField,
   DateField,
   EmailField,
+  FunctionField,
   NumberField,
   ReferenceArrayField,
   ReferenceField,
@@ -20,6 +21,7 @@ import type {
   BooleanFieldProps,
   DateFieldProps,
   EmailFieldProps,
+  FunctionFieldProps,
   NumberFieldProps,
   ReferenceArrayFieldProps,
   ReferenceFieldProps,
@@ -89,6 +91,19 @@ const renderField = (
 
   const fieldType = schemaAnalyzer.getFieldType(field);
 
+  if (field.enum) {
+    const renderEnum =
+      (props as FunctionFieldProps).render ??
+      ((record) =>
+        Object.entries(field.enum ?? {}).find(
+          ([, v]) => v === props.source,
+        )?.[0] ?? (props.source ? record[props.source] : null));
+
+    return (
+      <FunctionField {...(props as FunctionFieldProps)} render={renderEnum} />
+    );
+  }
+
   switch (fieldType) {
     case 'email':
       return <EmailField {...(props as EmailFieldProps)} />;


@alanpoulain
Copy link
Member

Alright, I was reading this enum (generated from OpenAPI) and not this one (Hydra).

Still, I am really not sure it's a good idea to have a regexp to handle schema.org or any other RDF vocabulary like this.
I think we can postpone this until we have a fully JSON-LD/Hydra enum support in API Platform.

FunctionField seems to have all that is needed for job to be done, is dedicated EnumField than really needed?

Yes I think it will be better: your renderEnum function is not trivial.

@PawelSuwinski
Copy link
Contributor Author

Still, I am really not sure it's a good idea to have a regexp to handle schema.org or any other RDF vocabulary like this.

I agree. I rather thought about cutting off the name space and just humanize the rest value, something like this:
inflection.humanize(enumValue.replace(namespace,'')).

I think we can postpone this until we have a fully JSON-LD/Hydra enum support in API Platform.

OK, its just future considerations.

FunctionField seems to have all that is needed for job to be done, is dedicated EnumField than really needed?

Yes I think it will be better: your renderEnum function is not trivial.

OK, I will think about it and propose something in the next PR if it is still not solved to not too block this one and delay merging .

@alanpoulain
Copy link
Member

I agree. I rather thought about cutting off the name space and just humanize the rest value, something like this: inflection.humanize(enumValue.replace(namespace,'')).

Yes it would be better!

@alanpoulain alanpoulain merged commit 11e13cf into api-platform:main Oct 6, 2022
@alanpoulain
Copy link
Member

Thank you @PawelSuwinski 🙂

@PawelSuwinski PawelSuwinski deleted the enum_guesser branch October 6, 2022 08:57
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.

Enum support

2 participants