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

Feature request: Conditionally required fields #81

Open
bertramakers opened this issue Aug 25, 2023 · 2 comments
Open

Feature request: Conditionally required fields #81

bertramakers opened this issue Aug 25, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@bertramakers
Copy link
Contributor

Hi,

I'm not sure if it's feasible, but it would be helpful to make fields required but only in certain scenario's.
For example:

  1. Field A is optional, but required if field B is also present (or multiple other fields are present). This translates to dependentRequired in JSON schema, so can also easily be included in the auto-generated OpenAPI docs. This name might also be a good candidate for the method on the fields to set it, e.g. Str::make('a')->depentRequired(['b', 'c']) to mark it as required only if fields b and c are also present.
  2. Field A is required, except when field B is present (and vice-versa). This is harder to translate to JSON schema / OpenAPI sadly. Theoretically it should be done like this, but it will probably make the schema generation a lot harder:
{
  "type": "object",
  "properties": {
    "a": {"type": "string"},
    "b": {"type": "string"},
    "...": { ... }
  },
  "oneOf": [
    {"required": ["a"]},
    {"required": ["b"]}
  ],
  "required": [ ... any other, unrelated required fields ... ]
}

It's maybe feasible for 1 pair of fields like this, but it will become more complex if there are multiple. Then you'd need something like this I think:

{
  "type": "object",
  "properties": {
    "a": {"type": "string"},
    "b": {"type": "string"},
    "c": {"type": "string"},
    "d": {"type": "string"},
    "...": { ... }
  },
  "allOf": [
    {
        "oneOf": [
          {"required": ["a"]},
          {"required": ["b"]}
        ]
    },
    {
        "oneOf": [
          {"required": ["c"]},
          {"required": ["d"]}
        ]
    }
  ],
  "required": [ ... any other, unrelated required fields ... ]
}

(You could of course always generate it like this with an allOf wrapper, even for 1 oneOf pair.)

It's this latter functionality that I'm mostly interested in at this point, but sadly it's also the less obvious one to implement. Maybe I can also work around it for our use case by using a custom action/endpoint like you mentioned in #36

More concretely, our use case is that we want to allow a resource to be created from a template. So we'd have templates with an id each, and allow users/clients to create a new resource (of a specific type called "actions") with a template id. If they instead want to create the resource from scratch, they have to provide one or more required fields.

For example, either:

POST /api/actions
Content-type: "application/vnd.api+json"

{
  "data": {
    "relationships": {
      "template": { // Required (on create) if no attributes.name given
        "type": "actions-templates",
        "id": "..."
      }
    }
  }
}

OR

POST /api/actions
Content-type: "application/vnd.api+json"

{
  "data": {
    "attributes": {
      "name": "..." // Required (on create) if no relationships.template given
    }
  }
}

Maybe you also come up with a different solution for this?

The dependentRequired() on the other hand is not something that we need right now, but seems something that is easy enough to implement and that could be useful in the future.

@tobyzerner
Copy link
Owner

Hmm potentially would consider supporting this stuff in the future, but for now I wonder if one of these would support your use-case:

  1. You can implement custom multi-field validation in your resource's create or update method, after values have been set on the model:
public function create(object $model, Context $context): object
{
    if ($model->template && $model->name) {
        throw new UnprocessableEntityException([
            'source' => ['pointer' => '/data'],
            'detail' => 'You can only specify one of template or name',
        ]);
    }

    // persist the model...

    return $model;
}
  1. You could manually read the request body in a field validators to check if the other field is present:
Field\Str::make('name')
    ->validate(function ($value, $fail, $context) {
        if ($value && $context->body()['data']['relationships']['template']['data'] ?? null) {
            $fail('You can only specify one of template or name');
        }
    })
  1. I could potentially refactor the create/update endpoints to run validation and set values to the model simultaneously, rather than running all validation and then setting all values. This would mean that the model could be accessed during validation, but only filled with values for previous fields. Then your validator could look something like this:
Field\ToOne::make('template'),
Field\Str::make('name')
    ->validate(function ($value, $fail, $context) {
        if ($value && $context->model->template) {
            $fail('You can only specify one of template or name');
        }
    })

I think the first solution is probably best for now. As I said, will consider building the schema-compatible dependent stuff in, but probably only for v1.1 or later.

@tobyzerner tobyzerner added the enhancement New feature or request label Aug 26, 2023
@bertramakers
Copy link
Contributor Author

Thanks @tobyzerner , I'll go with either solution 1 or 2 for now then :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants