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

extra_validators change breaks many many forms #547

Closed
jwag956 opened this issue Jan 17, 2023 · 13 comments · Fixed by #548
Closed

extra_validators change breaks many many forms #547

jwag956 opened this issue Jan 17, 2023 · 13 comments · Fixed by #548

Comments

@jwag956
Copy link
Contributor

jwag956 commented Jan 17, 2023

In 1.1.0 the 'extra_validators' parameter was added as a positional parameter to the form.validate() call, whereas in WTForms and Flask-WTForms.validate_on_submit() it is a keyword parameter.

To reproduce:
Create a form with a validate() method which takes no parameters.

Desired Outcome:
FlaskForms.validate() shouldn't require any positional parameters (and therefore be backwards compatible with prior releases).

Environment:

  • Python version:3.9
  • Flask-WTF version:1.1.0
  • Flask version:2.2.2
@jwag956
Copy link
Contributor Author

jwag956 commented Jan 17, 2023

Hmm - this is certainly going to break a lot of things, and it won't be easy to have extensions/apps that work with both 1.0.1 and 1.1.0 - but I see the utility in the change and not sure a better way to do this....

@jwag956 jwag956 closed this as completed Jan 17, 2023
@jwag956
Copy link
Contributor Author

jwag956 commented Jan 17, 2023

Sorry - confusing myself - yes this is an issue with I believe a very simple fix:

return self.is_submitted() and self.validate(extra_validators)

should be:
return self.is_submitted() and self.validate(extra_validators=extra_validators)

@jwag956 jwag956 reopened this Jan 17, 2023
@HwangTaehyun
Copy link
Contributor

HwangTaehyun commented Jan 17, 2023

@jwag956 Yeah, this is also a problem in pgadmin with the following error when user clicks login button. (pgadmin-org/pgadmin4@e0b670f)
loginForm.validate() takes 1 positional argument but 2 were given

When I reflect on your code, then it works! (w/ @chlrlrhs95)
Can I open PR to that?
I would like to fix that asap.

@azmeuk
Copy link
Member

azmeuk commented Jan 17, 2023

Can I open PR to that?

Sure! Go on and I will try to release 1.1.1 tonight.

HwangTaehyun added a commit to HwangTaehyun/flask-wtf that referenced this issue Jan 17, 2023
Co-authored-by: jwag956 <jwag.wagner@gmail.com>
Co-authored-by: chlrlrhs95 <chlrlrhs95@naver.com>
HwangTaehyun added a commit to HwangTaehyun/flask-wtf that referenced this issue Jan 17, 2023
Co-authored-by: jwag956 <jwag.wagner@gmail.com>
Co-authored-by: chlrlrhs95 <chlrlrhs95@naver.com>
HwangTaehyun added a commit to HwangTaehyun/flask-wtf that referenced this issue Jan 17, 2023
Co-authored-by: jwag956 <jwag.wagner@gmail.com>
Co-authored-by: chlrlrhs95 <chlrlrhs95@naver.com>
@HwangTaehyun
Copy link
Contributor

Oh, sorry... I re-test but it does not fix the error.

@jwag956
Copy link
Contributor Author

jwag956 commented Jan 17, 2023

That simple fix worked for me (i.e. Flask-Security-Too unit tests now pass).

@HwangTaehyun
Copy link
Contributor

HwangTaehyun commented Jan 17, 2023

@jwag956 Could you give me some advice?
I changed that code on flask wtf 1.1.0 but it throws error like the following.

{"success":0,"errormsg":"LoginForm.validate() got an unexpected keyword argument 'extra_validators'","info":"","result":null,"data":null}

Why does keyword passing not working in my case (for pgadmin4==6.19 with fixed flask-wtf 1.1.0 which was fixed with this change)

wtforms receive exactly same name: extra_validators..
https://github.com/wtforms/wtforms/blob/6b570a55a2d9959804390bf0e74df2a96cf4c9a0/src/wtforms/form.py#:~:text=%3Dfield_extra_filters)-,def%20validate(self%2C%20extra_validators%3DNone)%3A,-%22%22%22

@jwag956
Copy link
Contributor Author

jwag956 commented Jan 17, 2023

They added 'extra_validators' in 1.1.0 - to validate_on_submit - but didn't pass that as a keyword param to validate
So this fix is to 1.1.0 only - 1.0.1 doesn't have this issue (and should work just fine)

@HwangTaehyun
Copy link
Contributor

HwangTaehyun commented Jan 17, 2023

I fixed my comment a few minutes ago. I changed that code on flask wtf 1.1.0!

@andrei-at-emtelligent
Copy link

Why does keyword passing not working in my case (for pgadmin4==6.19 with fixed flask-wtf 1.1.0 which was fixed with this change)

It's likely you need to add update your LoginForm.validate() method to also accept an extra_validators parameter. If you're subclassing FlaskForm, then that self.validate() call in FlaskForm.validate_on_submit() will be calling LoginForm.validate() not Form.validate(), so any keyword arguments included in that call will need to be supported by your LoginForm.validate() method.

@jwag956
Copy link
Contributor Author

jwag956 commented Jan 18, 2023

The default validate() method in Flask-Security takes **kwargs - so should work fine. Do you have your own LoginForm? or are you using FS's Loginform?

@HwangTaehyun
Copy link
Contributor

Probably pgadmin uses like that. I will dig it. Thank you!

@HwangTaehyun
Copy link
Contributor

HwangTaehyun commented Jan 18, 2023

I figured it out. pgadmin4 uses LoginForm of Flask-Security. And Flask-wtf's validate_on_submit function calls their derived class Flask-Security 4.1.5 LoginForm's validate function and that does not receive extra_validators and it throws an error. However, now the master branch receives extra_validators. You are right!
https://github.com/Flask-Middleware/flask-security/blob/a2f43ff35527f37839fa8da7af6827a4c5e52e31/flask_security/forms.py#L495-L503

So, if pgadmin uses the next version of flask-security, then it works with flask-wtf 1.1.0.
@jwag956 Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants