Skip to content

Conversation

@bessbd
Copy link
Member

@bessbd bessbd commented Dec 6, 2019

Overview

This change should allow users to supply all params in POST that can be supplied for GET now. This way we could avoid the ?key="foo" things that would probably cause a lot of pain for users.

As /{db}/_design_docs and /{db}/_local_docs are analogous to _all_docs, this change applies to all three of them.

Testing recommendations

Just run the tests included with this PR.
Let me know if you need instructions for manual testing.

Related Issues or Pull Requests

apache/couchdb-documentation#462

Checklist

bessbd pushed a commit to bessbd/couchdb-documentation that referenced this pull request Dec 6, 2019
In apache/couchdb#2343 the way POST _all_docs works
is altered so that the same functionality is available as in GET _all_docs.

This commit updates the POST _all_docs documentation to reflect this new behavior.
@bessbd
Copy link
Member Author

bessbd commented Dec 6, 2019

Link to the doc PR: apache/couchdb-documentation#462

Copy link
Member

@davisp davisp left a comment

Choose a reason for hiding this comment

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

Nice work! Turned out quite a bit simpler than I expected but that parse_body_and_params is reused to good effect.

Couple minor changes to clear up before merging.

{ok, VDoc} = ddoc_cache:open(DbName, <<"_design/", ViewDesignName/binary>>),
CB = fun list_cb/2,
QueryArgs = couch_mrview_http:parse_params(Req, Keys),
Args = couch_mrview_http:parse_body_and_query(Req, Keys),
Copy link
Member

Choose a reason for hiding this comment

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

I'd not change the variable name here which will shorten the diff. I assume that was done because its now more than the query string. Though we refer to view reads as queries in multiple places so there's no harm leaving it as QueryArgs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in 5f16384

parse_body_and_query(Req, Props, Keys);

parse_body_and_query(Req, Keys) ->
parse_params(chttpd:qs(Req), Keys, #mrargs{keys=Keys, group=undefined, group_level=undefined}, [keep_group_level]).
Copy link
Member

Choose a reason for hiding this comment

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

Project style is to wrap lines at 80 columns.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in b025b2c

@bessbd
Copy link
Member Author

bessbd commented Dec 6, 2019

Thank you for the review @davisp ! I'll push commits for the changes you've requested soon.

Copy link
Member

@davisp davisp left a comment

Choose a reason for hiding this comment

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

+1

@davisp
Copy link
Member

davisp commented Dec 6, 2019

Go ahead and squash those fixup commits and then we'll wait for CI to come back.

This change should allow users to supply all params in POST that can be supplied for GET now. This way we could avoid the ?key="foo" things that would probably cause a lot of pain for users.

As /{db}/_design_docs and /{db}/_local_docs are analogous to _all_docs, this change applies to all three of them.
@bessbd bessbd force-pushed the all-docs-post-all-params branch from b025b2c to 7ae4a2f Compare December 6, 2019 21:21
@garrensmith garrensmith merged commit 8ac108a into apache:master Dec 9, 2019
garrensmith pushed a commit to apache/couchdb-documentation that referenced this pull request Dec 9, 2019
Update POST for _all_docs

In apache/couchdb#2343 the way POST _all_docs works
is altered so that the same functionality is available as in GET _all_docs.

This commit updates the POST _all_docs documentation to reflect this new behavior.

Co-Authored-By: Jonathan Hall <flimzy@flimzy.com>
nickva pushed a commit to nickva/couchdb that referenced this pull request Sep 7, 2022
Update POST for _all_docs

In apache#2343 the way POST _all_docs works
is altered so that the same functionality is available as in GET _all_docs.

This commit updates the POST _all_docs documentation to reflect this new behavior.

Co-Authored-By: Jonathan Hall <flimzy@flimzy.com>
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