Bug 9987 - biblioitemnumber is useless for orders.
Summary: biblioitemnumber is useless for orders.
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact:
URL:
Keywords:
: 10091 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-04-04 08:52 UTC by Jonathan Druart
Modified: 2015-05-18 12:10 UTC (History)
4 users (show)

See Also:
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 (12.90 KB, patch)
2013-04-04 10:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9987: Add itemtype description instead of code (4.93 KB, patch)
2013-04-04 10:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9987: Remove DB field aqorders.biblioitemnunmber (12.88 KB, patch)
2013-04-15 07:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9987: Add itemtype description instead of code (4.93 KB, patch)
2013-04-15 07:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PATCH][SIGNED OFF] Bug 9987: Remove DB field aqorders.biblioitemnunmber (12.95 KB, patch)
2013-04-22 07:59 UTC, Mathieu Saby
Details | Diff | Splinter Review
Bug 9987: Remove DB field aqorders.biblioitemnunmber (12.99 KB, patch)
2013-06-14 15:43 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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!