Browser staff client / merge prep

Bug #1350042 reported by Bill Erickson on 2014-07-29
This bug affects 1 person
Affects Status Importance Assigned to Milestone

Bug Description

See action items:

Code is here:;a=shortlog;h=refs/heads/collab/berick/web-staff-proto

If you install this code as-is, it will not build the browser client. Some stuff will get installed, because it's in the web dir, but it won't function. The purpose of this bug is to see if the browser client can safely be installed w/o affecting existing code, so I won't go into detail about fetching dependencies and building the browser client here. I'll open a different LP for documenting and improving the build process.

For testing, the main things to look at are the bits of Evergreen proper that changed to support the browser client.

* I added a few permacrud sections to fm_IDL.xml. Changes are all used in the browser client, so they have received some amount of testing, but extra eyes won't hurt.
* Holds placement in the XUL client (via embedded browser)
* Bookbag actions in the XUL client (via embedded browser)
* Copy barcode detail link in record copy grid (via embedded browser)

Regarding the merge, it's a giant pile (349) of practically non-documented commits. Fine-grained squashing would take too long, so we can either commit as-is, with a nice big merge commit message, or squash it all down to a handful of gigantor commits.

Bill Erickson (berick) wrote :

Based on conversations in IRC, I'm going to do some heavy squashing. Will report back soon.

Bill Erickson (berick) wrote :

I've pushed a mightily squashed branch to:;a=shortlog;h=refs/heads/user/berick/web-staff-phase1-squash

It starts w/ a few misc. commits and ends with one giant scripts + templates commit.

Ben Shum (bshum) wrote :

Thanks for the squashing Bill. I'm retargeting this work to the newly created 2.7.0-beta2 milestone for further review after the websockets in OpenSRF work is tested.

Changed in evergreen:
milestone: 2.7.0-beta1 → 2.7.0-beta2
status: New → Confirmed
Bill Erickson (berick) wrote :

Rebased to master and resolved a TPAC tempate conflict. Force-pushed to same squash branch.

Bill Erickson (berick) wrote :

Also pushed a patch to add a missing dependency to the staff JS Gruntfile.

I just went through a full install on a clean system of the OpenSRF websockets branch plus this branch and was able to get the browser client running without any problems.

Note to anyone doing a manual install of the browser client dependencies... install a v.0.10.* version of Node, not the most recent v.0.11.* series, which I presume is their unstable series (and fails to function for me).

Bill Erickson (berick) wrote :

Also, for those installing the dependencies, after the dependencies are fetched, it's necessary to copy them into place so the browser can access them:

cp -r build /openils/var/web/js/ui/default/staff/

Thomas Berezansky (tsbere) wrote :

I was told to object in public on IRC over merging this into master if I have objections. I figure that this is actually the better place to object.

On a general note, at this point I cannot merge this on my dev machine, no matter what I do about setting up websockets, and not have major issues with bookbags in particular in the staff client. Regardless of whether or not people want it merged for 2.7 beta 2, "breaks existing functionality" seems like a really good reason to not merge.

And that is without objections over needing a third/fourth apache port to be opened in firewalls (and some of those may be outgoing blocks from overly paranoid groups, not incoming, and thus are harder to get holes in) to get the apparently broken parts of the regular client working after the fact without sufficient documentation to say that it is, in fact, needed. At the very least the core Evergreen README file should get a mention of that, in addition to the general need for the websocket components of OpenSRF to be installed, if not more detailed instructions.

Bill Erickson (berick) wrote :

Hi Thomas, if this code breaks bookbag functionality, then that's definitely a showstopper. The first rule of this LP is do no harm... This is the first I've heard of that issue, though. Can you please elaborate?

I'm not quite following your last paragraph. Are you saying that websockets have to be configured just to use the XUL client? Or are you simply saying the browser client / websockets needs more documentation?

Ben Shum (bshum) wrote :

Well, I tested my server without turning on the websockets portion and bookbags work fine. The only issue is with adding to temporary lists, but this appears to be a pre-existing problem in 2.6 systems regardless of the new changes.

As for my issues that I had encountered with hold placements, it turns out that I was victim to pickup library issues due to unresolved complications from bug 1167541. Once someone reminded me of that, it turns out holds work just fine as well.

No seeing any serious blocker caused by this new code presently. We can work on refinements to the documentation and other installation details post-merge, pre-release, barring any further discoveries?

Galen Charlton (gmc) wrote :

I've tested master+user/berick/web-staff-phase1-squash on top of OpenSRF 2.3.0, and did not find any regressions with placing a hold or with adding an item to a (permanent) bookbag from within the staff client. I did observe the infinite redirect issue attempting to add an item to a temporary list, but I see that in 2.6.2 as well.

Mike Rylander (mrylander) wrote :

Here's a branch that, while not changing behavior, removes some magic ("return last lvalue") in case the method becomes more complex in the future and breaks the contract with Perl that it currently depends on. It also improves readability and maintainability, IMO. It's the same as user/berick/web-staff-phase1-squash, but with one commit on top to use explicit "return"s.;a=shortlog;h=refs/heads/collab/miker/web-staff-phase1-squash-with-returns

Ben Shum (bshum) wrote :

I committed the bulk of this, but did not include Mike's last commit because I hadn't tested that far yet. Marking this bug as mostly fix committed, but we need to get some eyes on that final commit before we push it.

Changed in evergreen:
status: Confirmed → Fix Committed
Bill Erickson (berick) wrote :

Pushed some grid fixes... This fixes the show/hide all column actions, noted in an email to -general. (We may ultimately remove the actions, but if they're there, they may as well work). Also fixes the grid CSV print action.;a=shortlog;h=refs/heads/user/berick/lp1350042-grid-print-and-show-all-cols

Ben Shum (bshum) wrote :

Picked that fix to master, thanks Bill.

Jason Etheridge (phasefx) wrote :

I pushed the latest tip from Bill's branch into master after running it through the live tester. Thanks Bill

Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers