Optional FunctionVar Filters in a Dynamic Get #576
No reviewers
Labels
No Milestone
No project
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: joomla/Component-Builder#576
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "master"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary of Changes
Referring to this topic: https://groups.google.com/a/vdm.io/forum/#!topic/jcb/b0OUQ80h69Q
I found it is more flexible to make the FunctionVar filters optional, and the case descriped in the above topic explains it in more details.
I just removed the code that adds the else statement in case the filter value is empty.
Testing Instructions
Add a FunctionVar filter to a dynamic get and compile.
Expected result
The generated filter code adds the filter condition only if the filter value is not empty, which makes it optional.
Actual result
The generated filter code has an else statement in case the filter value is empty, it returns false, which causes a database error.
I have read over the communication on the forum, I need to spend more time on this, since it is a change of logic, and current expected behavior. Which will upset upstream users, so making a change the way you did will break many existing components. We will need to be more creative.
Ok @Llewellynvdm
When you have time, please explain the logic behind this else statement, maybe I can help.
Just that you know, I am not saying that we are not going to make a change, that we should do it in a way that server all. So it is happening.
Sorry for the delay here. I have not yet been able to give more time here.
Yet after a quick look at the compiler, it seems like I am treating the filter like an absolute requirement and this serves as a barrier to the data. So that if the filter is not set no data is returned. I understand that is not the expected behavior, but that is what I did, and that is how I have used it in many projects, so have other. So to change this just directly will expose data which otherwise would not have happened. So my idea is to add a switch of some kind that gives the developer the option to allow empty filter values (consciously), and not just change the default (how it works now) and then unconsciously data gets exposed.
If you can come up with some ideas in the meantime that will be helpful, but just removing the else statement without giving the upstream developers a heads-up (switch to choose) is never a good idea.
Never mind, it is ok.
So, what you suggest and believe that it is more efficient is to add, for example, a checkbox in each filter row, to choose whether the filter to enable/disable having empty filters.
Do you think this is a good implementation?
Okay the staging branch has moved with almost 100 lines, so merge is not possible anymore.
I have added the checkbox to the filters, that gives the option to allow empty:
In the Compiler I have changed the code:
So the change has been effected in this commit. Let me know if there is any issues.
Great!
Thank you for adding this feature.
Pull request closed