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

Proposal for accessing populated relations #1125

Merged
merged 4 commits into from Nov 7, 2013

Conversation

cebe
Copy link
Member

@cebe cebe commented Nov 3, 2013

fixes #842

  • allows checking whether a relation has been populated
  • getting a list of relation names that have been populated
  • getting all populated relation data

todo:

  • add phpdoc

fixes yiisoft#842

- allows checking whether a relation has been populated
- getting a list of relation names that have been populated
- getting all populated relation data

todo:

- [] add phpdoc
@ghost ghost assigned qiangxue Nov 3, 2013
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 90839ce on cebe:842-relation-data into d8b94d6 on yiisoft:master.

@qiangxue
Copy link
Member

qiangxue commented Nov 3, 2013

What are the use cases for getPopulatedRelationNames() and getPopulatedRelations()?

@cebe
Copy link
Member Author

cebe commented Nov 3, 2013

Use case for getPopulatedRelations() is described in #842 don't have a use case for the names only right now.

@qiangxue
Copy link
Member

qiangxue commented Nov 3, 2013

Perhaps it's better to turn _related into protected or public, and rename it to relatedRecords.

@cebe
Copy link
Member Author

cebe commented Nov 3, 2013

There is a problem with storing them with lower case name. They should be available as their original methods name in camel case.

@qiangxue
Copy link
Member

qiangxue commented Nov 3, 2013

Agree. We can remove strtolower() call.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.17%) when pulling a23c54a on cebe:842-relation-data into d8b94d6 on yiisoft:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 0bc8cbf on cebe:842-relation-data into d8b94d6 on yiisoft:master.

@cebe
Copy link
Member Author

cebe commented Nov 7, 2013

I'd prefer to have the methods instead of public property as they allow to get overriden and to interact with the process of populating and getting relational data.
What do you think @qiangxue ?

@qiangxue
Copy link
Member

qiangxue commented Nov 7, 2013

Looks good to me. Thanks.

qiangxue added a commit that referenced this pull request Nov 7, 2013
Proposal for accessing populated relations
@qiangxue qiangxue merged commit 3701cbd into yiisoft:master Nov 7, 2013
@cebe cebe deleted the 842-relation-data branch November 7, 2013 17:49
@klimov-paul klimov-paul mentioned this pull request Dec 20, 2013
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.

Cannot access $_related in Child class of \yii\db\ActiveRecord
3 participants