Bug 9987

Summary: biblioitemnumber is useless for orders.
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: AcquisitionsAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact:
Severity: enhancement    
Priority: P5 - low CC: colin.campbell, gmcharlt, kyle, mathsabypro
Version: Main   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7593
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14172
Change sponsored?: --- Patch complexity: Medium patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 9987: Remove DB field aqorders.biblioitemnunmber
Bug 9987: Add itemtype description instead of code
Bug 9987: Remove DB field aqorders.biblioitemnunmber
Bug 9987: Add itemtype description instead of code
[PATCH][SIGNED OFF] Bug 9987: Remove DB field aqorders.biblioitemnunmber
Bug 9987: Remove DB field aqorders.biblioitemnunmber

Description Jonathan Druart 2013-04-04 08:52:04 UTC
The DB field aqorders.biblioitemnumber seems to be unused and useless.
It should be removed.
Comment 1 Jonathan Druart 2013-04-04 10:25:07 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2013-04-04 10:25:10 UTC Comment hidden (obsolete)
Comment 3 Mathieu Saby 2013-04-05 14:46:03 UTC
Jonathan, you suppress 2 subs in C4::Acquisition. Is it necessary? Are you sure they will not be usefull ?

Mathieu Saby
Comment 4 Jonathan Druart 2013-04-08 10:35:03 UTC
(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
Comment 5 Mathieu Saby 2013-04-08 10:39:51 UTC
OK. But are you sure they cannot be usefull for a developer in the future?
Mathieu
Comment 6 Jonathan Druart 2013-04-08 10:49:32 UTC
(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.
Comment 7 Mathieu Saby 2013-04-12 13:42:20 UTC
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
Comment 8 Jonathan Druart 2013-04-15 07:55:44 UTC
(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).
Comment 9 Jonathan Druart 2013-04-15 07:57:31 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2013-04-15 07:57:35 UTC Comment hidden (obsolete)
Comment 11 Mathieu Saby 2013-04-21 12:17:20 UTC
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
Comment 12 Jonathan Druart 2013-04-22 07:33:15 UTC
(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.
Comment 13 Mathieu Saby 2013-04-22 07:34:51 UTC
thank you, I thought it was a bad attachment...
I will test that this afternoon.

Mathieu
Comment 14 Jonathan Druart 2013-04-22 07:51:25 UTC
Comment on attachment 17436 [details] [review]
Bug 9987: Add itemtype description instead of code

I opened a new bug 10090 for it.
Comment 15 Mathieu Saby 2013-04-22 07:57:21 UTC
Ok, so I sign off bug 9987.
Mathieu
Comment 16 Mathieu Saby 2013-04-22 07:59:45 UTC Comment hidden (obsolete)
Comment 17 Jonathan Druart 2013-04-22 12:26:37 UTC
*** Bug 10091 has been marked as a duplicate of this bug. ***
Comment 18 Kyle M Hall 2013-06-14 15:43:48 UTC
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>
Comment 19 Kyle M Hall 2013-06-14 15:45:51 UTC
Passes koha-qa.pl and everything look good. Removes unused subroutines and references aqorders.biblioitemnumber as well as the field itself.
Comment 20 Galen Charlton 2013-07-02 22:29:23 UTC
Pushed to master.  Thanks, Jonathan!