From 3bb5bfac1882277b46bf7b67b4f37504f843c4fb Mon Sep 17 00:00:00 2001 From: Dan Wells Date: Tue, 17 Apr 2012 16:02:49 -0400 Subject: [PATCH] Limit excess serial data fetching Overfetching and duplication of data in the interface is both inefficient and bug-inducing. This reduces a couple big offenders. First, in the items tab, we will store distribution and subscription data separately, rather than duplicating for every row. Second, we will only redraw rows as needed rather than refresh the whole list so often. Finally, we no longer need to lookup call numbers separately, as they are included in the one-time distribution fetch. Clean up and refine serial note support, part 1: This commit fixes a number of minor problems with serial notes: 1) Serial notes are currently returned in "random" (database) order. Adding a create_date sort is a sensible default. 2) If you have many notes, they start to display in a squashed and unreadable fashion. Switching from a groupbox to a styled vbox provides a simple workaround. 3) It is currently impossible to display newlines in notes. We can deal with this by changing the display style. Clean up and refine serial note support, part 2: Because of the way our notes are being rendered, a handful of special XML characters can break the note interface when editing. These characters are now properly encoded as entities. Also, editing of newlines presents a similar issue with different consequences, and it is handled similarly but separately. Clean up and refine serial note support, part 3: Add needed menu entries for invoking the serial notes editor from the list in the Items tab. Signed-off-by: Dan Wells Signed-off-by: Lebbeous Fogle-Weekley --- .../src/perlmods/lib/OpenILS/Application/Serial.pm | 14 +- Open-ILS/web/opac/locale/en-US/lang.dtd | 10 +- .../server/locale/en-US/serial.properties | 1 + .../xul/staff_client/server/serial/manage_items.js | 202 +++++++++++++++++---- .../staff_client/server/serial/manage_items.xul | 6 + Open-ILS/xul/staff_client/server/serial/notes.xul | 22 ++- .../staff_client/server/serial/serctrl_main.xul | 3 + 7 files changed, 215 insertions(+), 43 deletions(-) diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm index 8333f3b3d3..377069dcf4 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Serial.pm @@ -65,6 +65,11 @@ my %MFHD_TAGS_BY_NAME = ( $MFHD_NAMES[0] => '853', $MFHD_NAMES[1] => '854', $MFHD_NAMES[2] => '855'); my $_strp_date = new DateTime::Format::Strptime(pattern => '%F'); +my %FM_NAME_TO_ID = ( + 'subscription' => 'ssub', + 'distribution' => 'sdist', + 'item' => 'sitem' + ); # helper method for conforming dates to ISO8601 sub _cleanse_dates { @@ -344,7 +349,7 @@ sub fleshed_serial_item_retrieve_batch { "open-ils.cstore.direct.serial.item.search.atomic", { id => $ids }, { flesh => 2, - flesh_fields => {sitem => [ qw/issuance creator editor stream unit notes/ ], sstr => ["distribution"], sunit => ["call_number"], siss => [qw/creator editor subscription/]} + flesh_fields => {sitem => [ qw/issuance creator editor stream unit notes/ ], sunit => ["call_number"], siss => [qw/creator editor subscription/]} }); } @@ -1947,18 +1952,19 @@ sub fetch_notes { my $id = $$args{object_id}; my $authtoken = $$args{authtoken}; + my $order_by = $$args{order_by} || 'create_date'; my( $r, $evt); if( $$args{pub} ) { return $U->cstorereq( 'open-ils.cstore.direct.serial.'.$type.'_note.search.atomic', - { $type => $id, pub => 't' } ); + { $type => $id, pub => 't' }, {'order_by' => {$FM_NAME_TO_ID{$type}.'n' => $order_by}} ); } else { # FIXME: restore perm check # ( $r, $evt ) = $U->checksesperm($authtoken, 'VIEW_COPY_NOTES'); # return $evt if $evt; return $U->cstorereq( - 'open-ils.cstore.direct.serial.'.$type.'_note.search.atomic', {$type => $id} ); + 'open-ils.cstore.direct.serial.'.$type.'_note.search.atomic', {$type => $id}, {'order_by' => {$FM_NAME_TO_ID{$type}.'n' => $order_by}} ); } return undef; @@ -2476,7 +2482,7 @@ sub fleshed_serial_distribution_retrieve_batch { "open-ils.cstore.direct.serial.distribution.search.atomic", { id => $ids }, { flesh => 1, - flesh_fields => {sdist => [ qw/ holding_lib receive_call_number receive_unit_template bind_call_number bind_unit_template streams / ]} + flesh_fields => {sdist => [ qw/ holding_lib receive_call_number receive_unit_template bind_call_number bind_unit_template streams notes / ]} }); } diff --git a/Open-ILS/web/opac/locale/en-US/lang.dtd b/Open-ILS/web/opac/locale/en-US/lang.dtd index e9385ddb22..fd3e752b38 100644 --- a/Open-ILS/web/opac/locale/en-US/lang.dtd +++ b/Open-ILS/web/opac/locale/en-US/lang.dtd @@ -1730,10 +1730,16 @@ - + - + + + + + + + diff --git a/Open-ILS/xul/staff_client/server/locale/en-US/serial.properties b/Open-ILS/xul/staff_client/server/locale/en-US/serial.properties index 30f348f56f..917dd682c7 100644 --- a/Open-ILS/xul/staff_client/server/locale/en-US/serial.properties +++ b/Open-ILS/xul/staff_client/server/locale/en-US/serial.properties @@ -66,6 +66,7 @@ staff.serial.manage_dists.delete_sstr.title=Delete Streams? staff.serial.manage_dists.delete_sstr.override=Override Delete Failure? Doing so will delete all attached items as well! staff.serial.manage_items.subscriber.label=Subscriber staff.serial.manage_items.holder.label=Holder +staff.serial.manage_items.notes_column.label=Notes (Item/Dist/Sub) staff.serial.manage_subs.add.error=error adding object in manage_subs.js: staff.serial.manage_subs.delete.error=error deleting object in manage_subs.js: staff.serial.manage_subs.delete_scap.confirm=Are you sure you would like to delete this caption and pattern? diff --git a/Open-ILS/xul/staff_client/server/serial/manage_items.js b/Open-ILS/xul/staff_client/server/serial/manage_items.js index ec562043a2..5e5e449c54 100644 --- a/Open-ILS/xul/staff_client/server/serial/manage_items.js +++ b/Open-ILS/xul/staff_client/server/serial/manage_items.js @@ -17,8 +17,11 @@ serial.manage_items = function (params) { serial.manage_items.prototype = { 'list_sitem_map' : {}, + 'sdist_map' : {}, + 'ssub_map' : {}, + 'row_map' : {}, - 'set_sdist_ids' : function () { + 'retrieve_ssubs_and_sdists' : function () { var obj = this; try { @@ -37,11 +40,41 @@ serial.manage_items.prototype = { if (robj != null) { if (typeof robj.ilsevent != 'undefined') throw(robj); obj.sdist_ids = robj.length ? robj : [robj]; + // now get actual sdist and ssub objects + robj = obj.network.simple_request( + 'FM_SDIST_FLESHED_BATCH_RETRIEVE.authoritative', + [ obj.sdist_ids ] + ); + if (robj != null) { + if (typeof robj.ilsevent != 'undefined') throw(robj); + robj = robj.length ? robj : [robj]; + for (var i = 0; i < robj.length; i++) { + obj.sdist_map[robj[i].id()] = robj[i]; + } + } + robj = obj.network.request( + 'open-ils.pcrud', + 'open-ils.pcrud.id_list.ssub', + [ ses(), {"+sdist" : {"id" : obj.sdist_ids}}, {"join":"sdist"} ] + ); + var ssub_ids = robj.length ? robj : [robj]; + robj = obj.network.simple_request( + 'FM_SSUB_FLESHED_BATCH_RETRIEVE.authoritative', + [ ssub_ids ] + ); + if (robj != null) { + if (typeof robj.ilsevent != 'undefined') throw(robj); + robj = robj.length ? robj : [robj]; + for (var i = 0; i < robj.length; i++) { + obj.ssub_map[robj[i].id()] = robj[i]; + } + } } else { obj.sdist_ids = []; } + } catch(E) { - obj.error.standard_unexpected_error_alert('set_sdist_ids failed!',E); + obj.error.standard_unexpected_error_alert('retrieve_ssubs_and_sdists failed!',E); } }, @@ -90,7 +123,7 @@ serial.manage_items.prototype = { //if (document.getElementById('serial_item_refresh_button')) document.getElementById('serial_item_refresh_button').focus(); obj.save_settings(); // get latest sdist id list based on library drowdown - obj.set_sdist_ids(); + obj.retrieve_ssubs_and_sdists(); obj.refresh_list('main'); obj.refresh_list('workarea'); }, @@ -138,7 +171,7 @@ serial.manage_items.prototype = { obj.build_menus(); obj.set_sunit($('serial_items_current_sunit').getAttribute('sunit_id'), $('serial_items_current_sunit').getAttribute('sunit_label'), $('serial_items_current_sunit').getAttribute('sdist_id'), $('serial_items_current_sunit').getAttribute('sstr_id')); - obj.set_sdist_ids(); + //obj.retrieve_ssubs_and_sdists(); obj.init_lists(); var mode_radio_group = $('serial_manage_items_mode'); @@ -207,7 +240,7 @@ serial.manage_items.prototype = { spawn_sitem_editor( { 'sitem_ids' : list, 'do_edit' : 1 } ); - obj.refresh_list(obj.selected_list); + obj.refresh_rows(list); } catch(E) { obj.error.standard_unexpected_error_alert(document.getElementById('catStrings').getString('staff.cat.copy_browser.edit_items.error'),E); @@ -277,7 +310,7 @@ serial.manage_items.prototype = { var robj = obj.network.request( 'open-ils.serial', 'open-ils.serial.item.fleshed.batch.update', - [ ses(), list, true ], + [ ses(), list ], null, { 'title' : document.getElementById('catStrings').getString('staff.cat.copy_browser.delete_items.override'), @@ -375,7 +408,7 @@ serial.manage_items.prototype = { } var prompt_text; if (obj.current_sunit_id == -1) { - prompt_text = 'for '+item.issuance().label()+ ' from Distribution: '+item.stream().distribution().label()+'/'+item.stream().id()+':'; + prompt_text = 'for '+item.issuance().label()+ ' from Distribution: '+obj.sdist_map[item.stream().distribution()].label()+'/'+item.stream().id()+':'; } else { // must be -2 prompt_text = 'for the new unit:'; } @@ -407,25 +440,18 @@ serial.manage_items.prototype = { barcodes[item.id()] = barcode; // now call numbers - if (typeof call_numbers_by_siss_and_sdist[item.issuance().id() + '@' + item.stream().distribution().id()] == 'undefined') { + if (typeof call_numbers_by_siss_and_sdist[item.issuance().id() + '@' + item.stream().distribution()] == 'undefined') { var default_cn = 'DEFAULT'; // if they defined a *_call_number, honor it as the default - var preset_cn_id = item.stream().distribution()[mode + '_call_number'](); - if (preset_cn_id) { - var preset_default_cn = obj.network.request( - 'open-ils.pcrud', - 'open-ils.pcrud.retrieve.acn', - [ ses(), preset_cn_id ] - ); - if (preset_default_cn) { - default_cn = preset_default_cn.label(); - } + var preset_cn = obj.sdist_map[item.stream().distribution()][mode + '_call_number'](); + if (preset_cn) { + default_cn = preset_cn.label(); } else { // for now, let's default to the last created call number if there is one var acn_list = obj.network.request( 'open-ils.pcrud', 'open-ils.pcrud.search.acn', - [ ses(), {"record" : obj.docid, "owning_lib" : item.stream().distribution().holding_lib(), "deleted" : 'f' }, {"order_by" : {"acn" : "create_date DESC"}, "limit" : "1" } ] + [ ses(), {"record" : obj.docid, "owning_lib" : obj.sdist_map[item.stream().distribution()].holding_lib().id(), "deleted" : 'f' }, {"order_by" : {"acn" : "create_date DESC"}, "limit" : "1" } ] ); if (acn_list) { @@ -442,10 +468,10 @@ serial.manage_items.prototype = { call_number = 'DEFAULT'; //TODO: real default by setting } call_numbers[item.id()] = call_number; - call_numbers_by_siss_and_sdist[item.issuance().id() + '@' + item.stream().distribution().id()] = call_number; + call_numbers_by_siss_and_sdist[item.issuance().id() + '@' + item.stream().distribution()] = call_number; } else { // we have already seen this same issuance and distribution combo, so use the same call number - call_numbers[item.id()] = call_numbers_by_siss_and_sdist[item.issuance().id() + '@' + item.stream().distribution().id()]; + call_numbers[item.id()] = call_numbers_by_siss_and_sdist[item.issuance().id() + '@' + item.stream().distribution()]; } if (obj.current_sunit_id == -2) { @@ -468,7 +494,7 @@ serial.manage_items.prototype = { obj.current_sunit_id = robj.new_unit_id; } - obj.rebuild_current_sunit(list[0].stream().distribution().label(), list[0].stream().distribution().id(), list[0].stream().id()); + obj.rebuild_current_sunit(obj.sdist_map[list[0].stream().distribution()].label(), list[0].stream().distribution(), list[0].stream().id()); obj.refresh_list('main'); obj.refresh_list('workarea'); @@ -498,10 +524,39 @@ serial.manage_items.prototype = { } } ], - + 'cmd_view_sitem_notes' : [ + ['command'], + function() { + try { + obj.view_notes('sitem'); + } catch(E) { + obj.error.standard_unexpected_error_alert('cmd_view_sitem_notes failed!',E); + } + } + ], + 'cmd_view_sdist_notes' : [ + ['command'], + function() { + try { + obj.view_notes('sdist'); + } catch(E) { + obj.error.standard_unexpected_error_alert('cmd_view_sdist_notes failed!',E); + } + } + ], + 'cmd_view_ssub_notes' : [ + ['command'], + function() { + try { + obj.view_notes('ssub'); + } catch(E) { + obj.error.standard_unexpected_error_alert('cmd_view_ssub_notes failed!',E); + } + } + ], 'cmd_items_print' : [ ['command'], function() { obj.items_print(obj.selected_list); } ], 'cmd_items_export' : [ ['command'], function() { obj.items_export(obj.selected_list); } ], - 'cmd_refresh_list' : [ ['command'], function() { obj.set_sdist_ids(); obj.refresh_list('main'); obj.refresh_list('workarea'); } ] + 'cmd_refresh_list' : [ ['command'], function() { obj.retrieve_ssubs_and_sdists(); obj.refresh_list('main'); obj.refresh_list('workarea'); } ] } } ); @@ -663,7 +718,7 @@ serial.manage_items.prototype = { obj.mode = mode; } - obj.set_sdist_ids(); + obj.retrieve_ssubs_and_sdists(); if (mode == 'receive' || mode == 'advanced_receive') { $('serial_workarea_mode_label').value = 'Recently Received'; @@ -736,6 +791,7 @@ serial.manage_items.prototype = { var sitem = robj[0]; obj.list_sitem_map[sitem.id()] = sitem; row.my.sitem = sitem; + row.my.parent_obj = obj; //params.treeitem_node.setAttribute( 'retrieve_id', js2JSON({'copy_id':copy_id,'circ_id':row.my.circ.id(),'barcode':row.my.acp.barcode(),'doc_id': ( row.my.record ? row.my.record.id() : null ) }) ); params.treeitem_node.setAttribute( 'retrieve_id', js2JSON({'sitem_id':sitem.id()}) ); dump('dumping... ' + js2JSON(obj.list_sitem_map[sitem.id()])); @@ -818,6 +874,28 @@ serial.manage_items.prototype = { obj.retrieve(list_name); }, + // accepts a list of ids or a list of objects + 'refresh_rows' : function(list) { + var obj = this; + + var id_list; + + if (typeof list[0] == 'object') { + id_list = util.functional.map_list( + list, + function(o) { + return o.id() + } + ); + } else { + id_list = list; + } + + for (var i = 0; i < id_list.length; i++) { + obj.lists[obj.selected_list].refresh_row(obj.row_map[id_list[i]]); + } + }, + 'retrieve' : function(list_name) { var obj = this; var list = obj.lists[list_name]; @@ -856,7 +934,8 @@ serial.manage_items.prototype = { } for (i = 0; i < robj.length; i++) { - list.append( { 'row' : { 'my' : { 'sitem_id' : robj[i] } }, 'to_bottom' : true, 'no_auto_select' : true } ); + var nparams = list.append( { 'row' : { 'my' : { 'sitem_id' : robj[i] } }, 'to_bottom' : true, 'no_auto_select' : true } ); + obj.row_map[robj[i]] = nparams; } }, @@ -885,6 +964,66 @@ serial.manage_items.prototype = { obj.refresh_list('main'); obj.refresh_list('workarea'); } + }, + + 'view_notes' : function(type) { + var obj = this; + + if (!obj.retrieve_ids || obj.retrieve_ids.length == 0) return; + + var object_id_fn; + var function_type; + var object_type; + var constructor; + + switch(type) { + case 'sitem': + object_id_fn = function(item) { return item.id() }; + title_fn = function(item) { return fieldmapper.IDL.fmclasses.sitem.field_map.id.label + ' ' + item.id() }; + function_type = 'SIN'; + object_type = 'item'; + constructor = sin; + break; + case 'sdist': + object_id_fn = function(item) { return item.stream().distribution() }; + title_fn = function(item) { + var sdist_id = object_id_fn(item); + return obj.sdist_map[sdist_id].label() + + ' -- ' + obj.sdist_map[sdist_id].holding_lib().shortname() + + ' (' + fieldmapper.IDL.fmclasses.sdist.field_map.id.label + ' ' + sdist_id + ')' + }; + function_type = 'SDISTN'; + object_type = 'distribution'; + constructor = sdistn; + break; + case 'ssub': + object_id_fn = function(item) { return item.issuance().subscription().id() }; + title_fn = function(item) { + var ssub_id = object_id_fn(item); + return obj.ssub_map[ssub_id].owning_lib().shortname() + + ' (' + fieldmapper.IDL.fmclasses.ssub.field_map.id.label + ' ' + ssub_id + ')' + }; + function_type = 'SSUBN'; + object_type = 'subscription'; + constructor = ssubn; + break; + default: + return; + } + + var seen_ids = {}; + for (var i = 0; i < obj.retrieve_ids.length; i++) { + var item = obj.list_sitem_map[obj.retrieve_ids[i].sitem_id]; + var obj_id = object_id_fn(item); + if (seen_ids[obj_id]) continue; + JSAN.use('util.window'); var win = new util.window(); + win.open( + urls.XUL_SERIAL_NOTES, + '','chrome,resizable,modal', + { 'object_id' : obj_id, 'function_type' : function_type, 'object_type' : object_type, 'constructor' : constructor, 'title' : $('serialStrings').getString('staff.serial.'+type+'_editor.notes') + ' -- ' + title_fn(item) } + ); + seen_ids[obj_id] = 1; + } } } @@ -919,15 +1058,16 @@ function item_columns(modify,params) { 'primary' : false, 'hidden' : false, 'persist' : 'hidden width ordinal', - 'render' : function(my) { return my.sitem.stream().distribution().label(); } - }, { + 'render' : function(my) { return my.parent_obj.sdist_map[my.sitem.stream().distribution()].label(); } + }, + { 'id' : 'distribution_ou', 'label' : $('serialStrings').getString('staff.serial.manage_items.holder.label'), 'flex' : 1, 'primary' : false, 'hidden' : false, 'persist' : 'hidden width ordinal', - 'render' : function(my) { return data.hash.aou[ my.sitem.stream().distribution().holding_lib() ].shortname(); } + 'render' : function(my) { return my.parent_obj.sdist_map[my.sitem.stream().distribution()].holding_lib().shortname(); } }, { 'id' : 'stream_id', @@ -967,11 +1107,11 @@ function item_columns(modify,params) { }, { 'id' : 'notes', - 'label' : 'Notes', + 'label' : $('serialStrings').getString('staff.serial.manage_items.notes_column.label'), 'flex' : 1, 'primary' : false, 'hidden' : false, - 'render' : function(my) { return my.sitem.notes().length; }, + 'render' : function(my) { return my.sitem.notes().length + ' / ' + my.parent_obj.sdist_map[my.sitem.stream().distribution()].notes().length + ' / ' + my.parent_obj.ssub_map[my.sitem.issuance().subscription().id()].notes().length; }, 'persist' : 'hidden width ordinal' }, { diff --git a/Open-ILS/xul/staff_client/server/serial/manage_items.xul b/Open-ILS/xul/staff_client/server/serial/manage_items.xul index c5bedca72a..b8f811522b 100644 --- a/Open-ILS/xul/staff_client/server/serial/manage_items.xul +++ b/Open-ILS/xul/staff_client/server/serial/manage_items.xul @@ -43,6 +43,9 @@ vim:noet:sw=4:ts=4: + + + @@ -67,6 +70,9 @@ vim:noet:sw=4:ts=4: + + + diff --git a/Open-ILS/xul/staff_client/server/serial/notes.xul b/Open-ILS/xul/staff_client/server/serial/notes.xul index 895b91368d..386680258b 100644 --- a/Open-ILS/xul/staff_client/server/serial/notes.xul +++ b/Open-ILS/xul/staff_client/server/serial/notes.xul @@ -58,6 +58,11 @@ g.function_type = xul_param('function_type',{'modal_xulG':true}); g.constructor = xul_param('constructor',{'modal_xulG':true}); + var window_title = xul_param('title',{'modal_xulG':true}); + if (window_title) { + try { document.title = window_title; } catch(E) {} + } + refresh(); } catch(E) { @@ -171,6 +176,10 @@ } + function xml_encode(str) { + return str.replace(/&/g, '&').replace(//g, '>').replace(/"/g, '"').replace(/'/g, '''); + } + function new_note(index) { var public = false; var title = ''; @@ -182,8 +191,8 @@ if (typeof index != 'undefined') { edit_mode = true; public = get_bool(g.notes[index].pub()); - title = g.notes[index].title(); - value = g.notes[index].value(); + title = xml_encode(g.notes[index].title()); + value = xml_encode(g.notes[index].value()); label_text = $('serialStrings').getString('staff.serial.notes.edit_note.label'); button_accesskey = $('serialStrings').getString('staff.serial.notes.edit_note.accesskey'); } else { @@ -193,6 +202,7 @@ try { netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect UniversalBrowserWrite"); + value = value.replace(/\n/g, " "); // preserve newlines var xml = ' \ \ \ @@ -247,16 +257,16 @@ diff --git a/Open-ILS/xul/staff_client/server/serial/serctrl_main.xul b/Open-ILS/xul/staff_client/server/serial/serctrl_main.xul index 444879405a..cde4abecca 100644 --- a/Open-ILS/xul/staff_client/server/serial/serctrl_main.xul +++ b/Open-ILS/xul/staff_client/server/serial/serctrl_main.xul @@ -83,6 +83,9 @@ vim:noet:sw=4:ts=4: + + + -- 2.11.0