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

Fix regression: allow using custom methods. #440

Merged
merged 1 commit into from
Jan 9, 2017
Merged

Conversation

peter17
Copy link
Contributor

@peter17 peter17 commented Dec 16, 2016

We used to be able to use our own methods to generate the $choiceLabel, instead of just the columns of the object. Now, Propel complains about the column not existing in the object.

We used to be able to use our own methods to generate the `$choiceLabel`, instead of just the columns of the object. Now, Propel complains about the column not existing in the object.
@tjveldhuizen
Copy link

I'm not experienced with pull-requests. This PR is to merge a commit into the 3.0 branch, however the problem also (or just?) exists in the 1.5 branch (which we are using). Should we create a second pull-request with the same change, for this other branch?

Second question: the PR has been created 21 days ago and nothing happend since. What has to happen before it will be merged?

@peter17
Copy link
Contributor Author

peter17 commented Jan 9, 2017

@marcj There are a few PRs with tests passing, here. Could you please take some time to review and merge them? That would be awesome! Thanks!

@marcj marcj merged commit c4359c3 into propelorm:3.0 Jan 9, 2017
$choiceLabel = function($choice) use ($valueProperty) {
$getter = 'get'.ucfirst($valueProperty);
if (!method_exists($choice, $getter)) {
$getter = 'get' . ucfirst($query->getTableMap()->getColumn($valueProperty)->getPhpName());

Choose a reason for hiding this comment

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

@peter17, are you sure this is working? $query is not available within the closure, isn't it?

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 have fixed that in #444

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