Bug 4437

Summary: acq.js uses XMLHttpRequest() directly; should use jQuery
Product: Koha Reporter: Galen Charlton <gmcharlt>
Component: AcquisitionsAssignee: Hugh Davenport <hugh>
Status: Failed QA --- QA Contact: Bugs List <koha-bugs>
Severity: minor    
Priority: P5 - low CC: hugh, mtompset, veron
Version: master   
Hardware: All   
OS: All   
URL: ./koha-tmpl/intranet-tmpl/prog/en/js/acq.js
Change sponsored?: --- Patch complexity: Small patch
Who signed the patch off: Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 9412    
Attachments: bug 4437 Change XMLHttpRequest calls to jQuery.ajax
bug 4437 Change XMLHttpRequest calls to jQuery.ajax
Bug 4437: acq.js uses XMLHttpRequest() directly; should use jQuery

Description Chris Cormack 2010-05-21 01:27:10 UTC


---- Reported by gmcharlt@gmail.com 2010-05-03 16:13:31 ----

acq.js uses XMLHttpRequest() directly to a web service call, which is more prone to errors and web browser compatibility issues.  It should the jQuery AJAX API instead, both to avoid such issues and to be consistent with the other JavaScript in Koha.



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 01:27 UTC  ---

This bug was previously known as _bug_ 4437 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=4437

Actual time not defined. Setting to 0.0
Setting qa contact to the default for this product.
   This bug either had no qa contact or an invalid one.

Comment 1 Owen Leonard 2013-01-09 14:10:09 UTC
Issue still exists in master.
Comment 2 Hugh Davenport 2013-01-17 01:08:02 UTC
I'm working on this now
Comment 3 Hugh Davenport 2013-01-17 03:15:42 UTC Comment hidden (obsolete)
Comment 4 Hugh Davenport 2013-01-17 03:17:57 UTC
That file also has a lot of other chunks of code using generic javascript, which should use jQuery when possible, so I reckon this bug should be kept open for that purpose.

Cheers,

Hugh
Comment 5 Hugh Davenport 2013-01-17 04:43:22 UTC
Created attachment 14649 [details] [review]
bug 4437 Change XMLHttpRequest calls to jQuery.ajax

The acq.js file used a few calls to XMLHttpRequest, which have been
changed to use jQuery.ajax instead

Signed-off-by: Hugh Davenport <hugh@davenport.net.nz>
Comment 6 Owen Leonard 2013-02-05 17:37:42 UTC
Can you please add a test plan for this? acq.js is modified in several places, so I assume several different functions are being affected. A test plan will ensure that we test all the changes properly.
Comment 7 Hugh Davenport 2013-07-05 21:12:14 UTC
Hey Owen,

What would the format of a test plan be?

Cheers,

Hugh
Comment 8 Owen Leonard 2013-08-08 13:25:37 UTC
A test plan is a simple description of the steps one would take to confirm that a patch works. Usually this means confirming that a patch fixes a particular problem and doesn't cause other problems. Is there a real-world case where this fixes a browser error? If not, you should describe how to test each affected page/action to confirm that the patch hasn't caused any problems.
Comment 9 Marc Véron 2016-08-22 08:38:58 UTC
Sill valid?
Comment 10 M. Tompsett 2017-05-27 02:55:45 UTC
(In reply to Marc Véron from comment #9)
> Sill valid?

Yes.
$ git grep -l XMLHttpRequest | grep acq
Comment 11 M. Tompsett 2017-05-27 03:00:57 UTC
Doesn't apply because fetchSortDropbox no longer exists.
The other three functional changes should still be possible.
Attempting resurrection.
Comment 12 M. Tompsett 2017-05-27 03:27:24 UTC
Created attachment 63756 [details] [review]
Bug 4437: acq.js uses XMLHttpRequest() directly; should use jQuery

The acq.js file used a few calls to XMLHttpRequest, which have been
changed to use jQuery.ajax instead.

The fetchSortDropbox no longer exists, which is why this patch
did not apply. However, the other three functions still had direct
XMLHttpRequest calls:
- totalExceedsBudget
- budgetExceedsParent
- checkBudgetParent

A test plan would require triggering these three javascript functions.
Comment 13 M. Tompsett 2017-05-27 03:37:42 UTC
The totalExceedsBudget is triggered by Check in koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt

The budgetExceedsParent is triggered by Check in koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt

The checkBudgetParent is triggered by Check in koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt

NOTE: I have not checked this myself, but should someone write a test plan for novice libarian techies, I will check.
Comment 14 Marc Véron 2017-05-27 08:23:43 UTC
> The budgetExceedsParent is triggered by Check in
> koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt

Testing budgetExceedsParent (without patch):

1) Go to Home > Administration > Budgets administration
2) Create a Budget 'Testbudget', give it a value of 100, make it active
3) Add a fund 'Testfund 1', give it a value of 90, save
4) Add a fund 'Testfund 2', give it a value of 20 (exceeds budget), save

Result OK: Message box 
-------------------------------------------------------
Form not submitted because of the following problem(s)
-------------------------------------------------------
- Fund amount exceeds period allocation
-------------------------------------------------------

Testing budgetExceedsParent (with patch):

a) Clear browser cache, restart memcached, plack
b) Make sure page is reloaded, repeat step 4) above

Result: Not OK, no message box, form saves, period allocated is 100, total available is 110 (should not)

(Other functions not yet tested)