Modify

Opened 8 years ago

Closed 8 years ago

#12899 closed defect (fixed)

kis_assistant .visible rule doesn't work with TracTicketChainedFieldsPlugin fields

Reported by: Brett Johnson Owned by: Jon Ashley
Priority: normal Component: KeepInterfaceSimplePlugin
Severity: normal Keywords:
Cc: Trac Release: 1.0

Description

If I try to make a field visible or invisible, when the field is managed by TracTicketChainedFieldsPlugin, it only works on the page when is first loaded, but not if the status is changed while manipulating the ticket.

For example, the following "tcf_*" fields in a kis_assistant rule are dropdown menus, which are dynamically populated by the chained fields plugin:

tcf_platform.visible = !type in 'New Platform'
tcf_product.visible = !type in 'New Platform'
tcf_product_release.visible = !type in 'New Platform'

These fields are visible for all ticket types except "New Platform", but only if that's the type selected when the page is loaded/reloaded. If I switch types without reloading the page, the visible status of the fields does not change.

FWIW, these fields *do* change visibility when using the DynamicFieldsPlugin .visible attribute (but that plugin has other bugs, and is much more complex :) ).

Attachments (3)

trac.ini (6.5 KB) - added by Brett Johnson 8 years ago.
Anonymized Trac config file
chainedfields.json (320 bytes) - added by Brett Johnson 8 years ago.
Example JSON for chained fields plugin, which works with the trac.ini above.
parse_json.py (5.2 KB) - added by Jon Ashley 8 years ago.
Python script to convert TracTicketChainedFieldsPlugin JSON into KeepInterfaceSimplePlugin configuration

Download all attachments as: .zip

Change History (13)

comment:1 Changed 8 years ago by Jon Ashley

Thanks for the fault report; I'm looking at it now. Can you please confirm that you are using version 1.0 of KeepInterfaceSimplePlugin rather than a version from the trunk?

I tried to reproduce the fault using the version 0.2 of TracTicketChainedFieldsPlugin (from the 0.11 branch in Subversion) and version 1.0 of KeepInterfaceSimplePlugin. I used the example configuration with the different phone brand fields from TracTicketChainedFieldsPlugin and the following kis_assistant rules on an otherwise fresh Trac environment:

[kis_assistant]
tcf_brand.visible = !type in 'defect'
tcf_os.visible = !type in 'defect'
tcf_phone.visible = !type in 'defect'

Disappointingly, this didn't reproduce the issue as reported - the visibility of the fields changed as expected. However, I noticed that the function of the chained fields plugin had stopped working, whereas it was working correctly before.

I haven't looked into how TracTicketChainedFieldsPlugin works yet, but I suspect that both plugins are using the same mechanism of registering Javascript callbacks on fields that need to be updated when the controlling field changes, and they're stepping on each other's toes and overwriting each other's callback. In that scenario, the failing plugin is the one that got there first.

As a temporary workaround until I can fix this, it may be possible to set up kis_assistant rules that behave like the chained fields plugin. E.g. these rules do something very similar to the TracTicketChainedFieldsPlugin example:

[kis_assistant]
tcf_brand.visible = !type in 'New Platform'

tcf_os.available.5800 = tcf_phone == '5800' && tcf_brand == 'Nokia'
tcf_os.available.iphone = tcf_phone == 'iPhone' && tcf_brand == 'Apple'
tcf_os.available.iphone3g = tcf_phone == 'iPhone 3G' && tcf_brand == 'Apple'
tcf_os.available.iphone_both = tcf_brand == 'Apple'
tcf_os.options.5800 = Symbian,Android
tcf_os.options.iphone_both = OS X
tcf_os.options.iphone = OS
tcf_os.options.iphone3g = other
tcf_os.visible = !type in 'New Platform'

tcf_phone.available.apple = tcf_brand == 'Apple'
tcf_phone.available.nokia = tcf_brand == 'Nokia'
tcf_phone.options.apple = iPhone,iPhone 3G
tcf_phone.options.nokia = 5800
tcf_phone.visible = !type in 'New Platform'

The fields that would have been controlled by TracTicketChainedFieldsPlugin need to be made into normal option-select fields for this to work, rather than the text types that TracTicketChainedFieldsPlugin requires. There may be drawbacks to this approach: option-select fields where being empty is a valid value aren't currently handled well.

Changed 8 years ago by Brett Johnson

Attachment: trac.ini added

Anonymized Trac config file

comment:2 Changed 8 years ago by Brett Johnson

Wow, thank you for looking at it so quickly!

I was using the version from trunk, but I just reverted to the 1.0 tag, and still see the same behavior. I don't see any problem with the TracTicketChainedFieldsPlugin functionality either, in either branch. Maybe there is some other contributing factor in my configuration? I'm using the trac package in Debian stable/8/jessie (that's version 1.0.2+dfsg-2), and I do have other plugins installed/enabled (BatchMod, CustomFieldAdmin, LDAPPlugin, TracRPC, TracTags, TracWYSIWYG, FieldTooltip, TracTicketTemplate), and several other custom fields which display after the chained fields. I'll attach my config file (with project identifiers replaced with "FOO"), if that will help. (EDIT: I also attached a TracTicketChainedFieldsPlugin JSON file that will work with the fields set up in the trac.ini)

I thought about using the kis_assistant to automatically generate the menu hierarchy as you suggest, as it would be nice to remove TracTicketChainedFieldsPlugin (it doesn't look like it's maintained, as it has several years-old bug reports). I'll have to ponder this approach some more. In this instance, the chained fields JSON structure is dynamically generated from another set of JSON files which are maintained elsewhere (all I have is a set of URLs to fetch the files from). So I'd have to figure out how to automatically generate the somewhat complex kis_assistant config from that JSON, and insert it into the .ini file from a script dynamically.

Last edited 8 years ago by Brett Johnson (previous) (diff)

Changed 8 years ago by Brett Johnson

Attachment: chainedfields.json added

Example JSON for chained fields plugin, which works with the trac.ini above.

comment:3 Changed 8 years ago by Jon Ashley

In 15915:

Branch for applying fix to #12899. This changeset fixes the handling of empty options, and allows sets of options to be merged.
re #12899

comment:4 Changed 8 years ago by Jon Ashley

I've tried again with your .ini and JSON. There were a small number of syntax errors in the .ini file (closing quotes missing from all but one of the "New Platform" constants), but I expect they were introduced when you cleaned up the config file to post it here.

Still no success in reproducing the issue. I can't even reproduce getting TracTicketChainedFieldsPlugin to fail now. I think I must have been doing something wrong when that happened.

I'm using Trac 1.0.13 from Debian.

I've put a debugging statement in the version of the plugin on trunk that logs every attempt to show or hide a field to the browser console. Can you try running your site with that and see whether it sheds any light on the problem?

(Note: the version on trunk is pre-release and uses slightly different configuration syntax to the 1.0 branch. The text supplied for option set options or templates now has to be enclosed in quotes and the 'has_role' operator has gone. But you will be fine if you aren't using the 'has_role' operator, or selectable sets of options, or templates.)

comment:5 Changed 8 years ago by Jon Ashley

I have finally managed to reproduce the problem. It's quite sensitive to the order in which the fields are first manipulated after the page has loaded, which I think is why I couldn't see it before.

The cause seems to be that the KeepInterfaceSimplePlugin caches the jQuery selectors that it uses to manipulate the fields, because it knows that the fields never change after the page has loaded. Except that when TracTicketChainedFieldsPlugin is active, they do, and the cached selector stops referring to the field. The TracTicketChainedFieldsPlugin must replace the field at some point with an identical one.

I have put a fix for this issue in the keepinterfacesimpleplugin/branch/12899 area of the repository. Can you please try it out and see if it fixes the fault? If it does, I'll make a proper release 1.1.

comment:6 Changed 8 years ago by Jon Ashley

Status: newaccepted

comment:7 in reply to:  2 Changed 8 years ago by Jon Ashley

Replying to linuxturtle:

I thought about using the kis_assistant to automatically generate the menu hierarchy as you suggest, as it would be nice to remove TracTicketChainedFieldsPlugin (it doesn't look like it's maintained, as it has several years-old bug reports). I'll have to ponder this approach some more. In this instance, the chained fields JSON structure is dynamically generated from another set of JSON files which are maintained elsewhere (all I have is a set of URLs to fetch the files from). So I'd have to figure out how to automatically generate the somewhat complex kis_assistant config from that JSON, and insert it into the .ini file from a script dynamically.

I've attached a Python script that's intended to do the rule-generation and custom-field-generation part of that. It hasn't had much testing, but it seems to work on the example that came with the chained fields plugin, and with the example that you attached to this ticket.

comment:8 in reply to:  5 Changed 8 years ago by Brett Johnson

Replying to ash:

I have put a fix for this issue in the keepinterfacesimpleplugin/branch/12899 area of the repository. Can you please try it out and see if it fixes the fault? If it does, I'll make a proper release 1.1.

Wow, thank you! Sorry I didn't react over the weekend.

Yes, I can confirm that this change fixes the issue for me, and the two plugins seem to work together without conflict now.

And thanks also for the rule generation script, I'll see if I can do something similar, as I think just eliminating the chained fields plugin is a better approach anyway.

Changed 8 years ago by Jon Ashley

Attachment: parse_json.py added

Python script to convert TracTicketChainedFieldsPlugin JSON into KeepInterfaceSimplePlugin configuration

comment:9 Changed 8 years ago by Jon Ashley

Great; I'll sort it out properly at some point over the next few days. I realised earlier that the script didn't work properly if there was more than one field at the top level, so have replaced it with an updated version that doesn't have that fault.

comment:10 Changed 8 years ago by Jon Ashley

Resolution: fixed
Status: acceptedclosed

KeepInterfaceSimplePlugin version 1.1 created in revision [15934], fixing this defect.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jon Ashley.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.