improvements to flattenerfilterdialog: use autowidget better

For explanation, see my last commit in this branch, plus this
[edited quickly] conversation:

08:44 < eeevil> senator: re "the solution", I think popping the field
off the path if path.length > 1 is universally right -- if we
can teach autowidget how to give us a downstream value (by passing
it shortname) instead of, say, an aou id automagically
08:45 < eeevil> senator: which I think is what you were planning ... I
just can't think of a reason to make the stop depth
configurable
10:57 < senator> eeevil: example of when i think it's not right:
10:58 < senator> core class acp, path call_number.record.tcn_value
10:58 < senator> there may be more... likely... examples, but sometimes
you'll actually want to deal with the end field
10:59 < senator> no?
11:00 < senator> one other idea i had, instead of making the stop depth
configurable, was to pop the end off iff the last field is the selector for
its class
11:01 < senator> then it seems fairly certain that an autowidget on the
preceding class would be the right thing. if you agree that's
the above is sane, i think i could live with that instead of
configurable stop depth
11:11 < eeevil> senator: call_number.record.tcn_value is a great example
selector came to mind for me too, I like that
11:12 < senator> very good, will make that so
11:12 < eeevil> senator: however ... selector would have to match the
leaf field, right?
11:14 < eeevil> senator: maybe that's still the answer ... a way to say,
on the <th>, that the named field is the selector for the class
in this instance, and if that's not there then look for an
IDL selector, and if neither of those is true, just show the
leaf in a simple widget
11:19 < senator> eeevil: i can't say i'm following you this time re
selector and leaf field
11:19 < eeevil> senator: for instance, shortname is the selector for
aou.id, but that -^ would let you have <th field='name' selector='true'/>
(maybe) and get a dropdown of names, instead of shortnames
11:20 < senator> oh, that is taking things a step further than i had in
mind.  you can tell autofieldwidget acp.circ_lib and get a
dropdown of shortnames, or you can tell it aou.shortname (or
aou.name) and get a text widget
11:21 < senator> getting a dropdown of, say, long names would seem to
mean teaching autofieldwidget a new trick, or developing
something new, which might be nice in the long run, but maybe
more new, which might be nice in the long run, but maybe
more effort than we need right now? unless i'm missing
something
11:21 < eeevil> senator: ahh... ok, so ... with a path of
acp.circ_lib.name, you get a text field, and with acp.circ_lib.shortname
you get a dropdown of shortnames?
11:22 < senator> right. not perfect, but faster to make reality, and
maybe a user's acp.circ_lib.name column shouldn't be a
filter:true column anyway
11:22 < eeevil> +1
11:22 < eeevil> senator++
11:23 < eeevil> since we're shoving the pkey in behind the scenes, would
that end up being the filtered column?
11:24 < eeevil> or are we just telling autofieldwidget that we want the
                shortname instead of the id as the value
11:25 < senator> when building the filter from the autowidget's
selection, we'll take the display value (or whatever that
attribute is called) instead of the item identifier like we would
for pcrudfilterdialog,
11:25 < senator> so our filter result is {"circ_lib_short_name": "BR1"}
11:25 < senator> which is what flattener wants. sound good?
11:32 < eeevil> senator: sounds like it'll work perfectly

Signed-off-by: Lebbeous Fogle-Weekley <lebbeous@esilibrary.com>

diff --git a/Open-ILS/web/js/dojo/openils/widget/FlattenerFilterDialog.js b/Open-ILS/web/js/dojo/openils/widget/FlattenerFilterDialog.js
index 47bbe04..7754608 100644 (file)
--- a/Open-ILS/web/js/dojo/openils/widget/FlattenerFilterDialog.js
+++ b/Open-ILS/web/js/dojo/openils/widget/FlattenerFilterDialog.js
@@ -8,6 +8,7 @@ if (!dojo._hasResource["openils.widget.FlattenerFilterDialog"]) {
         "openils.widget.FlattenerFilterDialog",
         [openils.widget.PCrudFilterDialog], {
             "mapTerminii": null,
+            "indirect": true,
 
             "constructor": function(args) {
                 dojo.mixin(this, args);

diff --git a/Open-ILS/web/js/dojo/openils/widget/FlattenerGrid.js b/Open-ILS/web/js/dojo/openils/widget/FlattenerGrid.js
index 939cdac..ab23e04 100644 (file)
--- a/Open-ILS/web/js/dojo/openils/widget/FlattenerGrid.js
+++ b/Open-ILS/web/js/dojo/openils/widget/FlattenerGrid.js
@@ -129,31 +129,63 @@ if (!dojo._hasResource["openils.widget.FlattenerGrid"]) {
 
             /* The FlattenerStore doesn't need this, but it has at least two
              * uses: 1) FlattenerFilterDialog, 2) setting column header labels
-             * to IDL defaults. */
+             * to IDL defaults.
+             *
+             * To call these 'Terminii' can be misleading. In certain
+             * (actually probably common) cases, they won't really be the last
+             * field in a path, but the next-to-last. Read on. */
             "_calculateMapTerminii": function() {
-                function _fm_get_field_def(hint, field) {
-                    /* XXX this assumes we have the whole IDL loaded. I guess
-                     * we could teach this to work by loading classes on demand
-                     * when we don't have the whole IDL loaded. */
-                    return fieldmapper.IDL.fmclasses[hint].fields.filter(
-                        function(f) { return f.name == field; }
-                    ).shift();
+                function _fm_is_selector_for_class(hint, field) {
+                    var cl = fieldmapper.IDL.fmclasses[hint];
+                    return (cl.field_map[cl.pkey].selector == field);
                 }
 
                 function _follow_to_end(hint, path) {
+                    var last_field, last_hint;
+                    var orig_path = dojo.clone(path);
+                    var field;
+
                     while (field = path.shift()) {
-                        var field_def = _fm_get_field_def(hint, field);
-                        if (field_def["class"])
+                        /* XXX this assumes we have the whole IDL loaded. I
+                         * guess we could teach this to work by loading classes
+                         * on demand when we don't have the whole IDL loaded. */
+                        var field_def =
+                            fieldmapper.IDL.fmclasses[hint].field_map[field];
+
+                        if (field_def["class"] && path.length) {
+                            last_field = field;
+                            last_hint = hint;
+
                             hint = field_def["class"];
-                        else
-                            break;
+                        } else if (path.length) {
+                            /* There are more fields left but we can't follow
+                             * the chain via IDL any further. */
+                            throw new Error(
+                                "_calculateMapTerminii can't parse path " +
+                                orig_path + " (at " + field + ")"
+                            );
+                        } else {
+                            break;  /* keeps field defined after loop */
+                        }
+                    }
+
+                    var datatype = field_def.datatype;
+                    /* Back off the last field in the path if it's a selector
+                     * for its class, because the preceding field will be
+                     * a better thing to hand to AutoFieldWidget.
+                     */
+                    if (orig_path.length > 1 &&
+                            _fm_is_selector_for_class(hint, field)) {
+                        hint = last_hint;
+                        field = last_field;
+                        datatype = "link";
                     }
 
                     return {
                         "fmClass": hint,
                         "name": field,
                         "label": field_def.label,
-                        "datatype": field_def.datatype
+                        "datatype": datatype
                     };
                 }
 
@@ -173,6 +205,7 @@ if (!dojo._hasResource["openils.widget.FlattenerGrid"]) {
 
                     this.mapTerminii.push(terminus);
                 }
+                console.log(dojo.toJson(this.mapTerminii));
             },
 
             "_supplementHeaderNames": function() {

diff --git a/Open-ILS/web/js/dojo/openils/widget/PCrudFilterDialog.js b/Open-ILS/web/js/dojo/openils/widget/PCrudFilterDialog.js
index 47cc9e8..86e55e7 100644 (file)
--- a/Open-ILS/web/js/dojo/openils/widget/PCrudFilterDialog.js
+++ b/Open-ILS/web/js/dojo/openils/widget/PCrudFilterDialog.js
@@ -169,7 +169,7 @@ if (!dojo._hasResource['openils.widget.PCrudFilterDialog']) {
     function PCrudFilterRowManager() {
         var self = this;
 
-        this._init = function(container, field_store, fm_class) {
+        this._init = function(container, field_store, fm_class, indirect) {
             this.container = container;
             this.field_store = field_store;
             this.fm_class = fm_class;
@@ -177,6 +177,9 @@ if (!dojo._hasResource['openils.widget.PCrudFilterDialog']) {
             this.rows = {};
             this.row_index = 0;
 
+            /* 'indirect' is really for FlattenerFilterDialog */
+            this.indirect = indirect || false;
+
             this._build_table();
         };
 
@@ -470,7 +473,12 @@ if (!dojo._hasResource['openils.widget.PCrudFilterDialog']) {
         this.compile = function() {
             if (this.value_widgets) {
                 var values = this.value_widgets.map(
-                    function(widg) { return widg.getFormattedValue(); }
+                    function(widg) {
+                        return widg[
+                            self.filter_row_manager.indirect && widg.linkClass ?
+                                "getDisplayString" : "getFormattedValue"
+                            ]();
+                    }
                 );
 
                 if (!values.length) {
@@ -506,6 +514,8 @@ if (!dojo._hasResource['openils.widget.PCrudFilterDialog']) {
         [dijit.Dialog, openils.widget.AutoWidget],
         {
 
+            "indirect": false,
+
             constructor : function(args) {
                 for(var k in args)
                     this[k] = args[k];
@@ -614,7 +624,7 @@ if (!dojo._hasResource['openils.widget.PCrudFilterDialog']) {
 
                 this.filter_row_manager = new PCrudFilterRowManager(
                     dojo.create("div", {}, this.domNode),
-                    this.fieldStore, this.fmClass
+                    this.fieldStore, this.fmClass, this.indirect
                 );
 
                 this._buildButtons();