Skip to content

Commit 95f89b3

Browse files
Prefer InputRequired over DataRequired on form validation (#13696)
* DataRequired -> InputRequired Signed-off-by: Jack Leightcap <[email protected]> * `make translations` Signed-off-by: William Woodruff <[email protected]> * pyproject: exclude webob.* from mypy There's no current tracking issue for supporting type hints in WebOb. Signed-off-by: William Woodruff <[email protected]> * pyproject: tracking issue Signed-off-by: William Woodruff <[email protected]> * tests: mash a type into place Signed-off-by: William Woodruff <[email protected]> --------- Signed-off-by: Jack Leightcap <[email protected]> Signed-off-by: William Woodruff <[email protected]> Co-authored-by: William Woodruff <[email protected]>
1 parent a5d6e11 commit 95f89b3

File tree

16 files changed

+243
-209
lines changed

16 files changed

+243
-209
lines changed

pyproject.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ module = [
4848
"sqlalchemy.*", # https://docs.sqlalchemy.org/en/14/orm/extensions/mypy.html
4949
"transaction.*",
5050
"venusian.*",
51+
"webob.*", # https://github.com/python/typeshed/pull/9874
5152
"whitenoise.*",
5253
"wtforms.*", # https://github.com/wtforms/wtforms/issues/618
5354
"yara.*",

tests/unit/accounts/test_forms.py

Lines changed: 86 additions & 70 deletions
Large diffs are not rendered by default.

tests/unit/admin/views/test_banners.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,18 +218,21 @@ def test_preview_banner(self, db_request):
218218

219219
class TestBannerForm:
220220
def test_required_fields(self, banner_data):
221-
form = views.BannerForm(data={})
221+
form = views.BannerForm(formdata=MultiDict())
222222

223223
assert form.validate() is False
224224
assert set(form.errors) == set(banner_data)
225225

226226
def test_valid_data(self, banner_data):
227-
form = views.BannerForm(data=banner_data)
227+
form = views.BannerForm(formdata=MultiDict(banner_data))
228228
assert form.validate() is True
229229
data = form.data
230230
defaults = {
231231
"fa_icon": Banner.DEFAULT_FA_ICON,
232232
"active": False,
233233
"link_label": Banner.DEFAULT_BTN_LABEL,
234234
}
235+
236+
# Mash the `end` into a date object to match the form's coerced result.
237+
banner_data["end"] = datetime.date.fromisoformat(banner_data["end"])
235238
assert data == {**banner_data, **defaults}

tests/unit/admin/views/test_sponsors.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -260,29 +260,29 @@ def test_required_fields(self):
260260
assert field in form.errors
261261

262262
def test_valid_data(self):
263-
form = views.SponsorForm(data=self.data)
263+
form = views.SponsorForm(formdata=MultiDict(self.data))
264264
assert form.validate() is True
265265

266266
def test_white_logo_is_required_for_footer_display(self):
267267
self.data["footer"] = True
268268

269269
# don't validate without logo
270-
form = views.SponsorForm(data=self.data)
270+
form = views.SponsorForm(formdata=MultiDict(self.data))
271271
assert form.validate() is False
272272
assert "white_logo" in form.errors
273273

274274
self.data["white_logo_url"] = "http://domain.com/white-logo.jpg"
275-
form = views.SponsorForm(data=self.data)
275+
form = views.SponsorForm(formdata=MultiDict(self.data))
276276
assert form.validate() is True
277277

278278
def test_white_logo_is_required_for_infra_display(self):
279279
self.data["infra_sponsor"] = True
280280

281281
# don't validate without logo
282-
form = views.SponsorForm(data=self.data)
282+
form = views.SponsorForm(formdata=MultiDict(self.data))
283283
assert form.validate() is False
284284
assert "white_logo" in form.errors
285285

286286
self.data["white_logo_url"] = "http://domain.com/white-logo.jpg"
287-
form = views.SponsorForm(data=self.data)
287+
form = views.SponsorForm(formdata=MultiDict(self.data))
288288
assert form.validate() is True

tests/unit/manage/test_forms.py

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ def test_validate_username_with_user(self):
5757
[
5858
("", "Select role"),
5959
("invalid", "Not a valid choice."),
60-
(None, "Not a valid choice."),
60+
(None, "Select role"),
6161
],
6262
)
6363
def test_validate_role_name_fails(self, value, expected):
@@ -81,7 +81,7 @@ def test_creation(self):
8181
def test_email_exists_error(self, pyramid_config):
8282
user_id = pretend.stub()
8383
form = forms.AddEmailForm(
84-
data={"email": "[email protected]"},
84+
formdata=MultiDict({"email": "[email protected]"}),
8585
user_id=user_id,
8686
user_service=pretend.stub(find_userid_by_email=lambda _: user_id),
8787
)
@@ -95,7 +95,7 @@ def test_email_exists_error(self, pyramid_config):
9595

9696
def test_email_exists_other_account_error(self, pyramid_config):
9797
form = forms.AddEmailForm(
98-
data={"email": "[email protected]"},
98+
formdata=MultiDict({"email": "[email protected]"}),
9999
user_id=pretend.stub(),
100100
user_service=pretend.stub(find_userid_by_email=lambda _: pretend.stub()),
101101
)
@@ -109,7 +109,7 @@ def test_email_exists_other_account_error(self, pyramid_config):
109109

110110
def test_prohibited_email_error(self, pyramid_config):
111111
form = forms.AddEmailForm(
112-
data={"email": "[email protected]"},
112+
formdata=MultiDict({"email": "[email protected]"}),
113113
user_service=pretend.stub(find_userid_by_email=lambda _: None),
114114
user_id=pretend.stub(),
115115
)
@@ -148,7 +148,7 @@ def test_verify_totp_invalid(self, monkeypatch):
148148
monkeypatch.setattr(otp, "verify_totp", verify_totp)
149149

150150
form = forms.ProvisionTOTPForm(
151-
data={"totp_value": "123456"}, totp_secret=pretend.stub()
151+
formdata=MultiDict({"totp_value": "123456"}), totp_secret=pretend.stub()
152152
)
153153
assert not form.validate()
154154
assert form.totp_value.errors.pop() == "Invalid TOTP code. Try again?"
@@ -158,7 +158,7 @@ def test_verify_totp_valid(self, monkeypatch):
158158
monkeypatch.setattr(otp, "verify_totp", verify_totp)
159159

160160
form = forms.ProvisionTOTPForm(
161-
data={"totp_value": "123456"}, totp_secret=pretend.stub()
161+
formdata=MultiDict({"totp_value": "123456"}), totp_secret=pretend.stub()
162162
)
163163
assert form.validate()
164164

@@ -182,10 +182,9 @@ def test_validate_confirm_password(self):
182182
),
183183
)
184184
form = forms.DeleteTOTPForm(
185-
username="username",
185+
formdata=MultiDict({"username": "username", "password": "password"}),
186186
request=request,
187187
user_service=user_service,
188-
password="password",
189188
)
190189

191190
assert form.validate()
@@ -218,7 +217,7 @@ def test_verify_assertion_invalid_json(self):
218217
)
219218

220219
form = forms.ProvisionWebAuthnForm(
221-
data={"credential": "invalid json", "label": "fake label"},
220+
formdata=MultiDict({"credential": "invalid json", "label": "fake label"}),
222221
user_service=user_service,
223222
user_id=pretend.stub(),
224223
challenge=pretend.stub(),
@@ -239,7 +238,7 @@ def test_verify_assertion_invalid(self):
239238
get_webauthn_by_label=pretend.call_recorder(lambda *a: None),
240239
)
241240
form = forms.ProvisionWebAuthnForm(
242-
data={"credential": "{}", "label": "fake label"},
241+
formdata=MultiDict({"credential": "{}", "label": "fake label"}),
243242
user_service=user_service,
244243
user_id=pretend.stub(),
245244
challenge=pretend.stub(),
@@ -255,7 +254,7 @@ def test_verify_label_missing(self):
255254
verify_webauthn_credential=lambda *a, **kw: pretend.stub()
256255
)
257256
form = forms.ProvisionWebAuthnForm(
258-
data={"credential": "{}"},
257+
formdata=MultiDict({"credential": "{}"}),
259258
user_service=user_service,
260259
user_id=pretend.stub(),
261260
challenge=pretend.stub(),
@@ -272,7 +271,7 @@ def test_verify_label_already_in_use(self):
272271
get_webauthn_by_label=pretend.call_recorder(lambda *a: pretend.stub()),
273272
)
274273
form = forms.ProvisionWebAuthnForm(
275-
data={"credential": "{}", "label": "fake label"},
274+
formdata=MultiDict({"credential": "{}", "label": "fake label"}),
276275
user_service=user_service,
277276
user_id=pretend.stub(),
278277
challenge=pretend.stub(),
@@ -290,7 +289,7 @@ def test_creates_validated_credential(self):
290289
get_webauthn_by_label=pretend.call_recorder(lambda *a: None),
291290
)
292291
form = forms.ProvisionWebAuthnForm(
293-
data={"credential": "{}", "label": "fake label"},
292+
formdata=MultiDict({"credential": "{}", "label": "fake label"}),
294293
user_service=user_service,
295294
user_id=pretend.stub(),
296295
challenge=pretend.stub(),
@@ -323,7 +322,7 @@ def test_validate_label_not_in_use(self):
323322
get_webauthn_by_label=pretend.call_recorder(lambda *a: None)
324323
)
325324
form = forms.DeleteWebAuthnForm(
326-
data={"label": "fake label"},
325+
formdata=MultiDict({"label": "fake label"}),
327326
user_service=user_service,
328327
user_id=pretend.stub(),
329328
)
@@ -337,7 +336,7 @@ def test_creates_webauthn_attribute(self):
337336
get_webauthn_by_label=pretend.call_recorder(lambda *a: fake_webauthn)
338337
)
339338
form = forms.DeleteWebAuthnForm(
340-
data={"label": "fake label"},
339+
formdata=MultiDict({"label": "fake label"}),
341340
user_service=user_service,
342341
user_id=pretend.stub(),
343342
)
@@ -363,7 +362,7 @@ def test_creation(self):
363362

364363
def test_validate_description_missing(self):
365364
form = forms.CreateMacaroonForm(
366-
data={"token_scope": "scope:user"},
365+
formdata=MultiDict({"token_scope": "scope:user"}),
367366
user_id=pretend.stub(),
368367
macaroon_service=pretend.stub(),
369368
project_names=pretend.stub(),
@@ -374,7 +373,7 @@ def test_validate_description_missing(self):
374373

375374
def test_validate_description_in_use(self):
376375
form = forms.CreateMacaroonForm(
377-
data={"description": "dummy", "token_scope": "scope:user"},
376+
formdata=MultiDict({"description": "dummy", "token_scope": "scope:user"}),
378377
user_id=pretend.stub(),
379378
macaroon_service=pretend.stub(
380379
get_macaroon_by_description=lambda *a: pretend.stub()
@@ -387,7 +386,7 @@ def test_validate_description_in_use(self):
387386

388387
def test_validate_token_scope_missing(self):
389388
form = forms.CreateMacaroonForm(
390-
data={"description": "dummy"},
389+
formdata=MultiDict({"description": "dummy"}),
391390
user_id=pretend.stub(),
392391
macaroon_service=pretend.stub(get_macaroon_by_description=lambda *a: None),
393392
project_names=pretend.stub(),
@@ -398,7 +397,9 @@ def test_validate_token_scope_missing(self):
398397

399398
def test_validate_token_scope_unspecified(self):
400399
form = forms.CreateMacaroonForm(
401-
data={"description": "dummy", "token_scope": "scope:unspecified"},
400+
formdata=MultiDict(
401+
{"description": "dummy", "token_scope": "scope:unspecified"}
402+
),
402403
user_id=pretend.stub(),
403404
macaroon_service=pretend.stub(get_macaroon_by_description=lambda *a: None),
404405
project_names=pretend.stub(),
@@ -412,7 +413,7 @@ def test_validate_token_scope_unspecified(self):
412413
)
413414
def test_validate_token_scope_invalid_format(self, scope):
414415
form = forms.CreateMacaroonForm(
415-
data={"description": "dummy", "token_scope": scope},
416+
formdata=MultiDict({"description": "dummy", "token_scope": scope}),
416417
user_id=pretend.stub(),
417418
macaroon_service=pretend.stub(get_macaroon_by_description=lambda *a: None),
418419
project_names=pretend.stub(),
@@ -423,7 +424,9 @@ def test_validate_token_scope_invalid_format(self, scope):
423424

424425
def test_validate_token_scope_invalid_project(self):
425426
form = forms.CreateMacaroonForm(
426-
data={"description": "dummy", "token_scope": "scope:project:foo"},
427+
formdata=MultiDict(
428+
{"description": "dummy", "token_scope": "scope:project:foo"}
429+
),
427430
user_id=pretend.stub(),
428431
macaroon_service=pretend.stub(get_macaroon_by_description=lambda *a: None),
429432
project_names=["bar"],
@@ -434,7 +437,7 @@ def test_validate_token_scope_invalid_project(self):
434437

435438
def test_validate_token_scope_valid_user(self):
436439
form = forms.CreateMacaroonForm(
437-
data={"description": "dummy", "token_scope": "scope:user"},
440+
formdata=MultiDict({"description": "dummy", "token_scope": "scope:user"}),
438441
user_id=pretend.stub(),
439442
macaroon_service=pretend.stub(get_macaroon_by_description=lambda *a: None),
440443
project_names=pretend.stub(),
@@ -444,7 +447,9 @@ def test_validate_token_scope_valid_user(self):
444447

445448
def test_validate_token_scope_valid_project(self):
446449
form = forms.CreateMacaroonForm(
447-
data={"description": "dummy", "token_scope": "scope:project:foo"},
450+
formdata=MultiDict(
451+
{"description": "dummy", "token_scope": "scope:project:foo"}
452+
),
448453
user_id=pretend.stub(),
449454
macaroon_service=pretend.stub(get_macaroon_by_description=lambda *a: None),
450455
project_names=["foo"],
@@ -478,7 +483,7 @@ def test_validate_macaroon_id_invalid(self):
478483
remote_addr="1.2.3.4", banned=pretend.stub(by_ip=lambda ip_address: False)
479484
)
480485
form = forms.DeleteMacaroonForm(
481-
data={"macaroon_id": pretend.stub(), "password": "password"},
486+
formdata=MultiDict({"macaroon_id": pretend.stub(), "password": "password"}),
482487
request=request,
483488
macaroon_service=macaroon_service,
484489
user_service=user_service,
@@ -499,10 +504,15 @@ def test_validate_macaroon_id(self):
499504
remote_addr="1.2.3.4", banned=pretend.stub(by_ip=lambda ip_address: False)
500505
)
501506
form = forms.DeleteMacaroonForm(
502-
data={"macaroon_id": pretend.stub(), "password": "password"},
507+
formdata=MultiDict(
508+
{
509+
"macaroon_id": pretend.stub(),
510+
"username": "username",
511+
"password": "password",
512+
}
513+
),
503514
request=request,
504515
macaroon_service=macaroon_service,
505-
username="username",
506516
user_service=user_service,
507517
)
508518

0 commit comments

Comments
 (0)