Bug 26583

Summary: Unnecessary code in AddIssue
Product: Koha Reporter: Nick Clemens <nick>
Component: CirculationAssignee: Nick Clemens <nick>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: aleisha, gmcharlt, kyle.m.hall, lucas
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.06
Bug Depends on: 16272, 20912    
Bug Blocks:    
Attachments: Bug 26583: Remove unneccessary code in AddIssue
Bug 26583: Remove unneccessary code in AddIssue
Bug 26583: Remove unneccessary code in AddIssue
Bug 26583: Remove unneccessary code in AddIssue

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