Bug 32019

Summary: Add option to mark items returned in batch modification
Product: Koha Reporter: Nick Clemens <nick>
Component: ToolsAssignee: Nick Clemens <nick>
Status: Needs documenting --- QA Contact: Kyle M Hall <kyle>
Severity: enhancement    
Priority: P5 - low CC: andrewfh, kelly, kyle, martin.renvoize, mspagnuolo, rcoert
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31031
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00
Attachments: Bug 32019: Unit tests
Bug 32019: Add option to makr items returned via batch modification
Bug 32019: Unit tests
Bug 32019: Add option to makr items returned via batch modification
options display
Bug 32019: Unit tests
Bug 32019: Add option to mark items returned via batch modification
Bug 32019: (follow-up) Fix terminology and styling
Bug 32019: Unit tests
Bug 32019: Add option to mark items returned via batch modification
Bug 32019: (follow-up) Fix terminology and styling

Description Nick Clemens 2022-10-27 16:13:00 UTC

    
Comment 1 Nick Clemens 2022-10-27 16:21:07 UTC
Created attachment 142719 [details] [review]
Bug 32019: Unit tests
Comment 2 Nick Clemens 2022-10-27 16:21:11 UTC
Created attachment 142720 [details] [review]
Bug 32019: Add option to makr items returned via batch modification

Ther are options that allow avoiding marking an issue returned when se tot lost.
Libraries do this to tie a lost item to a patron clearly.

Eventually, after the patron has paid, or after some time, the library may wish
to delete the item. When doing so, they do not wish to check the item in and affect any statuses
or fines. This is also true for withdrawn items

It would be helpful to have a way to mark issues returned in bulk, when they have not been marked
returned in the past

To test:
 1 - Apply patches
 2 - Restart all
 3 - Set 'MarkLostItemsAsReturned' system preferences to 'None'
 4 - Check out some items that have replacement fees set
 5 - Mark them lost
 6 - Check out more items, mark them withdrawn
 7 - Set system preference 'BlockReturnOfWithdrawnItems' to block
 8 - Enter item barcodes into batch modification
 9 - Leave 'Mark items returned as blank
10 - Verify nothing was checked in
11 - Repeat but set 'Mark items as returned' to 'No'
12 - Verify nothing was checked in
13 - Repeat, but set 'Mark items as returned' to 'Yes'
14 - Verify items are returned
15 - Verify no fines have been adjusted
16 - Verify withdrawn items are sitll withdrawn
17 - Verify you can now delete the items
Comment 3 Andrew Fuerste-Henry 2022-10-28 13:30:19 UTC
Created attachment 142764 [details] [review]
Bug 32019: Unit tests

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Comment 4 Andrew Fuerste-Henry 2022-10-28 13:30:23 UTC
Created attachment 142765 [details] [review]
Bug 32019: Add option to makr items returned via batch modification

Ther are options that allow avoiding marking an issue returned when se tot lost.
Libraries do this to tie a lost item to a patron clearly.

Eventually, after the patron has paid, or after some time, the library may wish
to delete the item. When doing so, they do not wish to check the item in and affect any statuses
or fines. This is also true for withdrawn items

It would be helpful to have a way to mark issues returned in bulk, when they have not been marked
returned in the past

To test:
 1 - Apply patches
 2 - Restart all
 3 - Set 'MarkLostItemsAsReturned' system preferences to 'None'
 4 - Check out some items that have replacement fees set
 5 - Mark them lost
 6 - Check out more items, mark them withdrawn
 7 - Set system preference 'BlockReturnOfWithdrawnItems' to block
 8 - Enter item barcodes into batch modification
 9 - Leave 'Mark items returned as blank
10 - Verify nothing was checked in
11 - Repeat but set 'Mark items as returned' to 'No'
12 - Verify nothing was checked in
13 - Repeat, but set 'Mark items as returned' to 'Yes'
14 - Verify items are returned
15 - Verify no fines have been adjusted
16 - Verify withdrawn items are sitll withdrawn
17 - Verify you can now delete the items

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Comment 5 Andrew Fuerste-Henry 2022-10-28 13:35:13 UTC
Created attachment 142766 [details]
options display

The dropdown is a little disconnected from itself.
The way the explanatory text is to the left of the heading "Mark items returned" is somewhat visually confusing, and will become more so if further options are added. Can that text go under the heading instead?
Also, please add periods to "This option allows returning items to remove them from patron accounts" and "This feature is intended to allow removal and deletion of items without affecting fines or other statuses"
Comment 6 Katrin Fischer 2022-10-30 13:08:27 UTC
Sorry, ending up with some questions here:

0) Terminology

I have several terminology issues here in the GUI, but also in the code, we are still horribly inconsistent all over.

returned = checked in
issue = checkout

Please update at least the note on the form and use . for ending sentences :)

What statuses would remain untouched? It's used several times as an explanation, but I am not sure what is meant actually.

1) I assume we skip here, because they will be re-indexed when the job is processed later on?

+ skip_record_index => 1,
+ skip_holds_queue  => 1,

If we don't update the index, the item would still show as checked out in the result list.

2) Options on the form are misaligned in the form in the new staff interface design.

3) Why the empty setting? Wouldn't it make sense to preset to 'No'? 

4) You said this is to allow the library to delete the item. 

"Eventually, after the patron has paid, or after some time, the library may wish
to delete the item. When doing so, they do not wish to check the item in and affect any statuses"

Why not add this option to the item batch delete form then?
Comment 7 Nick Clemens 2022-10-31 10:44:28 UTC
(In reply to Katrin Fischer from comment #6)
> Sorry, ending up with some questions here:
> 
> 0) Terminology
> 
> I have several terminology issues here in the GUI, but also in the code, we
> are still horribly inconsistent all over.
> Please update at least the note on the form and use . for ending sentences :)
> 
> What statuses would remain untouched? It's used several times as an
> explanation, but I am not sure what is meant actually.

I added some explanation and added periods - it is wierd here because we are not really doing a normal check in - we are just moving the item to old issues.

By not affecting statuses I mean that normal check in things won't happen - we don't see if check in is allowed, we don't update fines, we don't change shelving location or holding branch

> 
> 1) I assume we skip here, because they will be re-indexed when the job is
> processed later on?
> 
> + skip_record_index => 1,
> + skip_holds_queue  => 1,
> 
> If we don't update the index, the item would still show as checked out in
> the result list.

The reindex happens to all items affected by batch mod in a single operation at the end

> 
> 2) Options on the form are misaligned in the form in the new staff interface
> design.

Should be improved
> 
> 3) Why the empty setting? Wouldn't it make sense to preset to 'No'? 

Ok

> 
> 4) You said this is to allow the library to delete the item. 
> 
> "Eventually, after the patron has paid, or after some time, the library may
> wish
> to delete the item. When doing so, they do not wish to check the item in and
> affect any statuses"
> 
> Why not add this option to the item batch delete form then?

While that is one use, I don't think it is the only use - some libraries simply want the option to get these items off a patrons account without triggering all the things a check in does. If a library chooses not to 'MarkLostItemsReturned' they have no way to recreate that action later. This is to fill that gap
Comment 8 Nick Clemens 2022-10-31 10:45:02 UTC
Created attachment 142823 [details] [review]
Bug 32019: Unit tests

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Comment 9 Nick Clemens 2022-10-31 10:45:06 UTC
Created attachment 142824 [details] [review]
Bug 32019: Add option to mark items returned via batch modification

Ther are options that allow avoiding marking an issue returned when se tot lost.
Libraries do this to tie a lost item to a patron clearly.

Eventually, after the patron has paid, or after some time, the library may wish
to delete the item. When doing so, they do not wish to check the item in and affect any statuses
or fines. This is also true for withdrawn items

It would be helpful to have a way to mark issues returned in bulk, when they have not been marked
returned in the past

To test:
 1 - Apply patches
 2 - Restart all
 3 - Set 'MarkLostItemsAsReturned' system preferences to 'None'
 4 - Check out some items that have replacement fees set
 5 - Mark them lost
 6 - Check out more items, mark them withdrawn
 7 - Set system preference 'BlockReturnOfWithdrawnItems' to block
 8 - Enter item barcodes into batch modification
 9 - Leave 'Mark items returned as blank
10 - Verify nothing was checked in
11 - Repeat but set 'Mark items as returned' to 'No'
12 - Verify nothing was checked in
13 - Repeat, but set 'Mark items as returned' to 'Yes'
14 - Verify items are returned
15 - Verify no fines have been adjusted
16 - Verify withdrawn items are sitll withdrawn
17 - Verify you can now delete the items

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Comment 10 Nick Clemens 2022-10-31 10:45:10 UTC
Created attachment 142825 [details] [review]
Bug 32019: (follow-up) Fix terminology and styling

This patch adds periods to description on batch mod form and alters
language to follow proper terminology

Styling is fixed to align forms

Blank option is removed, No made default
Comment 11 Nick Clemens 2022-11-16 11:27:59 UTC
*** Bug 26835 has been marked as a duplicate of this bug. ***
Comment 12 Kyle M Hall 2023-01-13 17:27:33 UTC
Created attachment 145294 [details] [review]
Bug 32019: Unit tests

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 13 Kyle M Hall 2023-01-13 17:27:45 UTC
Created attachment 145295 [details] [review]
Bug 32019: Add option to mark items returned via batch modification

Ther are options that allow avoiding marking an issue returned when se tot lost.
Libraries do this to tie a lost item to a patron clearly.

Eventually, after the patron has paid, or after some time, the library may wish
to delete the item. When doing so, they do not wish to check the item in and affect any statuses
or fines. This is also true for withdrawn items

It would be helpful to have a way to mark issues returned in bulk, when they have not been marked
returned in the past

To test:
 1 - Apply patches
 2 - Restart all
 3 - Set 'MarkLostItemsAsReturned' system preferences to 'None'
 4 - Check out some items that have replacement fees set
 5 - Mark them lost
 6 - Check out more items, mark them withdrawn
 7 - Set system preference 'BlockReturnOfWithdrawnItems' to block
 8 - Enter item barcodes into batch modification
 9 - Leave 'Mark items returned as blank
10 - Verify nothing was checked in
11 - Repeat but set 'Mark items as returned' to 'No'
12 - Verify nothing was checked in
13 - Repeat, but set 'Mark items as returned' to 'Yes'
14 - Verify items are returned
15 - Verify no fines have been adjusted
16 - Verify withdrawn items are sitll withdrawn
17 - Verify you can now delete the items

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 14 Kyle M Hall 2023-01-13 17:27:49 UTC
Created attachment 145296 [details] [review]
Bug 32019: (follow-up) Fix terminology and styling

This patch adds periods to description on batch mod form and alters
language to follow proper terminology

Styling is fixed to align forms

Blank option is removed, No made default

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 15 Tomás Cohen Arazi 2023-02-20 12:54:48 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 16 Martin Renvoize 2023-02-24 08:38:04 UTC
Enhancement, won't be backporting to the 22.11.x LTS.