Bug 9223 - Multiple values of AdvancedSearchTypes in suggestions
Summary: Multiple values of AdvancedSearchTypes in suggestions
Status: CLOSED INVALID
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low normal with 10 votes (vote)
Assignee: Alex Arnaud
QA Contact:
URL:
Keywords:
: 8036 (view as bug list)
Depends on:
Blocks: 7115 8036
  Show dependency treegraph
 
Reported: 2012-12-06 12:55 UTC by Fridolin Somers
Modified: 2016-12-05 21:23 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Proposed patch (4.05 KB, patch)
2012-12-06 13:04 UTC, Fridolin Somers
Details | Diff | Splinter Review
Proposed patch (main) (11.94 KB, patch)
2013-01-04 16:57 UTC, Fridolin Somers
Details | Diff | Splinter Review
Proposed patch (syspref add) (3.13 KB, patch)
2013-01-04 16:57 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 9223: Multiple values of AdvancedSearchTypes in suggestions (11.49 KB, patch)
2013-07-31 16:52 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 9223: Add SupportsAuthorizedValues syspref (followup 1) (3.40 KB, patch)
2013-07-31 16:52 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 9223 [Follow-up] Multiple values of AdvancedSearchTypes in suggestions (13.65 KB, patch)
2013-07-31 16:53 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 9223: Multiple values of AdvancedSearchTypes in suggestions (11.95 KB, patch)
2015-01-23 12:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9223: Add SupportsAuthorizedValues syspref (followup 1) (3.20 KB, patch)
2015-01-23 12:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9223 [Follow-up] Multiple values of AdvancedSearchTypes in suggestions (14.80 KB, patch)
2015-01-23 12:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9223: Add SupportsAuthorizedValues syspref (followup 1) (3.20 KB, patch)
2015-02-26 14:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9223 [Follow-up] Multiple values of AdvancedSearchTypes in suggestions (14.80 KB, patch)
2015-02-26 14:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9223: Multiple values of AdvancedSearchTypes in suggestions (12.02 KB, patch)
2015-03-06 13:19 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 9223: Add SupportsAuthorizedValues syspref (followup 1) (3.26 KB, patch)
2015-03-06 13:20 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 9223 [Follow-up] Multiple values of AdvancedSearchTypes in suggestions (15.04 KB, patch)
2015-03-06 13:23 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 9223 [Follow-up] add unit tests for GetSupportList and GetSupportName subroutine (3.95 KB, patch)
2015-06-09 08:01 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223: Multiple values of AdvancedSearchTypes in suggestions (12.03 KB, patch)
2015-06-09 08:09 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223: Add SupportsAuthorizedValues syspref (followup 1) (3.38 KB, patch)
2015-06-09 08:10 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223 [Follow-up] Multiple values of AdvancedSearchTypes in suggestions (15.05 KB, patch)
2015-06-09 08:10 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223 [Follow-up] change the way syspref SuggestionsUseValues is added: Use the new worflow (atomic update) (2.04 KB, patch)
2015-06-09 08:10 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223 [Follow-up] add unit tests for GetSupportList and GetSupportName subroutine (3.95 KB, patch)
2015-06-09 08:10 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223: Multiple values of AdvancedSearchTypes in suggestions (12.03 KB, patch)
2015-09-03 10:21 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223: Add SupportsAuthorizedValues syspref (followup 1) (3.38 KB, patch)
2015-09-03 10:21 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223 [Follow-up] Multiple values of AdvancedSearchTypes in suggestions (15.80 KB, patch)
2015-09-03 10:21 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223 [Follow-up] change the way syspref SuggestionsUseValues is added: Use the new worflow (atomic update) (2.04 KB, patch)
2015-09-03 10:21 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223 [Follow-up] add unit tests for GetSupportList and GetSupportName subroutine (4.00 KB, patch)
2015-09-03 10:21 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223: Multiple values of AdvancedSearchTypes in suggestions (12.03 KB, patch)
2015-09-03 14:55 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223: Add SupportsAuthorizedValues syspref (followup 1) (3.38 KB, patch)
2015-09-03 14:55 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223 [Follow-up] Multiple values of AdvancedSearchTypes in suggestions (15.07 KB, patch)
2015-09-03 14:55 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223 [Follow-up] change the way syspref SuggestionsUseValues is added: Use the new worflow (atomic update) (2.04 KB, patch)
2015-09-03 14:55 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223 [Follow-up] add unit tests for GetSupportList and GetSupportName subroutine (4.00 KB, patch)
2015-09-03 14:56 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223 [Follow-up] add unit tests for GetSupportList and GetSupportName subroutine (4.26 KB, patch)
2015-10-28 09:33 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223: Multiple values of AdvancedSearchTypes in suggestions (11.94 KB, patch)
2015-10-28 09:59 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223: Add SupportsAuthorizedValues syspref (followup 1) (3.38 KB, patch)
2015-10-28 09:59 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223 [Follow-up] Multiple values of AdvancedSearchTypes in suggestions (15.07 KB, patch)
2015-10-28 09:59 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223 [Follow-up] change the way syspref SuggestionsUseValues is added: Use the new worflow (atomic update) (2.04 KB, patch)
2015-10-28 09:59 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223 [Follow-up] add unit tests for GetSupportList and GetSupportName subroutine (4.26 KB, patch)
2015-10-28 10:00 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 9223 - Fix QA issues (798 bytes, patch)
2015-10-28 10:00 UTC, Alex Arnaud
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2012-12-06 12:55:36 UTC
Suggestion form uses C4::Koha::GetSupportList to get a list of supports that can come from itemtypes or authoritized values LOC or CCODE.
C4::Koha::GetSupportList uses AdvancedSearchTypes syspref like it has only one value.
But this syspref can have mutliple values separated by a pipe.
Comment 1 Fridolin Somers 2012-12-06 13:04:13 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2012-12-06 23:05:38 UTC
I am not sure if this changes makes sense for the library user. Collections, locations and itemtypes are totally different things. Which label would we show? What is the user expected to choose/do here?

Maybe it would be better to have a separate pref to define which value(s) are offered in the suggestion form and not reuse the settings for the advanced search.
Comment 3 Fridolin Somers 2012-12-10 13:28:13 UTC
(In reply to comment #2)
> Maybe it would be better to have a separate pref to define which value(s)
> are offered in the suggestion form and not reuse the settings for the
> advanced search.
I do agree.
I'd like to have other opinions before rewriting patch.
Comment 4 Katrin Fischer 2012-12-10 13:38:45 UTC
Hi Fridolyn, thx for putting this in discussion - hope others share what they think about it.
Comment 5 Nicole C. Engard 2012-12-12 16:19:05 UTC
I think that these options should be on the staff client, but not the OPAC. It's too confusing to the actual user (I would think).  Right now the form shows nothing ... http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8036
Comment 6 Katrin Fischer 2012-12-12 16:26:02 UTC
It's working correctly in 3.6.10, so I think we can't just remove the feature. I agree most of the time it's confusing, but I can imagine that the values might make sense for libraries that are using itemtypes and collection codes/locations in a special way. For example if your location is something like a department of your company. I checked, the element has an id - so it should be easy to hide with CSS:

<li><label for="itemtype">Item Type:</label><select name="itemtype" id="itemtype">
Comment 7 Nicole C. Engard 2012-12-12 16:28:58 UTC
I wasn't clear - an item type makes sense to the user. And it should be there and it should work. The collections and shelving locations though make so sense to the OPAC user when making a suggestion. How does the patron know where you want to shelve the item?

Nicole
Comment 8 Nicole C. Engard 2012-12-12 16:29:53 UTC
(In reply to comment #7)

I meant - make no sense - not 'so'.
Comment 9 Francois Charbonnier 2012-12-26 09:03:20 UTC
I guess someone used/is using the LOC or CCODE as itemtypes (maybe to display the information?) or thought it might suit everybody to get everything within one field...

Anyway, if Koha 3.6 can returns itemtypes / LOC and CCODE values, the patch for this bug should make it works the same way here.

Then, we should open up a "feature request" to change the behaviour.

I agree we should separate the advanced search and the suggestion droplist setups to avoid confusion for the patron.

If a library want to catalogue the location and the ccode at the suggestion level, there should be two new fields that would be display depending on a new syspref similar to AdvancedSearchTypes syspref.

And maybe, we could be wilder and add not just one syspref but two sysprefs to distinguish opac and staff suggestion droplist set ups! :^)
Comment 10 Fridolin Somers 2013-01-04 16:57:01 UTC Comment hidden (obsolete)
Comment 11 Fridolin Somers 2013-01-04 16:57:30 UTC Comment hidden (obsolete)
Comment 12 Fridolin Somers 2013-01-04 17:05:25 UTC
I have rewritten patch.

I introduce a new syspref to choose witch between item types, shelving location and collection code represents document physical support and will be used as 'document type' in suggestions.
I changed 'item type' in 'document type' in suggestions management to avoid misunderstanding. It was already used in suggestion creation form.

First patch is the code modifications, second is the syspref addition.

See commit comment for test plan.

This syspref may be used in other enhancements.
Comment 13 Katrin Fischer 2013-01-24 22:49:53 UTC
Hi Fridolyn, 

thx for working on this. 

I am not sure the point here is that the value in the pull down on the item form has to represent the physical document type. 

I could imagine a company library having offices/rooms as locations, using the pull down on the form to indicate where the book should be accessible later.

Also if item types are used, they could say something like: short loan, reference, normal loan and for some libraries it would make sense to have people indicate that on the suggestion form.

I liked the idea of having separate fields with the proper description, but I guess doing multiple is harder than having the library to choose one, because of the database structure.

Maybe we could just have one field and change the label according to what is selected in the pref? Also it might be worth having a value "don't show", because that would avoid having another pref (see 9468)
Comment 14 Katrin Fischer 2013-01-24 22:54:11 UTC
Sorry for being annoying about this, it's just some thoughts.
Comment 15 Owen Leonard 2013-01-28 14:59:29 UTC
(In reply to comment #7)
> I wasn't clear - an item type makes sense to the user. And it should be
> there and it should work. The collections and shelving locations though make
> no sense to the OPAC user when making a suggestion. How does the patron know
> where you want to shelve the item?

Item type may or may not make sense to the user depending on how the library uses it. In my library item type would make no sense, but collection code would.

I think this latest version of the patch works well, but I agree that label should change according to the selection. We shouldn't presume that the correct term ("item type," etc.) won't be understandable to the patron. Better to let the library decide to customize it somehow.

I also like the idea of a "don't show" option.
Comment 16 Owen Leonard 2013-02-05 16:43:40 UTC
Looking at this again, the other thing I worry about is the name of the preference and its description. The preference is tied directly to suggestions isn't it? But the name, "SupportsAuthorizedValues," doesn't imply a connection. The description, "(itemtype|ccode|loc) represents document physical support." doesn't make sense to me.

Maybe something like, "SuggestionsUseValues," "Suggestions can be submitted using the following authorized values: (itemtype|ccode|loc)"
Comment 17 Owen Leonard 2013-07-31 16:52:02 UTC Comment hidden (obsolete)
Comment 18 Owen Leonard 2013-07-31 16:52:17 UTC Comment hidden (obsolete)
Comment 19 Owen Leonard 2013-07-31 16:53:15 UTC Comment hidden (obsolete)
Comment 20 Owen Leonard 2013-07-31 16:55:00 UTC
I've attached rebased versions of the original patches along with mine. I'm setting back to "needs signoff" although I don't think this can go to QA without an additional followup for the issue I describe in comment 19.
Comment 21 Fridolin Somers 2013-08-07 15:08:00 UTC
*** Bug 8036 has been marked as a duplicate of this bug. ***
Comment 22 Fridolin Somers 2013-08-07 15:27:41 UTC
I'd like to discuss more on this bug.

How can the syspref concerne only suggestions when it is used in C4::Koha::GetSupportList() ?
This is the method doc :
"Returns an array ref containing informations about Support (since itemtype is rather a circulation code when item-level-itypes is used)."

This is the point, we need to have a syspref telling where to find the physical category of the record (itemtype or ccode).
This code is used for suggestions (both intranet and opac) and should be used in other places like opac-topissues.pl filter (actually also broken if AdvancedSearchTypes is multivalued).
Comment 23 Katrin Fischer 2013-08-07 15:49:28 UTC
Hi Fridolyn,

I think this assumption is not quite right:

>This is the point, we need to have a syspref telling where to find the physical >category of the record (itemtype or ccode).

There is not guarantee or reason that a library will use itemtype or ccode to define the physical category of a record. They can use it for whatever they like and from my experience they do use it in a lot of different ways.

I think we schould just make it a pref that defines what to show on the suggestion form with a correct label: Location, (Circulation?) type, Collection. And use that for suggestons in staff and OPAC.

Also it might make sense for a library to have different values used for suggestions and opac-topissues - so maybe we should not tie those together.
Comment 24 Fridolin Somers 2013-08-09 08:01:54 UTC
(In reply to Katrin Fischer from comment #23)
Hie Katrin,
Ok, I understand.
In this cas the code C4::Koha::GetSupportList() and C4::Koha::GetSupportName() must be moved into C4::Suggestion and renamed.
It is this code who speaks about a "support" definition.
Comment 25 I'm just a bot 2013-09-18 10:01:21 UTC
Applying: Bug 9223: Multiple values of AdvancedSearchTypes in suggestions
Using index info to reconstruct a base tree...
M	C4/Koha.pm
M	koha-tmpl/opac-tmpl/prog/en/modules/opac-suggestions.tt
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/opac-tmpl/prog/en/modules/opac-suggestions.tt
Auto-merging C4/Koha.pm
CONFLICT (content): Merge conflict in C4/Koha.pm
Patch failed at 0001 Bug 9223: Multiple values of AdvancedSearchTypes in suggestions
The copy of the patch that failed is found in:
   /home/christopher/git/koha/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Comment 26 Joseph DeVore 2013-09-21 16:44:12 UTC
Hi I have noticed this issue as well..
My local Library (Dover, NH USA) just switched over to Koha I have to say its sooo much better then that ancient Dinosaur DYNIX...

I chat with my Libraries tech lady almost everyday ^_^ so she directed me here...

Its nice too see a more integrated solution to requesting new material...


I(born in the year of the Dog, ie fault finder/(Loves Beta Testing ^_^)

some basic formats that should be listed include:
Books
Books  --  Large Print
Audio Books
DVDs
Manga

just to name a few...
I suppose a specific list of material types could be hard-codded on a per library basis... or maybe a new section in the database that can have a small list generated per library basis...


ps: just thought I would offer my thoughts loi ;D
Comment 27 Jonathan Druart 2015-01-23 12:53:11 UTC Comment hidden (obsolete)
Comment 28 Jonathan Druart 2015-01-23 12:53:20 UTC Comment hidden (obsolete)
Comment 29 Jonathan Druart 2015-01-23 12:53:25 UTC Comment hidden (obsolete)
Comment 30 Jonathan Druart 2015-01-23 12:54:20 UTC
I would like to see this one moving forward.
I rebased the 3 patches against master and adapted them to the bootstrap theme.
Comment 31 Joseph DeVore 2015-01-23 13:32:21 UTC
Hi Jonathan!

its nice to see some one finally working on this again =D
good luck.
Comment 32 Katrin Fischer 2015-01-25 19:41:44 UTC
Hi Joseph, 
this has the status 'needs signoff' now, which means it's waiting for a tester. Anyone can test and we have sandboxes, so you don't need a development environment to do it :)
Maybe you'd like to have a try - it would really help getting this further:
http://wiki.koha-community.org/wiki/Sandboxes
Comment 33 Joseph DeVore 2015-01-25 23:25:20 UTC
I took a quick look at this sandboxing.... 
seems a bit too much for me... that is the setup seem a bit complex so I don't really know what I would be doing...


I will pass this on the tech lady at my local library, and see if she can do anything to test it.
Comment 34 Jonathan Druart 2015-02-26 14:58:37 UTC Comment hidden (obsolete)
Comment 35 Jonathan Druart 2015-02-26 14:58:47 UTC Comment hidden (obsolete)
Comment 36 Jonathan Druart 2015-02-26 14:59:19 UTC
Last patches fixed easy conflict on updatedatabase.pl
Comment 37 Olli-Antti Kivilahti 2015-03-06 13:00:30 UTC
AFAIK:
The suggestions.itemtype-column is not used anywhere other than displayed as-is in the suggestions.pl-view.
I tried setting the itemtype-column to "ITYPE - CCODE - LOCATION" and it seems to work quite well.
I don't know why anybody would want to expose their patrons to ILS-configuration technical parameters like that, but using this simple trick it should be possible to collect and store all three values.

On a followup those three parameters can be separated to their distinctive columns.

Also using only one ITEMTYPE or COLLECTIONCODE should be more than enough for a borrower to categorize his suggestion.
I think the suggestions-mechanism is meant for ordinary library users and if further "resource" description accurancy is needed, then an acquisitions integration would be more suitable for the task using EDI or similar.


I feel sadness when a new system preference is added. In this case
"SupportsAuthorizedValues"||"SuggestionsUseValues".
A better solution would be to use the first value in "AdvancedSearchTypes", eg "ccode|itemtypes|loc" would translate to using "ccode" as the selection displayed for the user, or simply displaying all those options and concatenating them to the suggestions.itemtype-column.
Comment 38 Olli-Antti Kivilahti 2015-03-06 13:03:40 UTC
Other than that, this feature seems to work well enough.
Comment 39 Olli-Antti Kivilahti 2015-03-06 13:19:52 UTC Comment hidden (obsolete)
Comment 40 Olli-Antti Kivilahti 2015-03-06 13:20:32 UTC Comment hidden (obsolete)
Comment 41 Olli-Antti Kivilahti 2015-03-06 13:23:49 UTC Comment hidden (obsolete)
Comment 42 Olli-Antti Kivilahti 2015-03-06 13:24:59 UTC
Failed QA because no unit tests of any kind.
Comment 43 Alex Arnaud 2015-06-09 08:01:49 UTC Comment hidden (obsolete)
Comment 44 Alex Arnaud 2015-06-09 08:09:56 UTC Comment hidden (obsolete)
Comment 45 Alex Arnaud 2015-06-09 08:10:00 UTC Comment hidden (obsolete)
Comment 46 Alex Arnaud 2015-06-09 08:10:06 UTC Comment hidden (obsolete)
Comment 47 Alex Arnaud 2015-06-09 08:10:10 UTC Comment hidden (obsolete)
Comment 48 Alex Arnaud 2015-06-09 08:10:14 UTC Comment hidden (obsolete)
Comment 49 Alex Arnaud 2015-06-09 08:16:37 UTC
Hello,

I have rebased patches and make syspref adding use the new workflow (atomic update).
And i have created unit tests for GetSupportList and GetSupportName subroutine.
Comment 50 Alex Arnaud 2015-09-03 10:21:03 UTC Comment hidden (obsolete)
Comment 51 Alex Arnaud 2015-09-03 10:21:09 UTC Comment hidden (obsolete)
Comment 52 Alex Arnaud 2015-09-03 10:21:20 UTC Comment hidden (obsolete)
Comment 53 Alex Arnaud 2015-09-03 10:21:25 UTC Comment hidden (obsolete)
Comment 54 Alex Arnaud 2015-09-03 10:21:32 UTC Comment hidden (obsolete)
Comment 55 Alex Arnaud 2015-09-03 10:23:21 UTC
Patch rebased again
Comment 56 Alex Arnaud 2015-09-03 14:55:34 UTC Comment hidden (obsolete)
Comment 57 Alex Arnaud 2015-09-03 14:55:42 UTC Comment hidden (obsolete)
Comment 58 Alex Arnaud 2015-09-03 14:55:48 UTC Comment hidden (obsolete)
Comment 59 Alex Arnaud 2015-09-03 14:55:54 UTC Comment hidden (obsolete)
Comment 60 Alex Arnaud 2015-09-03 14:56:01 UTC Comment hidden (obsolete)
Comment 61 Koha Team University Lyon 3 2015-09-03 17:12:26 UTC
I have tested the different values of the new syspref "suggestionsUseValues".
In each case, in pro interface or in OPAC, the authorized values match with the choice in the syspref. And it's the description that is displayed in the dropdown.
I tried to modify the suggestions in the pro interface and it's Ok too.

If, "hide this option" is selected, none are in the form (tested in pro and opac too).

There's just one thing that could be much better : code are displayed in the list of suggestions (OPAC and PRO) and it would be more useful to have the description instead.
cf. cgi-bin/koha/suggestion/suggestion.pl and cgi-bin/koha/opac-suggestions.pl

But I don't want to block this patch for that.

Sonia
Comment 62 Jonathan Druart 2015-09-15 07:52:50 UTC
Please remove tabs and fix the author in the last patch.
Comment 63 Alex Arnaud 2015-10-28 09:33:53 UTC Comment hidden (obsolete)
Comment 64 Alex Arnaud 2015-10-28 09:39:23 UTC
Fixed last patch.

A rebase against master is coming soon
Comment 65 Alex Arnaud 2015-10-28 09:59:20 UTC
Created attachment 44113 [details] [review]
Bug 9223: Multiple values of AdvancedSearchTypes in suggestions

Suggestion form uses C4::Koha::GetSupportList to get a list of supports that can come from itemtypes or authoritized values LOC or CCODE. C4::Koha::GetSupportList uses AdvancedSearchTypes syspref like it has only one value. But this syspref can contain several values separated by a pipe.

This patch introduces a new syspref SupportsAuthorizedValues to select wich authorized values category that represents the physical support (itemtypes by default, loc or ccode). These authorized values will be used in suggestion management, known as 'document type'.
(Note that database field is still 'itemtype').

Test plan :
-----------
- Set SupportsAuthorizedValues with itemtypes or loc or ccode
- Get to suggestion form (OPAC or intranet)
- Look at "Document type" combobox, it must contains descriptions of selected authorized values
- Save the suggestion
- Get to intranet suggestions management
- Organize suggestions by document type
=> your suggestion must appear in a tab with its document type description

Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Comment 66 Alex Arnaud 2015-10-28 09:59:32 UTC
Created attachment 44114 [details] [review]
Bug 9223: Add SupportsAuthorizedValues syspref (followup 1)

Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Comment 67 Alex Arnaud 2015-10-28 09:59:44 UTC
Created attachment 44115 [details] [review]
Bug 9223 [Follow-up] Multiple values of AdvancedSearchTypes in suggestions

This follow-up implements some things I think will improve on the
previous patches:

- Change the system preference name to "SuggestionsUseValues"
- Change the description of the preference to "Suggestions can be
  submitted using the following authorized values:"
- Move the new preference to the OPAC tab so that it is grouped with
  other suggestions-related preferences.
- Add the option offer no itemtype/collection code chooser on the
  submission form.
- Display the correct label on the submission form depending on the
  value of the SuggestionsUseValues preference.

To test, apply this patch on top of the previous ones. Test submission
of purchase suggestions through the OPAC and staff client with the
SuggestionsUseValues preference set to all the various choices: item
type, collection code, shelving location, and 'none.' The suggestion
forms should look correct and work correctly.

HOWEVER, neither the previous patches nor mine address the problem of
the suggestions management page's "organize by" option, which is
hard-coded to work with item types. Whether or not my patch is
acceptable this issue will need to be addressed.

Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>

  Tested the TEST PLAN. With ccode, itype, none, loc.
  On both OPAC and SC.
  Making a code review, but it is hard during the hackfest.
  Morbid lack of any kind of unit test!
Comment 68 Alex Arnaud 2015-10-28 09:59:56 UTC
Created attachment 44116 [details] [review]
Bug 9223 [Follow-up] change the way syspref SuggestionsUseValues is added: Use the new worflow (atomic update)
Comment 69 Alex Arnaud 2015-10-28 10:00:06 UTC
Created attachment 44117 [details] [review]
Bug 9223 [Follow-up] add unit tests for GetSupportList and GetSupportName subroutine
Comment 70 Alex Arnaud 2015-10-28 10:00:15 UTC
Created attachment 44118 [details] [review]
Bug 9223 - Fix QA issues
Comment 71 Katrin Fischer 2015-11-16 22:17:24 UTC
Sorry, but this patch no longer applies:

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 9223: Multiple values of AdvancedSearchTypes in suggestions
Using index info to reconstruct a base tree...
M	C4/Koha.pm
M	koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt
M	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-suggestions.tt
M	opac/opac-suggestions.pl
M	suggestion/suggestion.pl
Falling back to patching base and 3-way merge...
Auto-merging suggestion/suggestion.pl
CONFLICT (content): Merge conflict in suggestion/suggestion.pl
Auto-merging opac/opac-suggestions.pl
CONFLICT (content): Merge conflict in opac/opac-suggestions.pl
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-suggestions.tt
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-suggestions.tt
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
Auto-merging C4/Koha.pm
Failed to merge in the changes.
Patch failed at 0001 Bug 9223: Multiple values of AdvancedSearchTypes in suggestions
The copy of the patch that failed is found in:
   /home/katrin/kohaclone/.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-9223-Multiple-values-of-AdvancedSearchTypes-in-GRz_gw.patch
Comment 72 Alex Arnaud 2015-12-29 13:20:40 UTC
Is this Bug still relevant since we use SUGGEST_FORMAT authorized values as  document type ?
Comment 73 Alex Arnaud 2015-12-29 13:21:44 UTC
see bug 9468
Comment 74 Katrin Fischer 2016-02-15 22:53:14 UTC
I think this is no longer needed with SUGGEST_FORMAT - please reopen if I am missing something. Marking INVALID as we opted for another different solution with the new authorized value.