Circulation Policy Editor is not very easy to use.

Bug #800846 reported by Joseph
18
This bug affects 4 people
Affects Status Importance Assigned to Milestone
Evergreen
Incomplete
Wishlist
Unassigned

Bug Description

Problem:
Occasionally the grid is "clipped", scrollbars display strangely, the back and forward buttons display even when there are no more pages, there is no easy way to search/filter, and there is javascript in the HTML file.

Versions:
It looks like most versions are affected up to and including the current main branch in the git repo.

About the patch:
The included patch should be able to be backported and possibly fix bugs:
* 517699 Display clipped in the Circulation Policy Configuration editor
* 747402 Circulation Policy Configuration function broken
plus some of the concerns raised in 747402.

The patch has been tested on test system with sixteen records.

The git repo that the patch was taken from is at:
git://git.evergreen-ils.org/evergreen/joelewis.git
the branch is "circ_config_policy".

Thanks,
    Joseph

Revision history for this message
Joseph (joehms22) wrote :
Revision history for this message
Jason Stephenson (jstephenson) wrote :

I have a few hundred records on my test systems. I'll see if I can give this a look see this week.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

Got the following in JavaScript console while running with the attached patch on a system based on master with 271 config.circ_matrix_matchpoint entries:

Error: uncaught exception: {"hash":{"threadTrace":0,"type":"REQUEST","payload":{"hash":{"method":"open-ils.pcrud.search.ccmm","params":["be26267bc61cc35cd018d4b4e3459537",{"id":{"!=":null}},{"order_by":{"ccmm":"circ_modifier"},"limit":9000000000000000}]},"_encodehash":true,"method":null,"params":null,"serialize":null},"locale":"en-US"},"_encodehash":true,"threadTrace":null,"type":null,"payload":null,"locale":null,"serialize":null}

and the policy editor never populates, of course.

pen-ils.pcrud 2011-06-22 16:45:34 [INFO:5423:osrf_application.c:1040:130877528654856] CALL: open-ils.pcrud open-ils.pcrud.search.ccmm "be26267bc61cc35cd018d4b4e3459537",{"id":{"!=":null}},{"order_by":{"ccmm":"circ_modifier"},"limit":9000000000000000}
open-ils.pcrud 2011-06-22 16:45:34 [ERR :5423:oils_sql.c:5512:130877528654856] open-ils.pcrud: Error retrieving config::circ_matrix_matchpoint with query [SELECT "ccmm".id, "ccmm".is_renewal, "ccmm".active, "ccmm".org_unit, "ccmm".copy_circ_lib, "ccmm".copy_owning_lib, "ccmm".user_home_ou, "ccmm".grp, "ccmm".circ_modifier, "ccmm".marc_type, "ccmm".marc_form, "ccmm".marc_bib_level, "ccmm".marc_vr_format, "ccmm".ref_flag, "ccmm".juvenile_flag, "ccmm".usr_age_lower_bound, "ccmm".usr_age_upper_bound, "ccmm".item_age, "ccmm".circulate, "ccmm".duration_rule, "ccmm".recurring_fine_rule, "ccmm".max_fine_rule, "ccmm".hard_due_date, "ccmm".renewals, "ccmm".grace_period, "ccmm".script_test, "ccmm".total_copy_hold_ratio, "ccmm".available_copy_hold_ratio FROM config.circ_matrix_matchpoint AS "ccmm" WHERE "ccmm".id IS NOT NULL ORDER BY circ_modifier LIMIT -889552896;]: 0 ERROR: LIMIT must not be negative

Got the above in osrfsys.log.

It looks like the limit is going negative on my system. However, I don't see where in your patch the limit is being set. Rather bizarre.

On an unpatched system, running code that is otherwise identical to my dev server, I don't get the above error.

I applied the patch rather than merge your git branch because I'm trying to keep my dev environment as close to what we're running in production as possible at the moment. I might merge the branch and give that a try later to see if the behavior continues.

Revision history for this message
Joseph (joehms22) wrote : Re: [Bug 800846] Re: Circulation Policy Editor is not very easy to use.
Download full text (4.2 KiB)

I think the git and the patch should be exactly the same; I wonder if it is
due to attempting to use the largest int as the number of items per page?
Maybe there is some kind of sign change (does Javascript have signed and
unsigned?)

--
Public Key: [0xF8462E1593141C16]<http://pgp.mit.edu:11371/pks/lookup?op=get&search=0xF8462E1593141C16>

For a successful technology, reality must take precedence over public
relations, for nature cannot be fooled.
  - Richard Feynman

On Wed, Jun 22, 2011 at 3:00 PM, Jason Stephenson <<email address hidden>
> wrote:

> Got the following in JavaScript console while running with the attached
> patch on a system based on master with 271 config.circ_matrix_matchpoint
> entries:
>
> Error: uncaught exception:
> {"hash":{"threadTrace":0,"type":"REQUEST","payload":{"hash":{"method
> ":"open-
>
> ils.pcrud.search.ccmm","params":["be26267bc61cc35cd018d4b4e3459537",{"id":{"!=":null}},{"order_by":{"ccmm":"circ_modifier"},"limit":9000000000000000}]},"_encodehash":true,"method":null,"params":null,"serialize":null},"locale
> ":"en-
>
> US"},"_encodehash":true,"threadTrace":null,"type":null,"payload":null,"locale":null,"serialize":null}
>
> and the policy editor never populates, of course.
>
> pen-ils.pcrud 2011-06-22 16:45:34
> [INFO:5423:osrf_application.c:1040:130877528654856] CALL: open-ils.pcrud
> open-ils.pcrud.search.ccmm
> "be26267bc61cc35cd018d4b4e3459537",{"id":{"!=":null}},{"order_by":{"ccmm":"circ_modifier"},"limit":9000000000000000}
> open-ils.pcrud 2011-06-22 16:45:34 [ERR
> :5423:oils_sql.c:5512:130877528654856] open-ils.pcrud: Error retrieving
> config::circ_matrix_matchpoint with query [SELECT "ccmm".id,
> "ccmm".is_renewal, "ccmm".active, "ccmm".org_unit, "ccmm".copy_circ_lib,
> "ccmm".copy_owning_lib, "ccmm".user_home_ou, "ccmm".grp,
> "ccmm".circ_modifier, "ccmm".marc_type, "ccmm".marc_form,
> "ccmm".marc_bib_level, "ccmm".marc_vr_format, "ccmm".ref_flag,
> "ccmm".juvenile_flag, "ccmm".usr_age_lower_bound,
> "ccmm".usr_age_upper_bound, "ccmm".item_age, "ccmm".circulate,
> "ccmm".duration_rule, "ccmm".recurring_fine_rule, "ccmm".max_fine_rule,
> "ccmm".hard_due_date, "ccmm".renewals, "ccmm".grace_period,
> "ccmm".script_test, "ccmm".total_copy_hold_ratio,
> "ccmm".available_copy_hold_ratio FROM config.circ_matrix_matchpoint AS
> "ccmm" WHERE "ccmm".id IS NOT NULL ORDER BY circ_modifier LIMIT
> -889552896;]: 0 ERROR: LIMIT must not be negative
>
> Got the above in osrfsys.log.
>
> It looks like the limit is going negative on my system. However, I don't
> see where in your patch the limit is being set. Rather bizarre.
>
> On an unpatched system, running code that is otherwise identical to my
> dev server, I don't get the above error.
>
> I applied the patch rather than merge your git branch because I'm trying
> to keep my dev environment as close to what we're running in production
> as possible at the moment. I might merge the branch and give that a try
> later to see if the behavior continues.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/800846
>
> Title:
> Circulation Policy Editor is not very easy to use.
>
> Status...

Read more...

Revision history for this message
Dan Wells (dbw2) wrote :

Jason, look for this code to change the limit, I think:

displayLimit='9e15'

Not much else to say, other than that's one darn big number.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

I changed the patch to have

displayLimit='2e9' which is 2 billion or 200 million or something in that area.

The list displays, but the staff client window goes gray and stops responding very soon after the data show up.

I am running Mozilla XULRunner 1.9.2.18 - 20110615082502

On Ubuntu 10.04.2 (Lucid) 64-bit.

No error messages in osrfsys.log.

I'll have to try again with the JS Console and/of Venkman open.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

Neither JS Console nor Venkman have proved very helpful so far.

I do eventually get a dialog asking if I want to stop a long-running script and it offers this as the offending script:

Script: http://jasondev.mvlcstaff.org/js/dojo/dojo/dojo.js:16

I'll try some more with Venkman later, but as soon as the staff client freezes, Venkman freezes, too.

And just to clarify, I'm doing this on an Ubuntu workstation. Haven't tried on Windows or Mac OS X.

Revision history for this message
Joseph (joehms22) wrote :

I looked through the code in AutoGrid and can't find any loops there, although I think there might be one in PermaCrud that is trying to go over 2e9 records, possibly rather than a for-each?

Anyhow, I think I fixed it by removing the sort-of kludge of inputting a ridiculously high number. Now you can put in null and that will cut out the parts of the code which limit the query to what used to be 2e9 results so all results are returned instead. This also fixes a future bug and a message to the mailing list about their rows not displaying when they have 2,000,000,001 of them :P

Revision history for this message
Jason Stephenson (jstephenson) wrote :

The fix_2 patch is better, but I still get a hang. At least this time it stops the script when asked to.

The scrolling is a bit slow and jerky after that. That said, it does appear to be an improvement over what was there.

I think others should have a look at this patch, Ben @ Bibliomation perhaps?

I may have something messed up on my dev system and I'm pretty sure that Bibliomation has more circ matrix matchpoints than we do. They could provide a better test on their training system probably.

I should probably try this on our training system for that matter. It may be in better shape than my development machine.

Thanks for the work, Joe. It does look like things are heading in a better direction.

Changed in evergreen:
status: New → In Progress
importance: Undecided → Wishlist
Revision history for this message
Jason Stephenson (jstephenson) wrote :

Looks like Joe already has a git branch for this. Perhaps it should be more prominent in the bug description and the pullrequest tag added?

tags: added: pullrequest
tags: removed: pullrequest
Revision history for this message
Ben Shum (bshum) wrote :

Per Dyrcona's request, I'll take a look at seeing how this code behaves with our systems. We do have quite a few circ rules in our matrix, so it'll be interesting to see...

no longer affects: evergreen/2.0
no longer affects: evergreen/2.1
Revision history for this message
Jason Stephenson (jstephenson) wrote :

I'm setting the status of this bug to incomplete because that appears to be its current status. If someone is working on this, please change the status and assign it to yourself, thanks.

Ben Shum (bshum)
no longer affects: evergreen/master
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.