Skip to content

Conversation

jleightcap
Copy link
Contributor

@jleightcap jleightcap commented Jun 1, 2023

Prefer InputRequired over DataRequired

See wtforms note:

Unless a very specific reason exists, we recommend using the InputRequired instead.

This PR removes instances of DataRequired, and populates previously implicit data into formdata.

Testing Changes

  • All Form types now have an associated test_validate that gives an example of a valid construction and validation.

Functional Refactors

All constructions of form objects outside of the definitions in forms.py:

warehouse/forklift/legacy.py

  • No changes.

warehouse/manage/views/init.py

  • Wraps **self.request.POST into MultiDict.
  • Moves the required fields in DeleteMacaroon field into the formdata MultiDict.

warehouse/manage/init.py

  • Constructions by request, no change.

warehouse/manage/views/organizations.py

  • default_response -- this is partially initialized, later with a reponse object. This is a general pattern, I'm not sure why?
  • Most forms constructed by request, no change.

warehouse/manage/views/teams.py

  • One fix with the SaveTeamForm.

warehouse/accounts/views.py

  • Two instances of a **request. first arg being wrapped in a MultiDict.
  • What's with request.params instead of request.POST on reset_password?
  • These break tests. TODO

warehouse/views.py

  • Fix SetLocaleForm construction.

warehouse/manage/views/init.py

  • Wrap **self.request.POST in MultiDict.

warehouse/admin/views/users.py

  • How does a UserForm take a raw user object as the second arg?

warehouse/admin/views/sponsors.py

  • Same as above with SponsorForm and sponsor object.

warehouse/admin/views/banners.py

  • No changes.

Copy link
Member

@woodruffw woodruffw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be good to go -- I've tested the forms manually to confirm that they're working with these changes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User testing: confirmed functionality here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User testing: confirmed functionality here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User testing: confirmed functionality here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User testing: confirmed functionality here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(After fixing up the password reset form.)

@jleightcap jleightcap marked this pull request as ready for review June 1, 2023 17:08
@jleightcap jleightcap requested a review from a team as a code owner June 1, 2023 17:08
@woodruffw woodruffw requested a review from di June 1, 2023 21:39
jleightcap and others added 13 commits June 5, 2023 14:28
…3696)

* 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]>
Signed-off-by: Jack Leightcap <[email protected]>

Validated CreateMacaroonForm

Signed-off-by: Jack Leightcap <[email protected]>
Signed-off-by: Jack Leightcap <[email protected]>
Signed-off-by: Jack Leightcap <[email protected]>
Signed-off-by: Jack Leightcap <[email protected]>
Signed-off-by: Jack Leightcap <[email protected]>
Signed-off-by: Jack Leightcap <[email protected]>
Signed-off-by: Jack Leightcap <[email protected]>
Signed-off-by: Jack Leightcap <[email protected]>
Signed-off-by: Jack Leightcap <[email protected]>
Signed-off-by: Jack Leightcap <[email protected]>
@woodruffw
Copy link
Member

woodruffw commented Jun 5, 2023

Deconflicting now.

Edit: Whoops, you beat me to it 😅

Signed-off-by: William Woodruff <[email protected]>
@di di enabled auto-merge (squash) June 5, 2023 14:34
Signed-off-by: William Woodruff <[email protected]>
auto-merge was automatically disabled June 5, 2023 14:46

Head branch was pushed to by a user without write access

@woodruffw
Copy link
Member

Tests were failing because of a form rename; I've removed the old tests (since they're now covered by TestCreateOrganizationApplicationForm).

@di di enabled auto-merge (squash) June 5, 2023 14:47
@di di merged commit 20fce6e into pypi:main Jun 5, 2023
@woodruffw woodruffw deleted the jl/13695 branch June 5, 2023 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants