Bug 6372 - Batch progress bar does not check for 0 size jobs; should be in separate js too
Summary: Batch progress bar does not check for 0 size jobs; should be in separate js too
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low minor (vote)
Assignee: Srdjan Jankovic
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-19 07:09 UTC by Srdjan Jankovic
Modified: 2013-12-05 19:59 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
p (13.12 KB, patch)
2011-05-19 07:32 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
patch (12.65 KB, patch)
2012-02-13 07:46 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
bug 6372: moved background-job.inc to background-job-progressbar.js (12.87 KB, patch)
2012-03-10 16:20 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
patch (13.47 KB, patch)
2012-03-23 00:17 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
bug 6372: moved background-job.inc to background-job-progressbar.js (13.58 KB, patch)
2012-04-10 15:09 UTC, Liz Rea
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Srdjan Jankovic 2011-05-19 07:09:52 UTC
Batch progress bar does not check for 0 size jobs, and divides by 0, with unpredictable effects.
Also, putting progress bar javascript inline makes it impossible to debug.

It is possible that this change will improve situation with some bugs reported against background jobs that use progress bar.
Comment 1 Srdjan Jankovic 2011-05-19 07:32:44 UTC Comment hidden (obsolete)
Comment 2 Srdjan Jankovic 2012-02-13 07:46:09 UTC Comment hidden (obsolete)
Comment 3 Jared Camins-Esakov 2012-03-10 16:20:43 UTC Comment hidden (obsolete)
Comment 4 Ian Walls 2012-03-16 17:34:55 UTC
Scanning for remaining instances of [% INCLUDE 'background-job.inc' %], I found one left in:

koha-tmpl/intranet-tmpl/prog/en/modules/tools/upload-images.tt
Comment 5 Srdjan Jankovic 2012-03-23 00:17:00 UTC Comment hidden (obsolete)
Comment 6 Liz Rea 2012-04-10 15:09:20 UTC
Created attachment 9076 [details] [review]
bug 6372: moved background-job.inc to background-job-progressbar.js

Check for div 0 when calculating percentage

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
div0 no longer appears on 0 size jobs.
Passes t xt
Comment 7 Jonathan Druart 2012-05-03 14:46:26 UTC
QA comment:
- Move background job javascript code into a js file.
- Delete useless routine progress_callback (batchmod.pl)
- Fix the specific case where there is no job to process
Marked as Passed QA
Comment 8 Paul Poulain 2012-05-07 21:15:11 UTC
Patch pushed. I've tested that Koha still works with files that are not of size 0

Note that it's hard to spot the div by 0 in the midlde of the move to .js. Both are perfect things, but it would have been better to do 2 patches