Bug 18426

Summary: Subscriptions batch editing
Product: Koha Reporter: Julian Maurice <julian.maurice>
Component: SerialsAssignee: Julian Maurice <julian.maurice>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: caroline.cyr-la-rose, colin.campbell, jonathan.druart, katrin.fischer, martin.renvoize, nick, paul.poulain, severine.queune, stephane.delaye
Version: master   
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: 18785, 20599    
Bug Blocks: 18784, 35073, 21554, 22621    
Attachments: Bug 18426: Refactor subscriptions tables code in a BLOCK
Bug 18426: Allow to edit subscriptions in batch
Bug 18426: Allow to edit subscriptions in batch
Bug 18426: Refactor subscriptions tables code in a BLOCK
Bug 18426: Allow to edit subscriptions in batch
Bug 18426: Fix datepicker
Bug 18426: Address QA issues
Bug 18426: Address QA issues
Bug 18426: Make sure the logged in user can edit the subscriptions
Bug 18426: Replace 'Modify' with 'Edit'
Bug 18426: Refactor subscriptions tables code in a BLOCK
Bug 18426: Allow to edit subscriptions in batch
Bug 18426: Fix datepicker
Bug 18426: Address QA issues
Bug 18426: Make sure the logged in user can edit the subscriptions
Bug 18426: Replace 'Modify' with 'Edit'

Description Julian Maurice 2017-04-13 07:13:01 UTC

    
Comment 1 Julian Maurice 2017-04-13 07:14:21 UTC
Created attachment 62119 [details] [review]
Bug 18426: Refactor subscriptions tables code in a BLOCK
Comment 2 Julian Maurice 2017-04-13 07:14:24 UTC
Created attachment 62120 [details] [review]
Bug 18426: Allow to edit subscriptions in batch

This adds a checkbox column in serials-search.pl tables that allow to edit
selected subscriptions.
The following fields can be modified:
 - Bookseller
 - Location
 - Library
 - Item type
 - Public note
 - Nonpublic note
 - "Create item when receiving" flag
 - Expiration date
+ the additional fields defined in serials/add_fields.pl

Test plan:

1. Go to Serials module
2. If there is no additional fields defined, define some (at least one with an
   authorized value and one without)
3. Start a subscription search
4. Select some results using the checkboxes and click the "Edit" button above
   the table
5. Select values for some fields (not all) and click "Start batch edit"
6. Verify you are being redirected to the previous search results
7. Verify that the fields for which you selected a value were modified and that
   the others fields weren't
8. Repeat steps 4 to 7 but this time, modify the other fields.
Comment 3 delaye 2017-04-13 11:51:09 UTC
test in the sandbox 1 of biblibre with the test plan . It works.
Comment 4 Jonathan Druart 2017-05-02 17:15:03 UTC
Please move Koha::Subscription->biblio to its own bug report and provide tests.
Comment 5 Julian Maurice 2017-06-12 11:17:09 UTC
Created attachment 64198 [details] [review]
Bug 18426: Allow to edit subscriptions in batch

This adds a checkbox column in serials-search.pl tables that allow to edit
selected subscriptions.
The following fields can be modified:
 - Bookseller
 - Location
 - Library
 - Item type
 - Public note
 - Nonpublic note
 - "Create item when receiving" flag
 - Expiration date
+ the additional fields defined in serials/add_fields.pl

Test plan:

1. Go to Serials module
2. If there is no additional fields defined, define some (at least one with an
   authorized value and one without)
3. Start a subscription search
4. Select some results using the checkboxes and click the "Edit" button above
   the table
5. Select values for some fields (not all) and click "Start batch edit"
6. Verify you are being redirected to the previous search results
7. Verify that the fields for which you selected a value were modified and that
   the others fields weren't
8. Repeat steps 4 to 7 but this time, modify the other fields.
Comment 6 Julian Maurice 2017-06-12 11:17:40 UTC
(In reply to Jonathan Druart from comment #4)
> Please move Koha::Subscription->biblio to its own bug report and provide
> tests.

Moved into bug 18785
Comment 7 Caroline Cyr La Rose 2017-09-22 20:01:48 UTC
Hi, I applied the patch and was able to get the check boxes, but when I click "Edit", I get a software error.

Template process failed: undef error - The method biblio is not covered by tests! at /inlibro/git/koha-master-dev-inlibro/C4/Templates.pm line 121.
Comment 8 Nick Clemens 2017-10-07 00:43:03 UTC
Caroline, 

Did you apply dependencies and restart services and regenerate schema? All seems to work for me.

Julian,

I like this feature a lot, a few comments:
1 - The edit button is inconsistent, it should be a link as the others, and I would suggest 'Edit selected serials'
2 - On the edit page we should display all editable values - when the dropdown is 'No change' I cannot know what the original value was for things like 'Create items when receiving
3 - We should have a 'Cancel' button if we choose not to edit the subs
4 - It might be nice to be able to select/deselect subs before applying the edits - not a blocker but saves from going back a page on wrong click
5 - Maybe 'Save' instead of 'Start batch edit'?
Comment 9 Katrin Fischer 2017-10-07 14:55:52 UTC
Hi Julian,

didn't get far in testing, because even after running dbic and restart_all I still get an internal server error, when I try to 'edit':

Template process failed: undef error - The method biblio is not covered by tests! at /home/vagrant/kohaclone/C4/Templates.pm line 121.
Template process failed: undef error - The method biblio is not covered by tests! at /home/vagrant/kohaclone/C4/Templates.pm line 121.


The subscription search seems to work nicely, agreeing about Nick's 1:

1 - The edit button is inconsistent, it should be a link as the others, and I would suggest 'Edit selected serials' - Agreed
Comment 10 Caroline Cyr La Rose 2017-10-20 19:02:14 UTC
Nick,

I did not do any of those things and I don't know what they mean. I'm not that technical... Sorry!

BUT I just did a git pull and I had to reinstall Koha for the latest version. It asked to install dependencies, so I guess that's done...

I retested and IT WORKS! And it's really neat! :)
Comment 11 Katrin Fischer 2017-10-22 12:29:34 UTC
Caroline, did you intend to mark this signed off?
Comment 12 Caroline Cyr La Rose 2017-10-23 14:41:17 UTC
Katrin,

I wasn't sure if I should, since you also seemed to have problems testing it. I think it should be tested again by someone else before being signed off. Maybe put it back to needs signoff?
Comment 13 Katrin Fischer 2017-10-25 19:48:12 UTC
Hi Julian,

I really like it! The problem I previously had disappeared :) Some notes:

1) Agree with Nick: "Edit" button would be nicer as a link, matching other pages like the details page. 
2) If you select no subscriptions and click on edit, there should be a warning. Or it should be inactive until you have selected some.
3) It would be nice to have a checkbox like on the batch item edit in order to allow emptying the note fields. At the moment you'd have to add a space or something to clear them out.
4) Agree with Nick: A cancel link would be consistent with other pages and features.
5) Agree with Nick: Maybe 'Save' instead of 'Start batch edit'?
6) Agree with Nick:  It might be nice to be able to select/deselect subs before applying the edits - not a blocker but saves from going back a page on wrong click

Don't understand that one: 
>is 'No change' I cannot know what the original value was for things like 
>'Create items when receiving

Nick, can you explain?
Comment 14 Séverine Queune 2018-03-09 13:54:38 UTC
Hi everyone ! 
I'm interested to work on this one during next hackfest.
If no solution can be found during the next week, the BULAC would like to sponsor this development.
Thanks to all of you !
Comment 15 Jonathan Druart 2018-04-17 21:18:16 UTC
Created attachment 74393 [details] [review]
Bug 18426: Refactor subscriptions tables code in a BLOCK
Comment 16 Jonathan Druart 2018-04-17 21:18:20 UTC
Created attachment 74394 [details] [review]
Bug 18426: Allow to edit subscriptions in batch

This adds a checkbox column in serials-search.pl tables that allow to edit
selected subscriptions.
The following fields can be modified:
 - Bookseller
 - Location
 - Library
 - Item type
 - Public note
 - Nonpublic note
 - "Create item when receiving" flag
 - Expiration date
+ the additional fields defined in serials/add_fields.pl

Test plan:

1. Go to Serials module
2. If there is no additional fields defined, define some (at least one with an
   authorized value and one without)
3. Start a subscription search
4. Select some results using the checkboxes and click the "Edit" button above
   the table
5. Select values for some fields (not all) and click "Start batch edit"
6. Verify you are being redirected to the previous search results
7. Verify that the fields for which you selected a value were modified and that
   the others fields weren't
8. Repeat steps 4 to 7 but this time, modify the other fields.
Comment 17 Jonathan Druart 2018-04-17 21:18:23 UTC
Created attachment 74395 [details] [review]
Bug 18426: Fix datepicker
Comment 18 Jonathan Druart 2018-04-17 21:18:27 UTC
Created attachment 74396 [details] [review]
Bug 18426: Address QA issues

- Replaced with the "action" links we already have on the biblio detail
page and the items lost report.
- Display all the editable values in the table
- Add a 'Cancel' link
Comment 19 Jonathan Druart 2018-04-17 21:18:51 UTC
Trying to unstuck this one.

(In reply to Nick Clemens from comment #8)
> 1 - The edit button is inconsistent, it should be a link as the others, and
> I would suggest 'Edit selected serials'

Replaced with the "action" links we already have on the biblio detail page and the items lost report.

> 2 - On the edit page we should display all editable values - when the
> dropdown is 'No change' I cannot know what the original value was for things
> like 'Create items when receiving

Done.

> 3 - We should have a 'Cancel' button if we choose not to edit the subs

Done.

> 4 - It might be nice to be able to select/deselect subs before applying the
> edits - not a blocker but saves from going back a page on wrong click

TODO later I'd say

> 5 - Maybe 'Save' instead of 'Start batch edit'?

Done

(In reply to Katrin Fischer from comment #13)
> 1) Agree with Nick: "Edit" button would be nicer as a link, matching other
> pages like the details page.

So, done.

> 2) If you select no subscriptions and click on edit, there should be a
> warning. Or it should be inactive until you have selected some.

No longer valid.

> 3) It would be nice to have a checkbox like on the batch item edit in order
> to allow emptying the note fields. At the moment you'd have to add a space
> or something to clear them out.

We had lot of issue about that, I'd avoid to add it now.
Comment 20 Séverine Queune 2018-04-18 08:25:04 UTC
I just test it.
It works very well for all kind of fields.
I also have a few comments.

(In reply to Jonathan Druart from comment #19)
> 
> (In reply to Nick Clemens from comment #8)

> > 2 - On the edit page we should display all editable values - when the
> > dropdown is 'No change' I cannot know what the original value was for things
> > like 'Create items when receiving
> 
> Done.
> 
Some values are not displayed in the table :
  - item type
  - additional subscription fields

> > 5 - Maybe 'Save' instead of 'Start batch edit'?
> 
> Done
> 
Button is still 'Start batch edit'. I agree with a 'Save' button, that what we have on the 'Item batch modification'.


> (In reply to Katrin Fischer from comment #13)

> > 3) It would be nice to have a checkbox like on the batch item edit in order
> > to allow emptying the note fields. At the moment you'd have to add a space
> > or something to clear them out.
> 
> We had lot of issue about that, I'd avoid to add it now.
> 
Agree with Katrin, could be great to have this enhancement later.
Comment 21 Jonathan Druart 2018-04-18 16:17:52 UTC
Created attachment 74458 [details] [review]
Bug 18426: Address QA issues

- Replaced with the "action" links we already have on the biblio detail
page and the items lost report.
- Display all the editable values in the table
- Add a 'Cancel' link
Comment 22 Jonathan Druart 2018-04-18 16:19:13 UTC
(In reply to Séverine Queune from comment #20)

> Some values are not displayed in the table :
>   - item type
>   - additional subscription fields

I have added item type in the last version of the patch.
I consider the additional subscription fields out of my scope.

> > > 5 - Maybe 'Save' instead of 'Start batch edit'?
> > 
> > Done
> > 
> Button is still 'Start batch edit'. I agree with a 'Save' button, that what
> we have on the 'Item batch modification'.

Indeed, done now.
Comment 23 Séverine Queune 2018-04-18 18:27:59 UTC
(In reply to Jonathan Druart from comment #22)
> (In reply to Séverine Queune from comment #20)
> 
> > Some values are not displayed in the table :
> >   - item type
> >   - additional subscription fields
> 
> I have added item type in the last version of the patch.
> I consider the additional subscription fields out of my scope.
>
Ok for me for item type !
Let' open a new ticket for additionnal subscription fields :)
Comment 24 Katrin Fischer 2018-04-19 06:33:01 UTC
Hi Jonathan,

2 things:

1) We have a permission for editing existing subscriptions:
 edit_subscription Edit an existing subscription
I think we should hide everything related to the batch edit in the GUI, if users don't have this permission (checkboxes and links on top).

2) I'd use "Edit" instead of "Modify" to match with other patches/terminology in Koha.

Otherwise this works really well, would be good to see it make it over the finish line!
Comment 25 Jonathan Druart 2018-04-19 10:29:00 UTC
Created attachment 74536 [details] [review]
Bug 18426: Make sure the logged in user can edit the subscriptions
Comment 26 Jonathan Druart 2018-04-19 10:29:06 UTC
Created attachment 74537 [details] [review]
Bug 18426: Replace 'Modify' with 'Edit'
Comment 27 Jonathan Druart 2018-04-19 10:30:27 UTC
Indeed Katrin, thanks for catching the permission issue!
Comment 28 Katrin Fischer 2018-04-20 05:56:41 UTC
Something is not quite right yet. Testing with a staff patron that has all serial permissions but 'edit subscription'. I don't see the links, but I still have the checkboxes that now have no function.
Comment 29 Jonathan Druart 2018-04-20 14:22:02 UTC
(In reply to Katrin Fischer from comment #28)
> Something is not quite right yet. Testing with a staff patron that has all
> serial permissions but 'edit subscription'. I don't see the links, but I
> still have the checkboxes that now have no function.

You should remove superserials I guess.
Comment 30 Katrin Fischer 2018-04-21 09:52:54 UTC
Created attachment 74676 [details] [review]
Bug 18426: Refactor subscriptions tables code in a BLOCK

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 31 Katrin Fischer 2018-04-21 09:52:58 UTC
Created attachment 74677 [details] [review]
Bug 18426: Allow to edit subscriptions in batch

This adds a checkbox column in serials-search.pl tables that allow to edit
selected subscriptions.
The following fields can be modified:
 - Bookseller
 - Location
 - Library
 - Item type
 - Public note
 - Nonpublic note
 - "Create item when receiving" flag
 - Expiration date
+ the additional fields defined in serials/add_fields.pl

Test plan:

1. Go to Serials module
2. If there is no additional fields defined, define some (at least one with an
   authorized value and one without)
3. Start a subscription search
4. Select some results using the checkboxes and click the "Edit" button above
   the table
5. Select values for some fields (not all) and click "Start batch edit"
6. Verify you are being redirected to the previous search results
7. Verify that the fields for which you selected a value were modified and that
   the others fields weren't
8. Repeat steps 4 to 7 but this time, modify the other fields.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 32 Katrin Fischer 2018-04-21 09:53:03 UTC
Created attachment 74678 [details] [review]
Bug 18426: Fix datepicker

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 33 Katrin Fischer 2018-04-21 09:53:07 UTC
Created attachment 74679 [details] [review]
Bug 18426: Address QA issues

- Replaced with the "action" links we already have on the biblio detail
page and the items lost report.
- Display all the editable values in the table
- Add a 'Cancel' link

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 34 Katrin Fischer 2018-04-21 09:53:12 UTC
Created attachment 74680 [details] [review]
Bug 18426: Make sure the logged in user can edit the subscriptions

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 35 Katrin Fischer 2018-04-21 09:53:16 UTC
Created attachment 74681 [details] [review]
Bug 18426: Replace 'Modify' with 'Edit'

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 36 Jonathan Druart 2018-04-23 17:28:58 UTC
Pushed to master for 18.05, thanks to everybody involved!