A few mods.. - #1
Open
netadvanced wants to merge 3 commits into
Open
Conversation
Note: The `inclusive` argument could be ambiguous with the core.findAll()`include` argument and could be changed to something like `stack`, `stackArguments`. I initially tried to keep the same logic as the new `exclusive` argument.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Howdy Joshua,
As you will see, I added parentheses to the SQL statement to make it more bullet-proof in case you're using OR or BETWEEN statements for example. As well as made a few mods to keep backwards compatibility.
I had initially started creating extra DefaultScope arguments, such as
mandatoryWhere,mandatoryOrder, etc... to be able to force "additional" clauses, but I was working on an older version and now that I see your solution, it is cleaner. My only worry with your solution was the fact that it would then make all arguments "additional" unless we use theexcludeargument, which was kind of defeating the purpose of your original plugin idea of having default values overridden by the FindAll arguments.I used the
inclusivekeyword that might be confusing for some, but respecting your exclusive syntax... we might have to change that.Anyway, let me know what you think ?