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

TEST: boolean include template arguments don't evaluate correctly #2140

Conversation

jthomerson
Copy link
Contributor

When you pass boolean arguments to an include they get converted to strings and thus do not work for conditional evaluations inside the include. It seems that you should be able to use at least the following three types of arguments to an include and use them as a conditional statement inside the include:

  1. method return or field value that returns / is boolean
  2. method return or field value that returns / is zero or one
  3. hard-coded true/false in the calling template to turn features of the include on or off depending on where you are including it.

This test reproduces those scenarios and shows that they are failing so that we can use it as a basis for fixing the functionality.

@jthomerson
Copy link
Contributor Author

I have not yet come up with a clean fix, but here is a description of why this happens:

First, when the template that calls the include gets compiled, it calls $scope->obj('METHODNAME', null, true) which returns a casted object and uses that in the arguments array for the included template.

Example from the test case in this pull request:

$val .= SSViewer::execute_template('SSViewerTestUsingBooleanArgumentsInclude', $scope->getItem(), array('DoSomething' => $scope->obj('NumberIsEven', null, true)->self())); 

Since there is not a cast defined in $casting for arbitrary methods, this returns a Text object (the default cast).

Then, later, in the included template, if ($scope->hasValue('ARGUMENTNAME', null, true)) is called. Then, in SSViewer->__call (in the 'hasValue' block), $this->getInjectedValue($property, $params, false) is called - in an attempt to get an uncasted value. However, since the casted value was already put into the arguments, which are now the SSViewer_DataPresenter's overlay, the casted value is returned (Text object).

The Text object actually has the correct (boolean) value inside of it (i.e. from debugger: $val['value']->value = (bool) 0). However, SSViewer->__call just casts the object to a boolean, which doesn't work since this is an object ($hasInjected = true; $res = (bool)$val['value'];). Thus, you get a false.

I see three possible solutions at this point, but I haven't experimented with them to see what else they might break. I'll list them in order of my preference and hope that I can get some feedback from other devs:

  1. pass false instead of true when collecting arguments to avoid getting casted objects into the arguments array in the first place.
  2. try calling $val['value']->exists() instead of casting to (bool) in SSViewer->__call
  3. create a toBoolean function on DBField that can be overridden by each DB field type to appropriately cast the value to a boolean. Then use this method instead of casting to a (bool) in SSViewer->__call

@jthomerson
Copy link
Contributor Author

@hafriedlander any input on which way you want me to go with this? I think Boolean arguments are obviously one of the most useful ways to use includes (i.e. <% include something someDeatureOfInclude=true %>)

I just wasn't sure if you have input on which of the options above (or something different) would be best to pursue. Also note that number twos likely better than one because one may lose the casting info which is later needed if the Boolean came as a return value for something that was configured in $casting.

@jthomerson
Copy link
Contributor Author

@hafriedlander / @chillu / etc ... let me know if you determine something on this. I'm willing to make it work however the group thinks it should.

@hafriedlander
Copy link
Contributor

@jthomerson I haven't put a lot of thought into this, but off the top of my head I'm not sure there's a bug here. Does this issue occur if you properly cast the method / field that's returning the "false" or "0" as a Boolean via $casting? Assuming it doesn't, the only issue is that you can't explicitly hardcode "Foo=false" in the include statement.

Any fix will probably involve some tweaky changes to how casting is handled. At this point we're only looking at critical fixes for 3.1, so since this has a high chance of regressions and isn't critical it won't get merged. I'll give it some more thought once 3.1 is out.

@camspiers
Copy link
Contributor

Is now a good time to revisit this? I have having this issue too

@blueskies79
Copy link
Contributor

I'm having this issue too. This is especially a problem with a permissions structure where the persmissions I've set up (canView, canEdit, etc) are booleans and aren't properly passed to the include. For me at the moment this is an extremely critical issue. Any workaround will require some serious rebuilding of the app, and leaving it as is is not an option.
Oh, also tried casting the method as a Boolean... no dice...

@simonwelsh
Copy link
Contributor

As this pull request is never going to get merged (we don't merge failing tests), can you move this to an issue please?

@simonwelsh simonwelsh closed this Mar 15, 2014
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.

5 participants