[OPEN-ILS-DEV] prototype staff client, feedback/3

[steven]

This is more feedback on the prototype staff client that was recently
released.

I'm looking at the file staff/circ/checkin/app.js, which defines the
controller for the checkin view.  The view contains a single input text
box for the user to enter an item barcode; upon entry, the client sends
a request to the server to check in the item.  Below the input, there is
a a list of transactions showing the history of checkins, although its
control is delegated to another service.

The checkin controller contains a lengthy list of dependencies for what
it needs to do:

.controller('CheckinCtrl', ['$scope','$q','$modal','egStartup','checkinSvc','egNet', 'egAuth', 'orgAddrSvc','egOrg','egPCRUD','egAlertDialog','egConfirmDialog','egCheckinStrings', function () {} ]);

It seems the controller is exposed to some low-level details.  Is there an
opportunity to define more abstraction, and to simplify the controller?

1. One way is to define a data model for item barcode and item status, and to
bind the model to the server.  The item model can be as simple as

var item = { barcode: '', status: '' };

and is defined as a reusable service that can be injected into the checkin
controller (and into any other controller that needs it):

.factory('itemModel', ['$scope','$q', 'egNet', 'egAuth', 'egNotify', function () {} ]);

The checkin controller now has a slightly shorter list of dependencies:

.controller('CheckinCtrl', ['$scope', '$q', '$modal', 'egStartup', 'checkinSvc', 'itemModel', 'orgAddrSvc', 'egOrg', 'egPCRD', 'egNotify','egCheckinStrings', function () {} ]);

The itemModel is defined with a checkin() method, which is mapped to the server
request.  The server response is put into itemModel.status.  We could expand
the model to include checkout, renew, or any of the other operations which
can change item status.

The checkin controller can bind the model to the view, so that the model is
doubly bound, like so:

view <---> itemModel <---> server

The controller places a $watch on the barcode value; it also places a $watch on
the item status value:

$scope.$watch($scope.item.barcode, $scope.item.checkin);
$scope.$watch($scope.item.status, handleCheckinResponse);

2. On the view side, since the checkin template binds the input to the barcode
value, it doesn't seem necessary to use the ng-submit directive, nor to use a
<form> element, since a <div> will suffice. Because of the $watch on barcode
value, when the user changes the value, the controller will respond accordingly
by invoking the checkin() method, which is essentially a server request.

3. On the server side, handleCheckinResponse() is what is currently defined,
essentially a switch statement based on the textcode status of the server
response, but it doesn't need to do as much.  Is it necessary for the
controller to notify the user of the various non-normal statuses?  If the
itemModel is reusable by other controllers, then they will expect the same
notifications to appear, therefore, we can delegate notification to the model
itself, which is why I included egNotify as a dependency for itemModel. (Also,
I'm taking the suggestion I made in feedback/2 to use a more capable single
service in lieu of the egAlertDialog and egConfirmDialog services.)

4. Is it necessary for the controller to control the focussing and blurring of
the input text box?  As I argue in 5, probably not. This means
handleCheckinResponse() is reduced to routing the interface to different
templates (hold shelf dialog or transit dialog), updating the transactions
list, or doing checkin override.

5. The focussing and blurring of the input text box is a throttling mechanism
to prevent the user from scanning another barcode too quickly before the
current one is finished processing.  Currently, two reusable directives, called
focus-me and blur-me, are used, but the coding seems obtuse and awkward:
control variables are introduced into the $scope to allow the controller to set
boolean values, which get translated to focussing or blurring actions by the
directives.  The variables are artificial, and the controller has to attend to
the details depending on the context.  It would be better to define a single
directive that takes its cues from the state transitions of item.barcode and
item.status values. Since a directive has access to the $scope in which these
values are kept, it should be able to determine the state transitions, but I'm
unclear of the details. But I'm also not that concern, because of the next
point.

6. Is it necessary to throttle user input?  Since barcode processing proceeds
asynchronously and since egNotify, as I defined it in feedback/2, queues up
notifications and shows them to the user in a first-in, first-out basis, there
shouldn't be a problem for the user to lose track of things.  Also, we seem to
be underutilizing the transactions list.  It would be useful to colour code the
entries according to their checkin status, for example, red for checkins with
problems, and green for ones that were overridden.  We could even implement a
retry mechanism: for red scans, the user could click the list entry to have it
retry a checkin for that barcode.

In conclusion, it would be worthwhile to define reusable data models
that are bound to server requests.  Controllers need not perform server
requests directly; instead, they would inject the data models into its
$scope, install $watchers and do things that are tied to data updates,
much in the philosophy of how the controllers handle views.

(I also should add that I used sketchy fragments of angularjs constructs
that are not 100% correct.)

-- 
steven <steven.chan at bc.libraries.coop>
bc libraries co-op/sitka