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

[MRG+1] Allow dropping field in from_response formdata #2721

Merged
merged 3 commits into from
May 16, 2017

Conversation

HarrisonGregg
Copy link
Contributor

Implements #667. FormRequest.from_response is very helpful, but sometimes it includes a field that you would like to remove. With this change, you can include a field with value None in formdata when calling from_response, and the field will be omitted from the body of the returned request.

@HarrisonGregg HarrisonGregg changed the title Feature drop from response field Allow dropping from_response field Apr 30, 2017
@HarrisonGregg HarrisonGregg changed the title Allow dropping from_response field Allow dropping field in from_response formdata Apr 30, 2017
@codecov-io
Copy link

codecov-io commented Apr 30, 2017

Codecov Report

Merging #2721 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2721   +/-   ##
=======================================
  Coverage   84.75%   84.75%           
=======================================
  Files         162      162           
  Lines        9155     9155           
  Branches     1358     1359    +1     
=======================================
  Hits         7759     7759           
  Misses       1145     1145           
  Partials      251      251
Impacted Files Coverage Δ
scrapy/http/request/form.py 95.9% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dfe6d3d...ffef828. Read the comment docs.

@@ -135,7 +135,7 @@ def _get_inputs(form, formdata, dont_click, clickdata, response):
if clickable and clickable[0] not in formdata and not clickable[0] is None:
values.append(clickable)

values.extend(formdata.items())
values.extend([(k, v) for k, v in formdata.items() if v is not None])
Copy link
Member

Choose a reason for hiding this comment

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

no need to build an intermediate list. Just use:

values.extend((k, v) for k, v in formdata.items() if v is not None)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't realize you could do that. Thanks! Would you prefer if I squashed this into the other commit on this file?

@dangra
Copy link
Member

dangra commented May 15, 2017

Nice to have feature, simple implementation and good test case. thanks @HarrisonGregg

@dangra
Copy link
Member

dangra commented May 15, 2017 via email

@HarrisonGregg HarrisonGregg force-pushed the feature-drop-from-response-field branch from 313f157 to ffef828 Compare May 15, 2017 16:25
@HarrisonGregg
Copy link
Contributor Author

@dangra Squashed.

@dangra dangra changed the title Allow dropping field in from_response formdata [MRG+1] Allow dropping field in from_response formdata May 15, 2017
@dangra
Copy link
Member

dangra commented May 15, 2017

@redapple I think this is ready to go. Is it still in time for 1.4.0?

@redapple
Copy link
Contributor

@dangra , alright, lets include it.
Thank you @HarrisonGregg

@redapple redapple added this to the v1.4 milestone May 16, 2017
@dangra dangra merged commit 73668ce into scrapy:master May 16, 2017
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.

None yet

4 participants