The DB field aqorders.biblioitemnumber seems to be unused and useless. It should be removed.
Created attachment 17184 [details] [review] Bug 9987: Remove DB field aqorders.biblioitemnunmber The DB field aqorders.biblioitemnumber seems to be unused except to get the itype on the spent.pl page. This information can be retrieved uising another sql join. Test plan: Try a complete workflow in the acquisition module: create an order, receive it, play with the syspref AcqCreateItem. Check that no regression is found and that the data for existing orders don't change.
Created attachment 17185 [details] [review] Bug 9987: Add itemtype description instead of code In ordered.pl and spent.pl, the codes are displayed, instead of descriptions. Links for the ordernumber should be changed. In ordered.pl, we are redirected to the receive page. In spent.pl, the links are deleted.
Jonathan, you suppress 2 subs in C4::Acquisition. Is it necessary? Are you sure they will not be usefull ? Mathieu Saby
(In reply to comment #3) > Jonathan, you suppress 2 subs in C4::Acquisition. Is it necessary? Are you > sure they will not be usefull ? Yes, grep is my friend. git grep GetOrderNumber git grep ModOrderBibliotemNumber
OK. But are you sure they cannot be usefull for a developer in the future? Mathieu
(s)he will rewrite them :) Both use the field I delete and are not currently in use. I don't see any usefulness to keep them.
I have this error when I try to apply 17184 Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Auto-merging C4/Acquisition.pm Failed to merge in the changes. Patch failed at 0001 Bug 9987: Remove DB field aqorders.biblioitemnunmber When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort". M. Saby
(In reply to comment #7) > I have this error when I try to apply 17184 > > Auto-merging installer/data/mysql/updatedatabase.pl > CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl > Auto-merging C4/Acquisition.pm > Failed to merge in the changes. > Patch failed at 0001 Bug 9987: Remove DB field aqorders.biblioitemnunmber > When you have resolved this problem run "git am --resolved". > If you would prefer to skip this patch, instead run "git am --skip". > To restore the original branch and stop patching run "git am --abort". > > M. Saby Mathieu, It is a conflict on updatedatabase.pl, it is really easy to fix for testing (edit the file, merge conflict, git add /path/to/updatedatabase.pl, git am --resolved).
Created attachment 17435 [details] [review] Bug 9987: Remove DB field aqorders.biblioitemnunmber The DB field aqorders.biblioitemnumber seems to be unused except to get the itype on the spent.pl page. This information can be retrieved uising another sql join. Test plan: Try a complete workflow in the acquisition module: create an order, receive it, play with the syspref AcqCreateItem. Check that no regression is found and that the data for existing orders don't change.
Created attachment 17436 [details] [review] Bug 9987: Add itemtype description instead of code In ordered.pl and spent.pl, the codes are displayed, instead of descriptions. Links for the ordernumber should be changed. In ordered.pl, we are redirected to the receive page. In spent.pl, the links are deleted.
Hello Jonathan attachement 17435 is working well. I did not test 17436 because I am not sure it is related to the same problem. Could you please confirm it, and provide a test plan ? Mathieu
(In reply to comment #11) > I did not test 17436 because I am not sure it is related to the same > problem. Could you please confirm it, and provide a test plan ? On ordered.pl and spent.pl, itemtype codes are displayed instead of their descriptions. This patch just change that.
thank you, I thought it was a bad attachment... I will test that this afternoon. Mathieu
Comment on attachment 17436 [details] [review] Bug 9987: Add itemtype description instead of code I opened a new bug 10090 for it.
Ok, so I sign off bug 9987. Mathieu
Created attachment 17575 [details] [review] [PATCH][SIGNED OFF] Bug 9987: Remove DB field aqorders.biblioitemnunmber
*** Bug 10091 has been marked as a duplicate of this bug. ***
Created attachment 19016 [details] [review] Bug 9987: Remove DB field aqorders.biblioitemnunmber The DB field aqorders.biblioitemnumber seems to be unused except to get the itype on the spent.pl page. This information can be retrieved uising another sql join. Test plan: Try a complete workflow in the acquisition module: create an order, receive it, play with the syspref AcqCreateItem. Check that no regression is found and that the data for existing orders don't change. Signed-off-by: Mathieu Saby <mathieu.saby@univ-rennes2.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Passes koha-qa.pl and everything look good. Removes unused subroutines and references aqorders.biblioitemnumber as well as the field itself.
Pushed to master. Thanks, Jonathan!