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.

Sign up for GitHub

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

Jump to bottom

BUGFIX: Avoid fatal error when asset not available #2177

Merged
merged 1 commit into from Sep 4, 2018
Merged

BUGFIX: Avoid fatal error when asset not available #2177

merged 1 commit into from Sep 4, 2018

Conversation

kdambekalns
Copy link
Member

This fixes what is basically a merge error. It lead to a method call
on an unset variable.

@kdambekalns
BUGFIX: Avoid fatal error when asset not available
9b881c1 
This fixes what is basically a merge error. It lead to a method call
on an unset variable.
@kdambekalns kdambekalns self-assigned this Sep 4, 2018
@kdambekalns
Copy link
Member Author

Introduced with #1745

gerhard-boden
gerhard-boden approved these changes Sep 4, 2018
View reviewed changes
Copy link
Contributor

@gerhard-boden gerhard-boden left a comment

Choose a reason for hiding this comment

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

Looks odd to have a switch statement with just one case, but maybe keep it since we'll add a lot more extensions in the future?

Code checks out by reading

@gerhard-boden
Copy link
Contributor

gerhard-boden commented Sep 4, 2018 •
edited

@kdambekalns can you point me to the method call on the unset variable in this change? (sorry if it's really obvious)

@kdambekalns
Copy link
Member Author

Yes, I left the switch there "for future use"…

@kdambekalns
Copy link
Member Author

Sure: it would be $asset->getFileExtension()

@gerhard-boden
Copy link
Contributor

Ah thanks, ofc, was moved inside the if statement with this 👍

@gerhard-boden
Copy link
Contributor

gerhard-boden commented Sep 4, 2018 •
edited

and since we assume the NeosAssetProxy will always have a asset property set, right?
Since we throw an exception inside the constructor if it would not upon creation

aertmann
aertmann approved these changes Sep 4, 2018
View reviewed changes
Copy link
Contributor

@aertmann aertmann left a comment

Choose a reason for hiding this comment

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

makes sense

@gerhard-boden gerhard-boden merged commit 241d676 into neos:4.1 Sep 4, 2018
@kdambekalns kdambekalns deleted the bugfix/assetsource-preview-type branch September 4, 2018 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aertmann aertmann aertmann approved these changes

@gerhard-boden gerhard-boden gerhard-boden approved these changes

@robertlemke robertlemke Awaiting requested review from robertlemke

@dfeyer dfeyer Awaiting requested review from dfeyer

Assignees

@kdambekalns kdambekalns

Labels
P: Media Browser T: PHP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@kdambekalns @gerhard-boden @aertmann