Bug 17679

Summary: C4::Circulation - Remove unused GetItemIssues
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: josef.moravec, katrin.fischer, kyle
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 5549    
Bug Blocks: 17677, 17680    
Attachments: Bug 17679: C4::Circulation - Remove unused GetItemIssues
Bug 17679: C4::Circulation - Remove unused GetItemIssues
[SIGNED-OFF] Bug 17679: C4::Circulation - Remove unused GetItemIssues
Bug 17679: C4::Circulation - Remove unused GetItemIssues

Description Jonathan Druart 2016-11-25 10:51:48 UTC

    
Comment 1 Jonathan Druart 2016-11-25 11:01:24 UTC
Created attachment 57770 [details] [review]
Bug 17679: C4::Circulation - Remove unused GetItemIssues

Ready for an archaeology course?

C4::Circulation::GetItemIssues is only used once, from
catalogue/issuehistory.pl
This call has been added by
  commit 95d6452462a560ba0c0ac859a2cfb7783c25c925
    Adding some more information on issuehistory.
which says "Adding itemnumber to issuehistory.pl API so that one could search
for issuehistory of a specific item."
So it added the ability to see the item issue history but did not
provide a way to access it via the interface.
It's ok so far but this subroutine is broken since
  commit aa114f53499b9cffde0571fe7e08622f9c9a332a
    Bug 5549 : Only use DateTime for issues table
because of this change:
-    my $today = C4::Dates->today('iso');
+    my $today = DateTime->now( time_zome => C4::Context->tz);

I let you catch the typo ;)
And since this commit the subroutine explodes with "The following
parameter was passed in the call to DateTime::from_epoch but was not
listed in the validation options: time_zome"

Since it has never been raised by someone and that the feature is
hidden, I'd recommend to simply remove it.

Note that the "Checked out from" column would have been wrong even if we
fixed all the previous issue.

Test plan:
Just dig into the code and confirm what this commit message tells
Comment 2 Josef Moravec 2016-11-25 17:00:26 UTC
I totally agree with removing this part of code.

Just few comments:

qa tool is saying this:

 FAIL   catalogue/issuehistory.pl
   FAIL   valid
                "my" variable $biblio masks earlier declaration in same scope



also in the copied part of issuhistory.pl, line 53:

my $total  = scalar @$issues;

but $total is not used and the number of issues is passed to template by this lines:
 	
$template->param(
    total        => scalar @$issues,
...
Comment 3 Jonathan Druart 2016-11-28 08:33:39 UTC
Created attachment 57783 [details] [review]
Bug 17679: C4::Circulation - Remove unused GetItemIssues

Ready for an archaeology course?

C4::Circulation::GetItemIssues is only used once, from
catalogue/issuehistory.pl
This call has been added by
  commit 95d6452462a560ba0c0ac859a2cfb7783c25c925
    Adding some more information on issuehistory.
which says "Adding itemnumber to issuehistory.pl API so that one could search
for issuehistory of a specific item."
So it added the ability to see the item issue history but did not
provide a way to access it via the interface.
It's ok so far but this subroutine is broken since
  commit aa114f53499b9cffde0571fe7e08622f9c9a332a
    Bug 5549 : Only use DateTime for issues table
because of this change:
-    my $today = C4::Dates->today('iso');
+    my $today = DateTime->now( time_zome => C4::Context->tz);

I let you catch the typo ;)
And since this commit the subroutine explodes with "The following
parameter was passed in the call to DateTime::from_epoch but was not
listed in the validation options: time_zome"

Since it has never been raised by someone and that the feature is
hidden, I'd recommend to simply remove it.

Note that the "Checked out from" column would have been wrong even if we
fixed all the previous issue.

Test plan:
Just dig into the code and confirm what this commit message tells
Comment 4 Josef Moravec 2016-11-28 09:14:40 UTC
Created attachment 57784 [details] [review]
[SIGNED-OFF] Bug 17679: C4::Circulation - Remove unused GetItemIssues

Ready for an archaeology course?

C4::Circulation::GetItemIssues is only used once, from
catalogue/issuehistory.pl
This call has been added by
  commit 95d6452462a560ba0c0ac859a2cfb7783c25c925
    Adding some more information on issuehistory.
which says "Adding itemnumber to issuehistory.pl API so that one could search
for issuehistory of a specific item."
So it added the ability to see the item issue history but did not
provide a way to access it via the interface.
It's ok so far but this subroutine is broken since
  commit aa114f53499b9cffde0571fe7e08622f9c9a332a
    Bug 5549 : Only use DateTime for issues table
because of this change:
-    my $today = C4::Dates->today('iso');
+    my $today = DateTime->now( time_zome => C4::Context->tz);

I let you catch the typo ;)
And since this commit the subroutine explodes with "The following
parameter was passed in the call to DateTime::from_epoch but was not
listed in the validation options: time_zome"

Since it has never been raised by someone and that the feature is
hidden, I'd recommend to simply remove it.

Note that the "Checked out from" column would have been wrong even if we
fixed all the previous issue.

Test plan:
Just dig into the code and confirm what this commit message tells

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Looks fine for me.
Comment 5 Kyle M Hall 2016-12-23 12:14:33 UTC
Created attachment 58413 [details] [review]
Bug 17679: C4::Circulation - Remove unused GetItemIssues

Ready for an archaeology course?

C4::Circulation::GetItemIssues is only used once, from
catalogue/issuehistory.pl
This call has been added by
  commit 95d6452462a560ba0c0ac859a2cfb7783c25c925
    Adding some more information on issuehistory.
which says "Adding itemnumber to issuehistory.pl API so that one could search
for issuehistory of a specific item."
So it added the ability to see the item issue history but did not
provide a way to access it via the interface.
It's ok so far but this subroutine is broken since
  commit aa114f53499b9cffde0571fe7e08622f9c9a332a
    Bug 5549 : Only use DateTime for issues table
because of this change:
-    my $today = C4::Dates->today('iso');
+    my $today = DateTime->now( time_zome => C4::Context->tz);

I let you catch the typo ;)
And since this commit the subroutine explodes with "The following
parameter was passed in the call to DateTime::from_epoch but was not
listed in the validation options: time_zome"

Since it has never been raised by someone and that the feature is
hidden, I'd recommend to simply remove it.

Note that the "Checked out from" column would have been wrong even if we
fixed all the previous issue.

Test plan:
Just dig into the code and confirm what this commit message tells

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Looks fine for me.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Kyle M Hall 2016-12-28 13:52:43 UTC
Pushed to master for 17.05, thanks Jonathan!
Comment 7 Katrin Fischer 2017-01-04 21:51:23 UTC
This won't get ported back to 16.11.x as it is an enhancement.