Bug 26583 - Unnecessary code in AddIssue
Summary: Unnecessary code in AddIssue
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on: 16272 20912
Blocks:
  Show dependency treegraph
 
Reported: 2020-09-30 18:02 UTC by Nick Clemens
Modified: 2020-11-17 20:48 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.06


Attachments
Bug 26583: Remove unneccessary code in AddIssue (3.30 KB, patch)
2020-09-30 18:07 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 26583: Remove unneccessary code in AddIssue (2.42 KB, patch)
2020-10-01 10:51 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 26583: Remove unneccessary code in AddIssue (2.47 KB, patch)
2020-10-25 20:57 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 26583: Remove unneccessary code in AddIssue (2.53 KB, patch)
2020-11-01 00:26 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2020-09-30 18:02:18 UTC
A few small things:

1 - We calculate the datedue twice
2 - We check for an existing issue, but if we didn't do a renewal then we returned the book, there shouldn't be any issues
3 - We call ModDateLastSeen which simply set the datelast seen and marks item not lost, but we are already setting ti not lost when issuing
Comment 1 Nick Clemens 2020-09-30 18:07:24 UTC
Created attachment 111008 [details] [review]
Bug 26583: Remove unneccessary code in AddIssue

The lines to calc due date are doubled

ModDateLastSeen is not needed as we set the item not lost and we can set
the date as we do for date last borrowed and save a DB store

We should not have an existing issue, either we did and the borrower was the same so we renewed,
or we did and the borrower didn't match, so we returned it

Largely this is reading the code and confirming the changes make sense

To test:
1 - Apply patch
2 - Check out items, confirm it works as before
3 - prove -v t/db_dependent/Circulation.t
Comment 2 Nick Clemens 2020-10-01 10:36:00 UTC
Ah, switch on site checkout, will remove the removal for #2
Comment 3 Nick Clemens 2020-10-01 10:51:50 UTC
Created attachment 111030 [details] [review]
Bug 26583: Remove unneccessary code in AddIssue

The lines to calc due date are doubled

ModDateLastSeen is not needed as we set the item not lost and we can set
the date as we do for date last borrowed and save a DB store

Largely this is reading the code and confirming the changes make sense

To test:
1 - Apply patch
2 - Check out items, confirm it works as before
3 - prove -v t/db_dependent/Circulation.t
Comment 4 Chris Cormack 2020-10-25 20:57:05 UTC
Created attachment 112484 [details] [review]
Bug 26583: Remove unneccessary code in AddIssue

The lines to calc due date are doubled

ModDateLastSeen is not needed as we set the item not lost and we can set
the date as we do for date last borrowed and save a DB store

Largely this is reading the code and confirming the changes make sense

To test:
1 - Apply patch
2 - Check out items, confirm it works as before
3 - prove -v t/db_dependent/Circulation.t

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 5 Katrin Fischer 2020-11-01 00:26:15 UTC
Created attachment 112793 [details] [review]
Bug 26583: Remove unneccessary code in AddIssue

The lines to calc due date are doubled

ModDateLastSeen is not needed as we set the item not lost and we can set
the date as we do for date last borrowed and save a DB store

Largely this is reading the code and confirming the changes make sense

To test:
1 - Apply patch
2 - Check out items, confirm it works as before
3 - prove -v t/db_dependent/Circulation.t

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Jonathan Druart 2020-11-02 10:05:33 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 7 Lucas Gass 2020-11-14 02:23:29 UTC
backported to 20.05.x for 20.05.06
Comment 8 Aleisha Amohia 2020-11-17 05:08:22 UTC
enhancement, not backported to 19.11.x