Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fix magic findByXXXAndYYY finders when using related object arguments #158

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

maikg commented Oct 23, 2011

When using any of the findByXXXAndYYY and related magic methods, I receive an error when specifying a related object as one of the arguments:

BookQuery::create()->findOneByAuthorAndISBN($author, 1234);

This causes the following exception:

PropelException: filterByAuthor() only accepts arguments of type Author or PropelCollection

This PR should fix that.

Member

fzaninotto commented Oct 24, 2011

I don't understand why your fix - which probably breaks som other parts - is required...

Contributor

maikg commented Oct 24, 2011

The Propel documentation specifies the following:

If you need to find objects related to a model object that you already have, you can take advantage of the generated filterByXXX() methods in the query objects, where XXX is a relation name

It also specifies that magic finders should work with multiple arguments by using finders of the form findByXXXAndYYY. To me, that means that the example I mentioned above should not throw an exception, yet it does.

The existing code includes a bit of ambiguity in my opinion; according to PHP's documentation:

For any of the types: integer, float, string, boolean and resource, converting a value to an array results in an array with a single element with index zero and the value of the scalar which was converted. In other words, (array)$scalarValue is exactly the same as array($scalarValue).

If an object is converted to an array, the result is an array whose elements are the object's properties.

If the point of the existing code is to wrap the argument in an array, then wouldn't it be better to just write array($args)? All the unit tests still pass after this change.

fzaninotto added a commit to fzaninotto/Propel that referenced this pull request Oct 25, 2011

Owner

willdurand commented Oct 26, 2011

PR #163 fixed this issue.

@willdurand willdurand closed this Oct 26, 2011

willdurand added a commit that referenced this pull request Sep 9, 2013

Merge pull request #158 from omakoleg/patch-1
Update reference/active-record.markdown
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment