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 conditionCallback invocation when using DoctrineCollectionDataSource #672

Closed
wants to merge 1 commit into
base: v5.x
from

Conversation

3 participants
@ondrejmastik
Copy link

ondrejmastik commented Jun 22, 2018

Now, when using DoctrineCollectionDataSource, application fails on condition callback invocation. it's caused by DoctrineCollectionDataSource having private access to data_source property instead of protected.

Please merge

Thanks.

@ondrejmastik

This comment has been minimized.

Copy link
Author

ondrejmastik commented Jun 22, 2018

Would it be a bad idea to add abstract method getDataSource to FilterableDataSource? Obviously every DataSource should have accessable dataSource for purpose of filter condition callback invocation, so noone would make this mistake again when creating new DataSource

/**
* @var string
*/
private $primary_key;
protected $primary_key;

This comment has been minimized.

@paveljanda

paveljanda Jun 27, 2018

Member

Can you convert the identation to tabs?

/**
* @var Criteria
*/
private $criteria;
protected $criteria;

This comment has been minimized.

@paveljanda

paveljanda Jun 27, 2018

Member

Can you convert the identation to tabs?

This comment has been minimized.

@ondrejmastik

ondrejmastik Jun 27, 2018

Author

Aren't they already indented by tabs?

This comment has been minimized.

@ondrejmastik

ondrejmastik Jun 27, 2018

Author

Hmm they, arent. Sorry

@paveljanda

This comment has been minimized.

Copy link
Member

paveljanda commented Sep 19, 2018

👍 after fixing the indentation.

f3l1x added a commit that referenced this pull request Oct 29, 2018

@f3l1x

This comment has been minimized.

Copy link
Member

f3l1x commented Oct 29, 2018

I've merged it by my own. Thanks anyway @ondrejmastik.

@f3l1x f3l1x closed this Oct 29, 2018

@f3l1x f3l1x added this to the v5.8 milestone Oct 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.