|
Description
Fridolin Somers
2012-12-06 12:55:36 UTC
Created attachment 13903 [details] [review] Proposed patch Proposed patch : C4::Koha::GetSupportList returns a list with all supports from itemtypes, LOC and CCODE. Test plan : ----------- - Set AdvancedSearchTypes with "itemtypes|loc|ccode" - Get to suggestion form (OPAC or intranet) - Look at "Document type" combobox, it must contain all itemtypes, LOC and CCODE descriptions - Create a suggestion with a document type from LOC or CCODE - Get to suggestions management - Organize suggestions by item type => your suggestion must appear in a tab with ccode description 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. (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. Hi Fridolyn, thx for putting this in discussion - hope others share what they think about it. 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 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"> 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 (In reply to comment #7) I meant - make no sense - not 'so'. 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! :^) Created attachment 14427 [details] [review] Proposed patch (main) Created attachment 14428 [details] [review] Proposed patch (syspref add) 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. 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) Sorry for being annoying about this, it's just some thoughts. (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. 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)" Created attachment 20015 [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 http://bugs.koha-community.org/show_bug.cgi?id=6473 Created attachment 20016 [details] [review] Bug 9223: Add SupportsAuthorizedValues syspref (followup 1) http://bugs.koha-community.org/show_bug.cgi?id=6473 Created attachment 20017 [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. http://bugs.koha-community.org/show_bug.cgi?id=6473 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. *** Bug 8036 has been marked as a duplicate of this bug. *** 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). 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.
(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. 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". 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 Created attachment 35492 [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 Created attachment 35493 [details] [review] Bug 9223: Add SupportsAuthorizedValues syspref (followup 1) Created attachment 35494 [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. I would like to see this one moving forward. I rebased the 3 patches against master and adapted them to the bootstrap theme. Hi Jonathan! its nice to see some one finally working on this again =D good luck. 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 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. Created attachment 36178 [details] [review] Bug 9223: Add SupportsAuthorizedValues syspref (followup 1) Created attachment 36179 [details] [review] Bug 9223 [Follow-up] Multiple values of AdvancedSearchTypes in suggestions Last patches fixed easy conflict on updatedatabase.pl 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. Other than that, this feature seems to work well enough. Created attachment 36702 [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> Created attachment 36703 [details] [review] Bug 9223: Add SupportsAuthorizedValues syspref (followup 1) Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Created attachment 36704 [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! Failed QA because no unit tests of any kind. Created attachment 40011 [details] [review] Bug 9223 [Follow-up] add unit tests for GetSupportList and GetSupportName subroutine Created attachment 40012 [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> Created attachment 40013 [details] [review] Bug 9223: Add SupportsAuthorizedValues syspref (followup 1) Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Created attachment 40014 [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! Created attachment 40015 [details] [review] Bug 9223 [Follow-up] change the way syspref SuggestionsUseValues is added: Use the new worflow (atomic update) Created attachment 40016 [details] [review] Bug 9223 [Follow-up] add unit tests for GetSupportList and GetSupportName subroutine 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. Created attachment 42287 [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> Created attachment 42288 [details] [review] Bug 9223: Add SupportsAuthorizedValues syspref (followup 1) Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Created attachment 42289 [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! Created attachment 42290 [details] [review] Bug 9223 [Follow-up] change the way syspref SuggestionsUseValues is added: Use the new worflow (atomic update) Created attachment 42291 [details] [review] Bug 9223 [Follow-up] add unit tests for GetSupportList and GetSupportName subroutine Patch rebased again Created attachment 42336 [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> Created attachment 42337 [details] [review] Bug 9223: Add SupportsAuthorizedValues syspref (followup 1) Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Created attachment 42338 [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! Created attachment 42339 [details] [review] Bug 9223 [Follow-up] change the way syspref SuggestionsUseValues is added: Use the new worflow (atomic update) Created attachment 42340 [details] [review] Bug 9223 [Follow-up] add unit tests for GetSupportList and GetSupportName subroutine 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 Please remove tabs and fix the author in the last patch. Created attachment 44111 [details] [review] Bug 9223 [Follow-up] add unit tests for GetSupportList and GetSupportName subroutine Fixed last patch. A rebase against master is coming soon 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> Created attachment 44114 [details] [review] Bug 9223: Add SupportsAuthorizedValues syspref (followup 1) Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> 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! Created attachment 44116 [details] [review] Bug 9223 [Follow-up] change the way syspref SuggestionsUseValues is added: Use the new worflow (atomic update) Created attachment 44117 [details] [review] Bug 9223 [Follow-up] add unit tests for GetSupportList and GetSupportName subroutine Created attachment 44118 [details] [review] Bug 9223 - Fix QA issues 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 Is this Bug still relevant since we use SUGGEST_FORMAT authorized values as document type ? see bug 9468 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. |