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

required params in doc #537

Open
jgirardet opened this issue May 14, 2018 · 5 comments
Open

required params in doc #537

jgirardet opened this issue May 14, 2018 · 5 comments

Comments

@jgirardet
Copy link
Contributor

@jgirardet jgirardet commented May 14, 2018

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

@tomchristie
Copy link
Member

@tomchristie tomchristie commented May 14, 2018

Could you include an example, along with a screenshot?

@jgirardet
Copy link
Contributor Author

@jgirardet jgirardet commented May 14, 2018

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
Copy link
Contributor

@rhelms rhelms commented May 16, 2018

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
Copy link
Contributor

@cchartr cchartr commented Jun 10, 2018

@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 pushed a commit to cchartr/apistar that referenced this issue Jun 10, 2018
Fix doc template so that required parameters are shown correctly as such for Request Body fields. See encode#537.
@cchartr
Copy link
Contributor

@cchartr cchartr commented Jun 10, 2018

I have suggested a correction in PR #582 .

tomchristie added a commit that referenced this issue Jun 14, 2018
Fix doc template so that required parameters are shown correctly as such for Request Body fields. See #537.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.