Bug 23594

Summary: Add ability to batch modify itemtypes from the suggestions page
Product: Koha Reporter: Séverine Queune <severine.queune>
Component: AcquisitionsAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Katrin Fischer <katrin.fischer>
Severity: enhancement    
Priority: P5 - low CC: 1joynelson, jonathan.druart, katrin.fischer, martin.renvoize, oleonard, wizzyrea
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24536
Change sponsored?: Sponsored Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This enhancement allows users to update the item types for selected suggestions on the suggestions management page.
Version(s) released in:
20.05.00
Bug Depends on: 18743, 23855    
Bug Blocks: 23590    
Attachments: Bug 23594: Batch modification for itemtypes on suggestions page
Bug 23594: Fix [un]checkall links
Bug 23594: Restore previous view after delete or update itemtype
Bug 23594: Batch modification for itemtypes on suggestions page
Bug 23594: Fix [un]checkall links
Bug 23594: Restore previous view after delete or update itemtype
Bug 23594: Batch modification for itemtypes on suggestions page
Bug 23594: Fix [un]checkall links
Bug 23594: Restore previous view after delete or update itemtype
Bug 23594: Add missing html filters
Bug 23594: Fix bad resolution conflict with bug 16784
Bug 23594: Fix bad resolution conflict with bug 16784
Bug 23594: Batch modification for itemtypes on suggestions page
Bug 23594: Fix [un]checkall links
Bug 23594: Restore previous view after delete or update itemtype
Bug 23594: Add missing html filters
Bug 23594: Fix bad resolution conflict with bug 16784

Description Séverine Queune 2019-09-11 13:20:01 UTC
The BULAC (http://www.bulac.fr/) would like to sponsor an enhancement to allow a batch modification of ‘suggestions.itemtype’ (depending on authorized values SUGGEST_FORMAT list) at the bottom of the page /cgi-bin/koha/suggestion/suggestion.pl, like it exists for suggestions.status et suggestions.reason datas.

As long as “Change sponsored?” is set on “Seeking developer”, anyone wanting to quote us for the development of such a feature or just ask for more information is welcome to write to us: <sigb@bulac.fr>.
Comment 1 Jonathan Druart 2019-10-21 16:13:04 UTC
Created attachment 94469 [details] [review]
Bug 23594: Batch modification for itemtypes on suggestions page

This new enhancement adds the ability to update the item types for
selected suggestions on the suggestions management page
(suggestion/suggestion.pl)

To achieve this goal we needed to refresh a bit the template, in order
to get rid of weird code. To not recreate the previous coding awkwardness
some code has been rewritten (mainly the removal of suggestiontype)

Test plan:
- Create some suggestions
- On the suggestions management page, select some of them and a new item
types.
- Submit the form and confirm that the itemtype of the suggestions has
been updated

- Also you should confirm that the "delete" and "change status" still
work as before
Comment 2 Séverine Queune 2019-10-22 07:58:53 UTC
Hi Jonathan !
I can't apply patch on sandbox :

<h1>Something went wrong !</h1>Applying: Bug 23594: Batch modification for itemtypes on suggestions page
.git/rebase-apply/patch:345: trailing whitespace.
} 
fatal: sha1 information is lacking or useless (suggestion/suggestion.pl).
error: could not build fake ancestor
Patch failed at 0001 Bug 23594: Batch modification for itemtypes on suggestions page
Comment 3 Jonathan Druart 2019-10-22 08:44:22 UTC
It works for me with the following order:

Bug 23594: Batch modification for itemtypes on suggestions page
Bug 23855: Fix "Mark selected suggestion as" status
Bug 18743: Correct number of pending suggestions (acq home page)
Bug 18743: Fix suggestion listing when organized by library
Comment 4 Jonathan Druart 2019-10-22 10:01:39 UTC
Created attachment 94520 [details] [review]
Bug 23594: Fix [un]checkall links
Comment 5 Jonathan Druart 2019-10-22 10:01:42 UTC
Created attachment 94521 [details] [review]
Bug 23594: Restore previous view after delete or update itemtype

We want to restore the previous view when suggestions are deleted or
their itemtypes are updated.
To avoid c/p the code is moved to a new subroutine.
Comment 6 Séverine Queune 2019-10-29 14:04:21 UTC
Created attachment 94834 [details] [review]
Bug 23594: Batch modification for itemtypes on suggestions page

This new enhancement adds the ability to update the item types for
selected suggestions on the suggestions management page
(suggestion/suggestion.pl)

To achieve this goal we needed to refresh a bit the template, in order
to get rid of weird code. To not recreate the previous coding awkwardness
some code has been rewritten (mainly the removal of suggestiontype)

Test plan:
- Create some suggestions
- On the suggestions management page, select some of them and a new item
types.
- Submit the form and confirm that the itemtype of the suggestions has
been updated

- Also you should confirm that the "delete" and "change status" still
work as before

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 7 Séverine Queune 2019-10-29 14:04:24 UTC
Created attachment 94835 [details] [review]
Bug 23594: Fix [un]checkall links

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 8 Séverine Queune 2019-10-29 14:04:27 UTC
Created attachment 94836 [details] [review]
Bug 23594: Restore previous view after delete or update itemtype

We want to restore the previous view when suggestions are deleted or
their itemtypes are updated.
To avoid c/p the code is moved to a new subroutine.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 9 Liz Rea 2020-01-03 16:02:27 UTC
This still applies, QA?
Comment 10 Katrin Fischer 2020-01-20 21:52:13 UTC
Hi Jonathan, can you please rebase?

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 23594: Batch modification for itemtypes on suggestions page
.git/rebase-apply/patch:345: trailing whitespace.
}  
warning: 1 line adds whitespace errors.
Using index info to reconstruct a base tree...
M	koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt
M	suggestion/suggestion.pl
.git/rebase-apply/patch:345: trailing whitespace.
}  
warning: 1 line adds whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging suggestion/suggestion.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt
error: Failed to merge in the changes.
Patch failed at 0001 Bug 23594: Batch modification for itemtypes on suggestions page
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-23594-Batch-modification-for-itemtypes-on-sugg-BKdhvf.patch
Comment 11 Jonathan Druart 2020-01-21 13:22:43 UTC
Created attachment 97663 [details] [review]
Bug 23594: Batch modification for itemtypes on suggestions page

This new enhancement adds the ability to update the item types for
selected suggestions on the suggestions management page
(suggestion/suggestion.pl)

To achieve this goal we needed to refresh a bit the template, in order
to get rid of weird code. To not recreate the previous coding awkwardness
some code has been rewritten (mainly the removal of suggestiontype)

Test plan:
- Create some suggestions
- On the suggestions management page, select some of them and a new item
types.
- Submit the form and confirm that the itemtype of the suggestions has
been updated

- Also you should confirm that the "delete" and "change status" still
work as before

Sponsored-by: BULAC - http://www.bulac.fr/

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 12 Jonathan Druart 2020-01-21 13:22:48 UTC
Created attachment 97664 [details] [review]
Bug 23594: Fix [un]checkall links

Sponsored-by: BULAC - http://www.bulac.fr/

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 13 Jonathan Druart 2020-01-21 13:22:52 UTC
Created attachment 97665 [details] [review]
Bug 23594: Restore previous view after delete or update itemtype

We want to restore the previous view when suggestions are deleted or
their itemtypes are updated.
To avoid c/p the code is moved to a new subroutine.

Sponsored-by: BULAC - http://www.bulac.fr/

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 14 Jonathan Druart 2020-01-21 13:23:22 UTC
Conflict with bug 16784 fixed.
Comment 15 Katrin Fischer 2020-01-22 07:09:07 UTC
Sorry Joubu, some fails on the QA test tools:

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt
   FAIL	  filters
		wrong_html_filter at line 473 (            <a href="#tab_[% loop.count %]">)
		missing_filter at line 473 (            <a href="#tab_[% loop.count %]">)
		missing_filter at line 497 (<div id="tab_[% loop.count %]">)
		missing_filter at line 498 (<form class="update_suggestions" name="f" method="post" action="/cgi-bin/koha/suggestion/suggestion.pl#tab_[% loop.count %]">)
		missing_filter at line 502 (    <table id="table_[% loop.count %]" class="sorted">)
		missing_filter at line 648 (                    <label for="delete_[% loop.count %]">Delete selected</label>)
   FAIL	  forbidden patterns
		forbidden pattern: Attribute type should not be used for script tags (bug 20053) (line 840)
   OK	  git manipulation
   OK	  js_in_body
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template
Comment 16 Jonathan Druart 2020-01-22 09:06:24 UTC
Created attachment 97693 [details] [review]
Bug 23594: Add missing html filters

Also fixes the following QA failure:
forbidden pattern: Attribute type should not be used for script tags (bug 20053) (line 840)
Comment 17 Katrin Fischer 2020-01-23 07:22:48 UTC
Hi Jonathan,

the patch breaks the new datatables functionality in suggestions. :(

I also really believe that we have too many different forms on this page - I think we need to at least not add another submit button and should combine the batch editing options into one form (status, reason + new: itemtype). The 3 parts at the bottom.

Another note: I believe it would be really helpful to see the itemtype in the table when you can bulk change it. At the moment, you can only see it by viewing the suggestions which means you are 'blindly' bulk editing.

Adding Owen for design advice.
Comment 18 Jonathan Druart 2020-01-23 09:03:08 UTC
Created attachment 97781 [details] [review]
Bug 23594: Fix bad resolution conflict with bug 16784
Comment 19 Jonathan Druart 2020-01-23 09:04:03 UTC
(In reply to Katrin Fischer from comment #17)
> Hi Jonathan,
> 
> the patch breaks the new datatables functionality in suggestions. :(

Fixed with last patch.

> I also really believe that we have too many different forms on this page - I
> think we need to at least not add another submit button and should combine
> the batch editing options into one form (status, reason + new: itemtype).
> The 3 parts at the bottom.

I already contacted Owen before starting this dev. I advised me the current view.

> Another note: I believe it would be really helpful to see the itemtype in
> the table when you can bulk change it. At the moment, you can only see it by
> viewing the suggestions which means you are 'blindly' bulk editing.

The itemtype is displayed already (inside the column 'Suggestion')
Comment 20 Katrin Fischer 2020-01-23 09:11:17 UTC
I think I thought it deserved its own colum (maybe to be fixed separately :) )

Could it have been a misunderstanding? It seems really odd to not be able to update itemtypes and status in one go?
Comment 21 Jonathan Druart 2020-01-23 09:35:02 UTC
(In reply to Katrin Fischer from comment #20)
> I think I thought it deserved its own colum (maybe to be fixed separately :)
> )
> 
> Could it have been a misunderstanding? It seems really odd to not be able to
> update itemtypes and status in one go?

The problem is the ugliness of both script and template. Which makes the whole thing super hard to modify.
I am planning to clean that (suggestion related code) soon.
That should help to improve UI of this module.
Comment 22 Owen Leonard 2020-01-23 14:11:40 UTC
I agree about the item type going into its own column. I think the fact that it's a field that can be batch updated means it should be easier to see.

I agree that there are a lot of forms on the page but I don't think it's unreasonable. I do think a nice addition could be to hide the batch update forms until a selection has been made. But that could be a follow-up.
Comment 23 Katrin Fischer 2020-01-29 22:01:52 UTC
Created attachment 98094 [details] [review]
Bug 23594: Fix bad resolution conflict with bug 16784

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 24 Katrin Fischer 2020-01-29 22:02:20 UTC
Created attachment 98095 [details] [review]
Bug 23594: Batch modification for itemtypes on suggestions page

This new enhancement adds the ability to update the item types for
selected suggestions on the suggestions management page
(suggestion/suggestion.pl)

To achieve this goal we needed to refresh a bit the template, in order
to get rid of weird code. To not recreate the previous coding awkwardness
some code has been rewritten (mainly the removal of suggestiontype)

Test plan:
- Create some suggestions
- On the suggestions management page, select some of them and a new item
types.
- Submit the form and confirm that the itemtype of the suggestions has
been updated

- Also you should confirm that the "delete" and "change status" still
work as before

Sponsored-by: BULAC - http://www.bulac.fr/

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 25 Katrin Fischer 2020-01-29 22:02:25 UTC
Created attachment 98096 [details] [review]
Bug 23594: Fix [un]checkall links

Sponsored-by: BULAC - http://www.bulac.fr/

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 26 Katrin Fischer 2020-01-29 22:02:30 UTC
Created attachment 98097 [details] [review]
Bug 23594: Restore previous view after delete or update itemtype

We want to restore the previous view when suggestions are deleted or
their itemtypes are updated.
To avoid c/p the code is moved to a new subroutine.

Sponsored-by: BULAC - http://www.bulac.fr/

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 27 Katrin Fischer 2020-01-29 22:02:34 UTC
Created attachment 98098 [details] [review]
Bug 23594: Add missing html filters

Also fixes the following QA failure:
forbidden pattern: Attribute type should not be used for script tags (bug 20053) (line 840)

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 28 Katrin Fischer 2020-01-29 22:02:38 UTC
Created attachment 98099 [details] [review]
Bug 23594: Fix bad resolution conflict with bug 16784

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 29 Katrin Fischer 2020-01-29 22:03:32 UTC
Filed bug 24536 for adding a new column for itemtype.
Comment 30 Martin Renvoize 2020-01-30 11:03:56 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 31 Joy Nelson 2020-02-10 23:20:12 UTC
Feature not backported to 19.11.x