acqui/basketgroup.tt loads acq.js and a lot of acqui/*.tt loads js functions only used for basketgroup management. These functions should be in a separated js file.
Created attachment 19359 [details] [review] Bug 10533: basketgroup js functions should in a separated js file This patch moves js functions used for managing basketgroup into a separated js file. Test plan: - Try all actions for basketgroup ( drag/drop, add, delete, close, print, reopen, edit, export as csv). - Check that there is no regression on others acquisition pages: * acqui/neworderempty.tt * acqui/uncertainprice.tt * acqui/addorderiso2709.tt * acqui/basketheader.tt * admin/aqbudgets.tt * admin/aqcontract.tt * admin/aqbudgetperiods.tt * admin/aqplan.tt * suggestion/suggestion.tt
Created attachment 19875 [details] [review] [SIGNED-OFF] Bug 10533: basketgroup js functions should in a separated js file This patch moves js functions used for managing basketgroup into a separated js file. Test plan: - Try all actions for basketgroup ( drag/drop, add, delete, close, print, reopen, edit, export as csv). - Check that there is no regression on others acquisition pages: * acqui/neworderempty.tt * acqui/uncertainprice.tt * acqui/addorderiso2709.tt * acqui/basketheader.tt * admin/aqbudgets.tt * admin/aqcontract.tt * admin/aqbudgetperiods.tt * admin/aqplan.tt * suggestion/suggestion.tt Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 20235 [details] [review] Bug 10533: basketgroup js functions should in a separated js file This patch moves js functions used for managing basketgroup into a separated js file. Test plan: - Try all actions for basketgroup ( drag/drop, add, delete, close, print, reopen, edit, export as csv). - Check that there is no regression on others acquisition pages: * acqui/neworderempty.tt * acqui/uncertainprice.tt * acqui/addorderiso2709.tt * acqui/basketheader.tt * admin/aqbudgets.tt * admin/aqcontract.tt * admin/aqbudgetperiods.tt * admin/aqplan.tt * suggestion/suggestion.tt Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Passes koha-qa.pl, works as advertised
Comparing the acq.js with the new basketgroup.js reveals a bunch of diffs like this: - var answer = confirm(MSG_CONFIRM_CLOSE_BASKETGROUP); + var answer=confirm(_("Are you sure you want to close this basketgroup?")); - alert(MSG_CLOSE_EMPTY_BASKET); + alert(_("Why close an empty basket?")); - cantprint.innerHTML = MSG_SAVE_BEFORE_PRINTING; + cantprint.innerHTML=_("You need to save the page before printing"); In other words, this patch effectively causes a partial regression on bug 7598. Please try again; it may be better to start from scratch rather than to edit the patch.
(In reply to Galen Charlton from comment #4) > Comparing the acq.js with the new basketgroup.js reveals a bunch of diffs > like this: > > - var answer = confirm(MSG_CONFIRM_CLOSE_BASKETGROUP); > + var answer=confirm(_("Are you sure you want to close this > basketgroup?")); > > - alert(MSG_CLOSE_EMPTY_BASKET); > + alert(_("Why close an empty basket?")); > > - cantprint.innerHTML = MSG_SAVE_BEFORE_PRINTING; > + cantprint.innerHTML=_("You need to save the page before printing"); > > In other words, this patch effectively causes a partial regression on bug > 7598. > > Please try again; it may be better to start from scratch rather than to edit > the patch. It seems to be a merge error on the signed off patch. I will resubmit a new patch.
Created attachment 20292 [details] [review] Bug 10533: basketgroup js functions should in a separated js file This patch moves js functions used for managing basketgroup into a separated js file. Test plan: - Try all actions for basketgroup ( drag/drop, add, delete, close, print, reopen, edit, export as csv). - Check that there is no regression on others acquisition pages: * acqui/neworderempty.tt * acqui/uncertainprice.tt * acqui/addorderiso2709.tt * acqui/basketheader.tt * admin/aqbudgets.tt * admin/aqcontract.tt * admin/aqbudgetperiods.tt * admin/aqplan.tt * suggestion/suggestion.tt
Patch applied cleanly, go forth and signoff
"- Try all actions for basketgroup ( drag/drop, add, delete, close, print, reopen, edit, export as csv)." => OK for me M. Saby
Is there really a risk of regression on the other pages? Is there a specific point to test? M. Saby
(In reply to mathieu saby from comment #9) > Is there really a risk of regression on the other pages? Is there a specific > point to test? No, it is a c/p code.
I looked at the move of this code from one file to the other. And made a quick compare between what you deleted and what you created in a git format file. Unfortunately this compare was not very helpful. Too much differences (including spacing etc.) Makes it harder to verify what happens here..
Created attachment 27036 [details] [review] Bug 10533: move JavaScript functions for basket groups to a separate file This patch moves JavaScript functions used for managing basket groups to a file. This has the effect of putting the last (active) use of the YUI JavaScript library by the staff interface in one file: koha-tmpl/intranet-tmpl/prog/en/js/basketgroup.js Test plan: - Try all actions for basketgroup ( drag/drop, add, delete, close, print, reopen, edit, export as csv). - Check that there is no regression on others acquisition pages: * acqui/neworderempty.tt * acqui/uncertainprice.tt * acqui/addorderiso2709.tt * acqui/basketheader.tt * admin/aqbudgets.tt * admin/aqcontract.tt * admin/aqbudgetperiods.tt * admin/aqplan.tt * suggestion/suggestion.tt Signed-off-by: Galen Charlton <gmc@esilibrary.com>
(In reply to M. de Rooy from comment #12) > I looked at the move of this code from one file to the other. And made a > quick compare between what you deleted and what you created in a git format > file. > Unfortunately this compare was not very helpful. Too much differences > (including spacing etc.) Makes it harder to verify what happens here.. Indeed. However, with some use of diff -bw to compare the old acq.js, the new acq.js, and basketgroup.js (and with testing of the acq pages, of course), I was able to convince myself that the patch implements a clean move of those functions. Plus, it has the benefit of sticking the last active use of YUI by the staff interface in one file. Signed off.
Created attachment 27293 [details] [review] [PASSED QA] Bug 10533: move JavaScript functions for basket groups to a separate file This patch moves JavaScript functions used for managing basket groups to a file. This has the effect of putting the last (active) use of the YUI JavaScript library by the staff interface in one file: koha-tmpl/intranet-tmpl/prog/en/js/basketgroup.js Test plan: - Try all actions for basketgroup ( drag/drop, add, delete, close, print, reopen, edit, export as csv). - Check that there is no regression on others acquisition pages: * acqui/neworderempty.tt * acqui/uncertainprice.tt * acqui/addorderiso2709.tt * acqui/basketheader.tt * admin/aqbudgets.tt * admin/aqcontract.tt * admin/aqbudgetperiods.tt * admin/aqplan.tt * suggestion/suggestion.tt Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Works as described, passes all tests and QA script.
Pushed to master. Thanks, Jonathan!
Pushed to 3.14.x, will be in 3.14.10