encode / apistar

The Web API toolkit. 🛠
https://docs.apistar.com
BSD 3-Clause "New" or "Revised" License
5.57k stars 411 forks source link

required params in doc #537

Open jgirardet opened 6 years ago

jgirardet commented 6 years ago

In docs, path_parameter have the "required" mark but the required validators from Type should also have this mark.

tomchristie commented 6 years ago

Could you include an example, along with a screenshot?

jgirardet commented 6 years ago

image

class ActeCreateSchema(types.Type):
    patient = validators.Integer()
    motif = validators.String()
    body = validators.String(default="")

this idea is to have "patient" and "motif" in add look like "acte_id" in one because finally all those things are required.

rhelms commented 6 years ago

The template seems to have the code looking for the required property for fields in the body, but the disconnect seems to happen when inspecting the fields in a type. A Type has a required property, but the validators that are used to represent the fields on a Type in an Object validator only understands allow_null.

If the Validator had a @property to answer to required, then I'm almost certain the template would pick it up.

i.e.

@property
def required(self):
    return not self.allow_null

Though usage of a default could complicate that a bit, since if the validator has a default, then you don't necessarily need to supply a value. You may also need to rely on the description to communicate what that default is.

@property
def required(self):
    return not self.allow_null or not self.has_default()

My brain is too fuzzy this early in the morning to be sure that logic is correct.

cchartr commented 6 years ago

@rhelms Here's what I understand:

The Object validator already has a required property, which represents a list of required fields: https://github.com/encode/apistar/blob/master/apistar/validators.py#L329 https://github.com/encode/apistar/blob/master/apistar/validators.py#L377

For Types, it seems this list is based upon the return value of has_default: https://github.com/encode/apistar/blob/master/apistar/types.py#L40

It seems allow_null also has an effect on this: https://github.com/encode/apistar/blob/master/apistar/validators.py#L38

But to me this is not where the problem lies. The problem is in the template which looks for a required property in expanded: https://github.com/encode/apistar/blob/master/apistar/templates/docs/layout/link.html#L61

But according to the line above, it seems expanded is a list with no required property.

cchartr commented 6 years ago

I have suggested a correction in PR #582 .