We will add table sorter & filters « everywhere » in Koha We will avoid adding sorters & filters where perf is a caveat (like circulation)
If you want to participate to the upstreaming: * assign the bug to yourself * retrieve the patch(es) on git.biblibre.com * the branch name can be found using git branch -a|grep NNNN Usually, it should be biblibre/customer/ft/MTNNNN For example: remotes/origin/stetienne/ft/MT6522 This patch is in branch number 5546
We will use DataTables, not tablesorter
Created attachment 7719 [details] [review] parcel.tt
Created attachment 7720 [details] [review] acqui-home.tt
Created attachment 7721 [details] [review] addorderiso2709.tt
Created attachment 7722 [details] [review] basket.tt
Created attachment 7723 [details] [review] basketgroup.tt
Created attachment 7724 [details] [review] histsearch.tt
Created attachment 7725 [details] [review] lateorders.tt
Created attachment 7726 [details] [review] neworderbiblio.tt
Created attachment 7727 [details] [review] newordersuggestion.tt
Created attachment 7728 [details] [review] ordered.tt
Created attachment 7729 [details] [review] orderreceive.tt
Created attachment 7730 [details] [review] parcels.tt
Created attachment 7731 [details] [review] spent.tt
Created attachment 7732 [details] [review] supplier.tt
Created attachment 7733 [details] [review] uncertainprice.tt
Created attachment 7734 [details] [review] z3950_search.tt
Created attachment 8039 [details] [review] Bug 5345 [FOLLOW-UP] DataTables in Koha Adding two missing images: The "disabled" version of the next and previous buttons.
Created attachment 8040 [details] [review] DataTables missing some required images Adding missing images: The "disabled" version of the next, previous, first, and last buttons.
(In reply to comment #4) > Created attachment 7720 [details] [review] > acqui-home.tt Are the controls really necessary on this page? On my screen the table is pretty small, and the top controls wrap to two lines in an ugly way. I would say either eliminate the controls altogether or have only a minimal set of controls on the top.
Created attachment 8057 [details] [review] [PASSED_QA][SIGNED-OFF] Bug 5345: DataTables integration in acquisition module [16] z3950_search.tt Signed-off-by: Owen Leonard <oleonard@myacpl.org>
(In reply to comment #10) > Created attachment 7726 [details] [review] > neworderbiblio.tt Search results on this page are broken into separate pages by the script. I don't think it makes sense to add a table sorter to each individual pages of results. Integration of DataTables on this page should use AJAX to pull individual pages of results.
Created attachment 8058 [details] [review] [PASSED_QA][SIGNED-OFF] Bug 5345: DataTables integration in acquisition module [9] newordersuggestion.tt Signed-off-by: Owen Leonard <oleonard@myacpl.org>
(In reply to comment #12) > Created attachment 7728 [details] [review] > ordered.tt I'm testing this patch with my dateformat system preference set to "metric" and the sort on the "date ordered" column isn't correct. Is DataTables supposed to detect the metric date format automatically?
<h1>Something went wrong !</h1>Applying: Bug 5345: DataTables integration in acquisition module [1]<br/> Using index info to reconstruct a base tree...<br/> Falling back to patching base and 3-way merge...<br/> Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt<br/> Applying: Bug 5345: DataTables integration in acquisition module [2]<br/> Applying: Bug 5345: DataTables integration in acquisition module [3]<br/> Applying: Bug 5345: DataTables integration in acquisition module [4]<br/> Applying: Bug 5345: DataTables integration in acquisition module [5]<br/> Applying: Bug 5345: DataTables integration in acquisition module [6]<br/> Applying: Bug 5345: DataTables integration in acquisition module [7]<br/> Using index info to reconstruct a base tree...<br/> Falling back to patching base and 3-way merge...<br/> Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tt<br/> Applying: Bug 5345: DataTables integration in acquisition module [8]<br/> Applying: Bug 5345: DataTables integration in acquisition module [10]<br/> Applying: Bug 5345: DataTables integration in acquisition module [11]<br/> Applying: Bug 5345: DataTables integration in acquisition module [12]<br/> Using index info to reconstruct a base tree...<br/> Falling back to patching base and 3-way merge...<br/> Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcels.tt<br/> CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcels.tt<br/> Failed to merge in the changes.<br/> Patch failed at 0001 Bug 5345: DataTables integration in acquisition module [12]<br/> When you have resolved this problem run git am --resolved.<br/> If you would prefer to skip this patch, instead run git am --skip.<br/> To restore the original branch and stop patching run git am --abort.<br/> Bug 5345 - DataTables in Koha<br/> <br/> parcel.tt<br/> Apply? [yn] <br/> acqui-home.tt<br/> Apply? [yn] <br/> addorderiso2709.tt<br/> Apply? [yn] <br/> basket.tt<br/> Apply? [yn] <br/> basketgroup.tt<br/> Apply? [yn] <br/> histsearch.tt<br/> Apply? [yn] <br/> lateorders.tt<br/> Apply? [yn] <br/> neworderbiblio.tt<br/> Apply? [yn] <br/> ordered.tt<br/> Apply? [yn] <br/> orderreceive.tt<br/> Apply? [yn] <br/> parcels.tt<br/> Apply? [yn] <br/> Patch left in /tmp/parcelstt-AYmmg0.patch<br/> <br/>=== translation installation ===<br/>Install templates Install templates 'opac From: /home/koha/src/koha-tmpl/opac-tmpl/prog/en/ To : /home/koha/src/koha-tmpl/opac-tmpl/prog/fr-FR With: /home/koha/src/misc/translator/po/fr-FR-i-opac-t-prog-v-3006000.po The install seems to be successful. Install templates 'intranet From: /home/koha/src/koha-tmpl/intranet-tmpl/prog/en/ To : /home/koha/src/koha-tmpl/intranet-tmpl/prog/fr-FR With: /home/koha/src/misc/translator/po/fr-FR-i-staff-t-prog-v-3006000.po The install seems to be successful. If you use this system for the 1st time, you may find usefull to reach the wiki page http://wiki.koha-community.org/wiki/Sandboxes to have some information about those results, to check if everything went well & if you can test the patch properly
Created attachment 8292 [details] [review] parcels.tt Rebased parcels.tt
Applying: Bug 5345: DataTables integration in acquisition module [1] Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt Applying: Bug 5345: DataTables integration in acquisition module [2] Applying: Bug 5345: DataTables integration in acquisition module [3] Applying: Bug 5345: DataTables integration in acquisition module [4] Applying: Bug 5345: DataTables integration in acquisition module [5] Applying: Bug 5345: DataTables integration in acquisition module [6] Applying: Bug 5345: DataTables integration in acquisition module [7] Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tt Applying: Bug 5345: DataTables integration in acquisition module [8] Applying: Bug 5345: DataTables integration in acquisition module [10] Applying: Bug 5345: DataTables integration in acquisition module [11] Applying: Bug 5345: DataTables integration in acquisition module [13] Applying: Bug 5345: DataTables integration in acquisition module [14] Applying: Bug 5345: DataTables integration in acquisition module [15] Applying: Bug 5345 [FOLLOW-UP] DataTables in Koha Applying: Bug 5345: DataTables integration in acquisition module [16] Applying: Bug 5345: DataTables integration in acquisition module [9] Applying: Bug 5345: DataTables integration in acquisition module [12] Bug 5345 - DataTables in Koha parcel.tt Apply? [yn] acqui-home.tt Apply? [yn] addorderiso2709.tt Apply? [yn] basket.tt Apply? [yn] basketgroup.tt Apply? [yn] histsearch.tt Apply? [yn] lateorders.tt Apply? [yn] neworderbiblio.tt Apply? [yn] ordered.tt Apply? [yn] orderreceive.tt Apply? [yn] spent.tt Apply? [yn] supplier.tt Apply? [yn] uncertainprice.tt Apply? [yn] Bug 5345 [FOLLOW-UP] DataTables in Koha Apply? [yn] [SIGNED-OFF] Bug 5345: DataTables integration in acquisition module [16] Apply? [yn] [SIGNED-OFF] Bug 5345: DataTables integration in acquisition module [9] Apply? [yn] parcels.tt Apply? [yn] === translation installation === Install templates Install templates 'opac From: /home/koha/src/koha-tmpl/opac-tmpl/prog/en/ To : /home/koha/src/koha-tmpl/opac-tmpl/prog/fr-FR With: /home/koha/src/misc/translator/po/fr-FR-i-opac-t-prog-v-3006000.po The install seems to be successful. Install templates 'intranet From: /home/koha/src/koha-tmpl/intranet-tmpl/prog/en/ To : /home/koha/src/koha-tmpl/intranet-tmpl/prog/fr-FR With: /home/koha/src/misc/translator/po/fr-FR-i-staff-t-prog-v-3006000.po The install seems to be successful.
This patch can not be tested with a sandbox. Please Test these patches individually
After talking with Jonathan and Corinne (who tested the BZ ticket 7291), we thought that there should be a default sorting within the lateorders page. Is it possible to get the default sorting (ASC) on the "estimated date"? Thanks!
1) parcel.tt - still applies on current master, but basket and order line columns are not sortable. When clicking on the sort options, I see a javasript error in Firebug. FAILED QA 2) acqui-home.tt - still applies on current master and works. I am not sure having the full toolbar here is necessary - but it works. I see that Owen has commented on that too. NOT signing off. 3) addorderiso2709.tt - still applies on current master. Only affects the list of imports, not the list of records. Signing off. 4) basket.tt - still applies on current master. Modify and Delete columns are sortable. That's not necessary and could be removed in a follow up. Signing off. 5) basketgroup.tt - still applies to master but I don't see where it's applied? QUESTION 6) histsearch.tt - still applies to current master and works. Signing off. 7) lateorders.tt - does not apply. FAILED QA CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tt 8) neworderbiblio.tt - still applies on current master. There is still a pagination on top of the page. I think sorting only a single result page does not make sense. We should remove the pagination in favor of the datatables or keep pagination for now. FAILED QA 9) ordered.tt - still applies on current master and works. Signing off. 10) orderreceive.tt - does not apply. FAILED QA CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt 11) spent.tt - still applies on current master and works. Signing off. 12) supplier.tt - still applies on current master and works. Here - similar to aqui-home.tt I am not sure the full toolbars are needed for the contracts table. I think having sortable columns would be enough. NOT signing off. 13) uncertainprice.tt - still applies on current master and works. Signing off. 14) DataTables missing some required images Works, signing off. 15) parcels.tt - still applies on current master, but there is a merge marker in the patch - FAILED QA. # Failed test './koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcels.tt contains merge conflict markers in line 79' # at t/00-merge-conflict-markers.t line 51. # Looks like you failed 1 test of 6260.
Created attachment 8830 [details] [review] [SIGNED-OFF] Bug 5345 [FOLLOW-UP] DataTables in Koha Adding missing images: The "disabled" version of the next, previous, first, and last buttons. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 8831 [details] [review] [PASSED_QA][SIGNED-OFF] Bug 5345: DataTables integration in acquisition module [15] uncertainprice.tt Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 8833 [details] [review] [PASSED_QA][SIGNED-OFF] Bug 5345: DataTables integration in acquisition module [13] spent.tt Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 8834 [details] [review] [PASSED_QA][SIGNED-OFF] Bug 5345: DataTables integration in acquisition module [10] ordered.tt Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 8835 [details] [review] [SIGNED-OFF] Bug 5345: DataTables integration in acquisition module [6] histsearch.tt Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 8836 [details] [review] [SIGNED-OFF] Bug 5345: DataTables integration in acquisition module [4] basket.tt Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Columns for 'Modify' and 'Delete' are sortable, which doesn't hurt but could be removed.
Created attachment 8837 [details] [review] [SIGNED-OFF] Bug 5345: DataTables integration in acquisition module [3] addorderiso2709.tt Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Only affects the list of imports, not the list of records
Comment on attachment 8830 [details] [review] [SIGNED-OFF] Bug 5345 [FOLLOW-UP] DataTables in Koha This patch has been pushed: the -disabled png are referenced in the code already, so they are needed
Attachment 8057 [details] passed qa. Will continue later on.
Comment on attachment 8057 [details] [review] [PASSED_QA][SIGNED-OFF] Bug 5345: DataTables integration in acquisition module [16] this patch has been pushed
back to sign-off for the patches that are waiting for marcel's QA
While working on the next patch, I still have trouble with the 7178 patch (stopping me from creating items in acq [again!]). So going there first..
Attachment 8058 [details] passed qa. Will soon continue with 8831.
Created attachment 9519 [details] [review] [PASSED_QA][SIGNED-OFF] Bug 5345: DataTables integration in acquisition module [6] Rebased (changed case in a title)
8831, 8833 and 8834 passed qa. 8835 replaced by 9519 too. Coming back soon for 8836 and 8837.
All now-marked-obsolete patches have been pushed, others still need signoff
Created attachment 9601 [details] [review] lateorders.tt Rebased patch + default sort order = asc on delivery date
(In reply to comment #47) > All now-marked-obsolete patches have been pushed, others still need signoff Could be that a mistake was made somewhere? Note that Bug 5345: DataTables integration in acquisition module [6] did not yet pass QA, but has already been pushed. (Just noting for the record.) Also DataTables integration in acquisition module [4] has been obsoleted. Please check if that is correct. Not QAed nor pushed as far as I can see. DataTables integration in acquisition module [3] should still be QAed. Some of the other patches are actually in a kind of Failed QA-state; see the comments from Katrin.
Marcel is right, I failed some QA - in the comments. I didn't know how to better do it. I could have reattached the file pehraps, but instead of [SIGNED OFF] marked with [FAILED QA]. Even better I think woudl be a meta bug for changes like this, with lots of smaller bugs surrounding with single patches. Much easier to manage and not growing endlessly like this one.
(In reply to comment #50) > Marcel is right, I failed some QA - in the comments. I didn't know how to > better do it. I could have reattached the file pehraps, but instead of > [SIGNED OFF] marked with [FAILED QA]. You do not need to reattach, just change the description with Edit Details (two clicks away).
Created attachment 9602 [details] [review] basketgroup.tt
Comment on attachment 7719 [details] [review] parcel.tt see Bug 8098
Comment on attachment 7726 [details] [review] neworderbiblio.tt see Bug 8100
Comment on attachment 7729 [details] [review] orderreceive.tt see Bug 8101
Comment on attachment 7732 [details] [review] supplier.tt see Bug 8102
Comment on attachment 8292 [details] [review] parcels.tt see Bug 8103
Comment on attachment 9601 [details] [review] lateorders.tt see Bug 8104
Comment on attachment 8837 [details] [review] [SIGNED-OFF] Bug 5345: DataTables integration in acquisition module [3] see Bug 8106
Comment on attachment 9602 [details] [review] basketgroup.tt see Bug 8105
Comment on attachment 7720 [details] [review] acqui-home.tt see Bug 8099
(In reply to comment #50) > Even better I think woudl be a meta bug for changes like this, with lots of > smaller bugs surrounding with single patches. Much easier to manage and not > growing endlessly like this one. Done :)
(In reply to comment #64) > (In reply to comment #50) > > Even better I think woudl be a meta bug for changes like this, with lots of > > smaller bugs surrounding with single patches. Much easier to manage and not > > growing endlessly like this one. > > Done :) Did you check my last questions? Appreciate your hard work, but may be harder now to look back what happened..
(In reply to comment #49) > (In reply to comment #47) > > All now-marked-obsolete patches have been pushed, others still need signoff > > Could be that a mistake was made somewhere? > Note that Bug 5345: DataTables integration in acquisition module [6] did not > yet pass QA, but has already been pushed. (Just noting for the record.) Yes, indeed :-/ > Also DataTables integration in acquisition module [4] has been obsoleted. > Please check if that is correct. Not QAed nor pushed as far as I can see. > DataTables integration in acquisition module [3] should still be QAed. > Some of the other patches are actually in a kind of Failed QA-state; see the > comments from Katrin. Yes I created 2 new Bugs for them, Bug 8106 - DataTables in Acquisision module: addorderiso2709.tt for [3] and Bug 8109 - DataTables in Acquisision module: basket.tt for [4] I am under the impression that all bugs have a new Bug or have already pushed. I hope it will be much easier to test like this.
I'm marking this fixed because the staff client now has wide use of DataTables, with all pages (with the exception of the cart, Bug 9456) having been converted from the old tablesorter plugin. Future enhancements using DataTables can get their own individual bug reports.