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

docs: wtforms/validators: tweak InputRequired docs #788

Merged
merged 2 commits into from May 31, 2023

Conversation

woodruffw
Copy link
Contributor

Hello!

This is just a small docstring change: no behavioral changes have been made. As such, I haven't added tests or a changelog entry.

This change clarifies the behavior of the InputRequired validator by making its behavior around raw vs. coerced data slightly more explicitly documented: it adds an additional sentence ephasizing that "form-input data" means the formdata parameter going into a Form.

As a justification for this clarification: we're doing a bit of refactoring on Warehouse, including replacing usages of DataRequired with InputRequired. As part of that, we've observed that a large number of unit tests incorrectly assumed that Foo(x=1) has the same validation behavior as Foo(dict(x=1)), which it doesn't for fields whose validators look at the field's raw_data.

Please let me know if there's anything else I can do here!

cc @jleightcap

Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
Copy link
Contributor Author

xref pypi/warehouse#13696

@davidism
Copy link
Member

davidism commented May 26, 2023

Data passed in as keyword arguments or with the obj or data parameters is initial populated data, not data that should be validated directly. You populate the initial data, render the template, then validate the data that is sent back. This is true of all validators and all initial data, not the InputRequired validator on keyword arguments.

I think this change is obscuring that distinction. What the sentence is saying is that both validators are only validating sent data, but Input is checking that non-empty data was sent, while Data is checking that non-empty data was coerced from that sent data.

@woodruffw
Copy link
Contributor Author

Sorry for the delay!

I see your point -- my goal was to emphasize the existing behavior, but I don't think I've made it clearer.

How does something like this sound instead?

This means that this validator only checks whether non-empty data was sent, not whether non-empty data was coerced from that data. Initially populated data is not considered sent.

That's still a little clunky, though. Please feel free to close if you think it isn't a net improvement in that form either 🙂

@davidism
Copy link
Member

Yeah, that sounds good.

Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
Copy link
Contributor Author

Cool, thanks. Updated!

@davidism davidism merged commit 0524993 into wtforms:master May 31, 2023
7 of 8 checks passed
@woodruffw woodruffw deleted the ww/tweak-inputrequired-docs branch June 1, 2023 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants