Foswiki on GitHub is open for business! Next release meeting: Monday September 1, 1300Z

Item12466: SEARCH formfield method returning label instead of value in select+value fields

Priority: CurrentState: AppliesTo: Component: WaitingFor:
Urgent Closed Engine SEARCH  
$formfield SEARCH method is wrongly returning the label instead of the value of a field in Foswiki 1.1.7. There is this old task about this problem at http://foswiki.org/Tasks/Item9404. In Foswiki-1.1.2 it was solved, it used to return the value portion of the select+value field. Now in Foswiki-1.1.7 the problem is back, it is returning the label instead of the field. This is breaking applications developed in Foswiki-1.1.2 that cant be upgraded for this reason.

A workaround is to use QUERY inside the SEARCH, replacing
$formfield(FieldName)

by

$percntQUERY{\"'$topic'/fields[name = 'FieldName'].value\"}$percnt

However that requires to rewrite all applications.

I have not checked Foswiki 1.1.8 but as there was no bug report I assume the problem is still present in Foswiki 1.1.8

http://irclogs.foswiki.org/bin/irclogger_log/foswiki?date=2013-03-01,Fri&sel=357#l353

Example

In the web Formfield_example at the Sandbox there is an example of the problem.

This example from a real application running on Foswiki 1.1.2 where the same code at DocumentList
 [[DiffusionGroup$formfield(Groupe)][$formfield(Groupe)]]

with the Groupe field containing

| Groupe | select+values | 1 | A: Tous=A, B: Financiers; Rapports =B, C: Négociation; Syndicat =C, | Quel est le groupe de diffusion du document? | |

produces this output, returning the value of the select+value field and creating the link as expected.

DocumentList_foswiki_1.1.2.PNG

However in current foswiki the $formfield method is returning the label from the Groupe field, breaking the link on the Groupe column (DocumentList). Inspecting the output shows the link is built using the label instead of the value of the field. Even though it might be matter of discussion whether formfield should return the label or the value of the field, this unexpected change of behavior is breaking the application.

foswiki_1.2.8_DocumentList_link_broken.PNG

Thanks

-- EnriqueCadalso - 05 Apr 2013

Thanks Enrique.

-- MartinCleaver - 15 Apr 2013

See Item10071 for the root of this problem. The behaviour of $formfield changed in 1.1.5.

Resolved by adding the 'stored' modifier to $formfield to support seeing the stored value as against the mapped value.

I marked it as releasable in 1.2.0 but we have to decide whether to release this in 1.1.9 or not. George?

-- CrawfordCurrie - 15 Apr 2013

Doesn't extending $formfield() need a feature requrest?

-- MichaelDaum - 16 Apr 2013

After sleeping on it, I'm more of the opinion to reverse the behaviour such that the default is to display the stored value from a $formfield call (compatible with 1.1.4 and earlier, and with tmwiki) and require a modifier to get the displayed value.

The reasons are:
  • compatibility with older wiki applications, and with tmwiki
  • forward compatibility - wiki applications written for latest release should work with <= 1.1.4 if possible

So, instead of the current $formfield(Name,stored) to get the stored value, I'm going to change it to $formfield(Name,display) to get the mapped, displayed value.

This will change behaviour for wiki apps written for 1.1.4 through 1.1.8 but I think that's right; this functionality change was introduced in a patch and needs to be reverted and applied to the release branch for 1.1.9 (if it happens).

I don't think a feature request is required to fix the bug - the change in behaviour of plain $formfield in 1.1.5 - but I also think we need to provide a solution for 1.1.9 users too, until 1.2.0 is ready.

Later JulianLevens point out ConsistentFormFieldValues which covers this issue.

-- CrawfordCurrie - 16 Apr 2013

The bug report is merely a sad realization of bad design.

As far as I know returning the displayed value is indeed old behavior since the ages of tyrannosaurus rex.

All ways to return the "value" of a formfield should be somewhat consistent or have at least the same capabilities to return the displayed and/or the stored value. These places are:

  1. %SEARCH'es $formfield
  2. %FORMFIELD
  3. %A_VALUE part of templating forms
  4. %QUERY
  5. renderForEdit
  6. renderForDisplay

There might be even more.

Plugins such as FlexFormPlugin are affected. It's $value returns the displayed value as does %A_VALUE (these two are synonyms); it's $origvalue returns the stored value.

DBCachePlugin is affected as well.

A lot of currently used wiki applications are affected as a consequence.

I can't easily say this is a 1.1.x change, nor something non-feature-requesto-esque.

-- MichaelDaum - 16 Apr 2013

Woops I am wrong: tmwiki is unable to return display values at all.

-- MichaelDaum - 16 Apr 2013

Ok, I really appreciate the effort you are making to solve this problem, special thanks to CrawfordCurrie. However I still think if formfield used to return the value forever the moment it was changed it was a mistake, of course. And it was not just a mistake but a irresponsibility when some people knows the problem and chooses to adopt an undocummented change with workarounds instead of addressing the problem. If there was the need of a new behavior then a new parameter should have been included to return the label, making a request so everybody knows about. So I agree with CrawfordCurrie you should return to the behavior such that the default is to display the stored value.

  • It is the logical thing to do. In every development environment when you have a table with an ID and a label, you go for the ID first, by default, one you got it you do whatever you need, including getting the label. The ID, the value in this case, is the key.

  • Its the right thing to do. End users have the right to expect the code will always produce the same result. To hear now it is not producing the same result because there was a mistake and some people adapted to it is not fair. Its unfortunate for the developers who came later and created applications with the new behavior. But you sacrifice apps using the old behaviour (returning the value) over the ones using the new behaviour you would be condemning them based on a "unlawful" move, while making the new applications to include a new param on formfield would be based on fixing an existing problem following the right procedures (there was a mistake, we are fixing it, we are sorry).

  • Forcing the change on formfield may break more applications than keeping the old behaviour. Formfield is displaying the value "since the ages of tyrannosaurus rex". It is common sense (I hope common practice) if something is working ok then dont touch it. In my case this application was running since 3 years ago without incident, that proves that applications having this problem now are good applications, that have survived working ok for years, no need to change (like dinosaurs until this meteorite arrived). I believe if everyone that is going to upgrade at some point found this error, well, they wont be happy upgrading, with a cost in the confidence in the stability of Foswiki. On the contrary to announce, "sorry people, we made a mistake, we are fixing it now, we apologize", looks like we can trust when an error is spotted it is solved right away.

-- EnriqueCadalso - 16 Apr 2013

Indeed Enrique, that's why I pushed the fix back to 1.1.9, so that it can be released as a patch. That then gives the maximum opportunity to communicate what has happened.

-- CrawfordCurrie - 16 Apr 2013

I think what you did is wrong now as forms as displayed at the bottom of the page don't display the display values anymore.

Given a select+values field with allowed values such as Automobile Industry=Category12. Then users will get to see a select box listing "Automobile Industry", which is fine up to here. However when they save the topic the topic view template lists Categroy12 ... which is wrong. It should display "Automobile Industry" as that's what a display value is supposed to be. The real formfield value of the data model underneath should remain in opaque to them.

Just found out that there is a new boolean parameter for renderForDisplay() called display. Gawd, having to say renderForDisplay to render for display using a display=>1 setting is so awkward. Why is that required?

-- MichaelDaum - 17 Apr 2013

My error - A_VALUE needs to expand to the mapped value, not the raw value.

-- CrawfordCurrie - 23 Apr 2013

 

Topic revision: r29 - 19 Nov 2013, GeorgeClark
 
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. see CopyrightStatement. Creative Commons License