Item10625: The defined in an IF has problems with syntax precedence when used in a more complex way

THis was originally "New group topic has a false alarm telling to upgrade it."

Even if a group is of the new format there is a button visible telling to upgrade it.

-- KennethLavrsen - 11 Apr 2011

Further analysis shows that it is a much more severe problem. It is the IF that is broken and it seems it is the defined() part of the query language that is broken

Here is an example

In a bug item we have a preference VIEW_TEMPLATE = ItemView

Now we test for it with this

%IF{"defined('Tasks.Item10625'/preferences[name='VIEW_TEMPLATE'])"
  then="true"
  else="false" 
}%

Or in reality

true

It should return true and it does here on foswiki.org which is a beta of 1.1.3 so the code must be broken very recently.

-- KennethLavrsen - 12 Apr 2011

It was Olivier that broke it when he fixed something else.

r11162 | OlivierRaginel | 2011-03-20 13:57:40 +0100 (Sun, 20 Mar 2011) | 1 line

Item10465: Fix precedence on OP_ref, otherwise it is considered a constant...

-- KennethLavrsen - 12 Apr 2011

playing with unit tests, and I can't get much sanity.

the code clearly does not support any OP's - defined can only be followed by a simple string.

additionally, I didn't realise that defined was only an IF operator ( this won't change until 1.2 - unless there's a reason? (can't see any commentary)

I have no idea why changing the precedence for OP_ref makes a difference - so I'm going to commit my tests and leave it for Crawford to explain.

fundamentally, this is an operator that only works with a very small set of input parameters, and so must provide feedback to the user that their inputs are invalid - ie, a big red parse error. (not that I don't realise that is hard, but we really should never pass on the hard problems to our users).

It'll be interesting to see why this appeared to work in 1.1.0....

-- SvenDowideit - 12 Apr 2011

This appears to stem partly from a misunderstanding of how the defined operator works, and partly from a genuine bug. As the documentation states:

defined

True if a preference setting or url parameter of this name is defined.

"of this name" refers to the parameter to the defined operator. Given an expression such as

%IF{"defined('Tasks.Item10625'/preferences[name='VIEW_TEMPLATE'])"
  then="true"
  else="false" 
}%

you are asking if the name yielded by the query 'Tasks.Item10625'/preferences[name='VIEW_TEMPLATE'] is defined as a URL parameter or a preference. Whether this evaluates to true or not is highly dependent on the context, and it's for this reason (and the difficulty of mapping to SQL) that the defined operator was never adopted into queries.

Having said that, there is a problem with the evaluation when the tested term is bracketed. Investigating.

-- CrawfordCurrie - 12 Apr 2011

Pretty sure I fixed the real problem, but I don't fully understand the original report, which has a much more complex sub-expression than I would expect, so leaving open for feedback from Kenneth. if it's OK, please waiting-for-release it, thanks!

-- CrawfordCurrie - 12 Apr 2011

All seems to pass now.

Even the strange notation that was used.

I have removed the condition that caused the problem. It is not needed. The

not ( '%USERSWEB%.%groupname%'/preferences[name='VIEW_TEMPLATE'].value = 'GroupView' )

will only return true if the VIEW_TEMPLATE exists anyway so there is no need to also test if it is defined.

Setting to waiting for release. Changing the title again to reflect what we fixed.

-- KennethLavrsen - 12 Apr 2011

Commenting is disabled while not logged in