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

TrashedFilter: Stop get_class call when FQCN is passed #101

Open
wants to merge 1 commit into
base: 0.6
Choose a base branch
from
Open

TrashedFilter: Stop get_class call when FQCN is passed #101

wants to merge 1 commit into from

Conversation

acairns
Copy link

@acairns acairns commented Feb 14, 2015

When checking if an entity is soft-deletable, a FQCN is passed - resulting in get_class being called on a string.

Fixes:
ErrorException: get_class() expects parameter 1 to be object, string given

@kirkbushell
Copy link
Collaborator

This needs an update - it will break when the entity object is passed, so it should cater for both scenarios.

If you could update it to include that, then we can merge it in.

@acairns
Copy link
Author

acairns commented Feb 14, 2015

Wouldn't $metadata->rootEntityName always be a string?

@kirkbushell
Copy link
Collaborator

Not sure - but this change breaks the package :) If you can fix that up, we're good to go.

@acairns
Copy link
Author

acairns commented Mar 9, 2015

@kirkbushell I ran the tests before committing, they were passing locally.

Is the CI hosted somewhere where I can take a look?
Also, I'm up-to-date with 0.6 and the build still passes for me locally.

@kirkbushell
Copy link
Collaborator

@acairns not sure, tbh I checked this out a couple of weeks ago and it was breaking then - which was the reason for the original comment.

@sisve
Copy link

sisve commented Apr 24, 2015

This issue is blocking us from using a non-forked laravel-doctrine. The property ClassMetadata::$rootEntityName is a string according to the documentation. All tests run locally with "OK (11 tests, 17 assertions)", no matter how much I mess up TrashedFilter.php. (I guess it isn't covered at all.)

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