This repository has been archived by the owner on Sep 16, 2021. It is now read-only.
[RFC] PhpcrProvider refactoring #257
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| $document = $this->find($name, $options); | ||
|
|
||
| if (null === $document) { | ||
| return false; |
There was a problem hiding this comment.
Now .. the only situation where has returns false is when the document does not exist. If the document is not a ItemInterface instance an exception will be thrown.
|
great job, so much better! i guess we should add a note to the changelog to explain, in case somebody was working around strange data in their repository and now getting the clearer exceptions and being confused by them. |
|
can you rebase on master so we see what really changes in the test? i just merged #256 |
dantleech
force-pushed
the
provider_refactoring
branch
from
1b0192b
to
db0ac7a
Compare
|
Rebased, but is likely now taking the wrong approach as we are discussing in #254 |
|
This one and its issue is tagged with milesonte for version 2.2. Do we need that? |
|
Anything going on here? |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #255
Based on #256
WIP.
This PR refactors the PhpcrMenuProvider to throw errors more often.
The current problem is that
haswas hiding all of the Exceptions which were happening in thefindmethod and interpreting then as "menu not found".I think the only time it should return
falseis when the menu document was not found. If the menu was found and it subsequently encountered another problem, then we should throw an exception.F.e. if the menu document does not implement
ItemInterface, then what is it doing at the menubasepath? The most likely scenario is that the developer forgot to add theItemInterfaceto the document.It does alot of reorganizing, making the code easier to understand (I hope!).