Problem with CouchDB (fix included) #828

Closed
leonardp opened this Issue Feb 19, 2013 · 6 comments

Projects

None yet

4 participants

@leonardp

When querying the CouchDB design sheet in the form of:

$result = Model::find('all' , array( 'conditions' => array ('design' => 'design', 'view' => 'all')));
(the design sheet filters out a part of the CouchDB collection)

the result contains two unwanted entries, namely:
["id"] => null
["rev"] => null
I traced this back to
7c7832b
more specifically the introduced shorthand if expression in the _format method.

The issue is resolved by replacing it with an explicit if statement
if(isset($data["{$key}"])){
$data[$key] = $data["
{$key}"];
}
and the removal of line 494.

@blainesch
Member

Can a pull request be sent with a valid test?

@leonardp

How should this test look like?
Should i write a method for the CouchDB or would a simple
"do this do that" (as seen above) attached to the pull request do?
Are there any guidelines for how a pull request should look like?

@nateabele
Member

@leonardp Thanks for the report. The simplest way would be to write a test that calls CouchDb::item() with a $data parameter that triggers the offending issue (because item() passes its data to _format()).

For examples on how to write tests (and where to put them), check out the testItem() method of CouchDbTest: https://github.com/UnionOfRAD/lithium/blob/master/tests/cases/data/source/http/adapter/CouchDbTest.php#L79

For general information on submitting patches, please review our Contributor Guide. Thanks again!

Btw, @BlaineSch, that Branching section should probably be updated based on that conversation we had about branch naming, but I don't remember where we ended up. Do you?

@blainesch
Member
@nateabele
Member

Looks good to me. Merged. :-)

@jails
Collaborator
jails commented Apr 13, 2013

Same as #873 fixed in fadf37b

@jails jails closed this Apr 13, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment