See: http://screencast.com/t/PoGPO0PB If you have an authorized value for the public notes field on the item record it shows the description when editing items, but when you view the normal view in the opac and the staff client it shows the authorized value code and that means nothing to the public - it should show the description for authorized values.
Created attachment 7641 [details] [review] Bug 7515 - authorized value code showing in opac for public note This alters C4::Items::GetItemsInfo() to look up the authorised value for the itemnotes value if the field has been assigned an authorised value rather than being a free text field.
Created attachment 7643 [details] [review] Bug 7515 - authorized value code showing in opac for public note This alters C4::Items::GetItemsInfo() to look up the authorised value for the itemnotes value if the field has been assigned an authorised value rather than being a free text field.
Created attachment 7644 [details] [review] [SIGNED-OFF] Bug 7515 - authorized value code showing in opac for public note This alters C4::Items::GetItemsInfo() to look up the authorised value for the itemnotes value if the field has been assigned an authorised value rather than being a free text field. Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>
This patch doesn't differentiate between the OPAC description (opaclib) and the staff client description (lib, which is used here). This means that the public will see the staffside description, which is counter to expectations. Marking as Failed QA.
Created attachment 7702 [details] [review] Bug 7515 - authorized value code showing in opac for public note This alters C4::Items::GetItemsInfo() to look up the authorised value for the itemnotes value if the field has been assigned an authorised value rather than being a free text field. A new boolean parameter $opac has been added to let GetItemsInfo know if the data is destined for the opac or intranet view. Defaults to intranet view.
Updated the patch to take into account whether the data is to be used for the intranet or the opac.
QA comment: In GetItemsInfo I think that all added code can be replaced by two already existing subs: my $sql_itemnotes = " SELECT authorised_value FROM marc_subfield_structure WHERE kohafield = 'items.itemnotes' AND frameworkcode = ? "; my $sth_itemnotes = $dbh->prepare( $sql_itemnotes ); $sth_itemnotes->execute( $data->{'frameworkcode'} ); my ($authorised_valuecode) = $sth_itemnotes->fetchrow; can be replaced by: my ($authorised_valuecode) = C4::Koha::GetAuthValCode('items.itemnotes', $data->{frameworkcode}); and my $lib = $opac ? 'lib_opac' : 'lib'; if ($authorised_valuecode) { $sth_itemnotes = $dbh->prepare( "SELECT $lib FROM authorised_values WHERE category=? AND authorised_value=?" ); $sth_itemnotes->execute( $authorised_valuecode, $data->{'itemnotes'} ); my ($lib) = $sth_itemnotes->fetchrow; $data->{'itemnotes'} = $lib; } can be replaced by: $data->{itemnotes} = C4::Koha::GetKohaAuthorisedValueLib($authorised_valuecode, $data->{itemnotes}, $opac); Does anybody agree?
Created attachment 7833 [details] [review] Bug 7515 - authorized value code showing in opac for public note This alters C4::Items::GetItemsInfo() to look up the authorised value for the itemnotes value if the field has been assigned an authorised value rather than being a free text field. A new boolean parameter $opac has been added to let GetItemsInfo know if the data is destined for the opac or intranet view. Defaults to intranet view.
Updated patch to take into account the suggestions by Julian Maurice.
Created attachment 7893 [details] [review] [SIGNED-OFF] Bug 7515 - authorized value code showing in opac for public note This alters C4::Items::GetItemsInfo() to look up the authorised value for the itemnotes value if the field has been assigned an authorised value rather than being a free text field. A new boolean parameter $opac has been added to let GetItemsInfo know if the data is destined for the opac or intranet view. Defaults to intranet view. Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>
QA comment: * my pre-apply hook complains: "my" variable $authorised_valuecode masks earlier declaration in same scope at /home/paul/koha.dev/koha-community//C4/Items.pm line 1264. Looking at the code, you're adding + my ($authorised_valuecode) = C4::Koha::GetAuthValCode('items.itemnotes', $data->{frameworkcode}); and $authorised_valuecode is declared a few line before. Removing the my is probably enough, please fix, test & resubmit. (the code is fine otherwise)
Created attachment 7918 [details] [review] Bug 7515 - authorized value code showing in opac for public note This alters C4::Items::GetItemsInfo() to look up the authorised value for the itemnotes value if the field has been assigned an authorised value rather than being a free text field. A new boolean parameter $opac has been added to let GetItemsInfo know if the data is destined for the opac or intranet view. Defaults to intranet view.
Created attachment 7924 [details] [review] [SIGNED-OFF] Bug 7515 - authorized value code showing in opac for public note This alters C4::Items::GetItemsInfo() to look up the authorised value for the itemnotes value if the field has been assigned an authorised value rather than being a free text field. A new boolean parameter $opac has been added to let GetItemsInfo know if the data is destined for the opac or intranet view. Defaults to intranet view. Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> Tested in staff client and the OPAC and the note shows the description that is appropriate for that page (OPAC or staff client).
QA Comment: Filling itemnotes in Items.pm: Shouldn't you test if GetAuthValCode returned something? If it is not an authorized value, you are overwriting it, isn't it?
Created attachment 8182 [details] [review] Bug 7515 - Followup - Only set itemnotes to authorised value if said value exists.
After applying the two patches (was I only supposed to do the followup?) above and doing a search I get : Software error: Global symbol "$itemnotes" requires explicit package name at /home/nengard/kohaclone/C4/Items.pm line 1267. Compilation failed in require at /home/nengard/kohaclone/C4/Reserves.pm line 29. BEGIN failed--compilation aborted at /home/nengard/kohaclone/C4/Reserves.pm line 29. Compilation failed in require at /home/nengard/kohaclone/C4/Circulation.pm line 26. BEGIN failed--compilation aborted at /home/nengard/kohaclone/C4/Circulation.pm line 26. Compilation failed in require at /home/nengard/kohaclone/C4/Overdues.pm line 26. BEGIN failed--compilation aborted at /home/nengard/kohaclone/C4/Overdues.pm line 26. Compilation failed in require at /home/nengard/kohaclone/C4/Members.pm line 30. BEGIN failed--compilation aborted at /home/nengard/kohaclone/C4/Members.pm line 30. Compilation failed in require at /home/nengard/kohaclone/C4/Search.pm line 28. BEGIN failed--compilation aborted at /home/nengard/kohaclone/C4/Search.pm line 28. Compilation failed in require at /home/nengard/kohaclone/catalogue/search.pl line 146. BEGIN failed--compilation aborted at /home/nengard/kohaclone/catalogue/search.pl line 146. For help, please send mail to the webmaster (webmaster@debian), giving this error message and the time and date of the error.
Created attachment 8214 [details] [review] Bug 7515 - Followup - Only set itemnotes to authorised value if said value exists.
i have crete une link between items.itemnotes(995$u : UNIMARc) and authorised_values I create an item In the opac the description of the authorised value display it's work!
This patch looks to be a duplicate of bug 7379 if i'm not mistaking. I think we should find a generic solution and not add specific code. So switching to in discussion and also linking to bug 7525
(In reply to comment #19) > This patch looks to be a duplicate of bug 7379 if i'm not mistaking. I think > we should find a generic solution and not add specific code. > > So switching to in discussion and also linking to bug 7525 This is not a duplicate of bug 7379, but it is a similar situation. That one was pushed, so I don't see a reason why we shouldn't push this one too. Could we solve this at the template level using Koha::Template::Plugin::KohaAuthorisedValues? I think that would be an elegant solution, but we may need to add a more subroutine to it for various situations.
(In reply to comment #19) > This patch looks to be a duplicate of bug 7379 if i'm not mistaking. I think > we should find a generic solution and not add specific code. > > So switching to in discussion and also linking to bug 7525 Paul, can you give me some more input on this? I think there are a few solutions: 1) Solve at the .pl level: i.e. patch as we go along in the perl code ( i.e. this bug's patches ) 2) Solve at the .tt level: i.e. use Koha::Template::Plugin::KohaAuthorisedValues and patch as we go along at the template level. We can patch this sub to check for an authorised value mapping and return either the correct mapping or the original passed in data. The will greatly simplify the amount of code necessary to support the use of authorised values mapping. 3) Solve at the .pm level: i.e. all subs that send back data from biblio, biblioitems or items will first pass that data through a new sub ReplaceWithAuthorisedValues that will check each field to see if it is mapped to an authorised value list and replace the code with the data if necessary. I personally think choice 2 is will be the quickest, lightest weight solution.
(In reply to Kyle M Hall from comment #21) > 2) Solve at the .tt level: i.e. use > Koha::Template::Plugin::KohaAuthorisedValues and patch as we go along at the > template level. We can patch this sub to check for an authorised value > mapping and return either the correct mapping or the original passed in > data. The will greatly simplify the amount of code necessary to support the > use of authorised values mapping. I prefer this option.
(In reply to Galen Charlton from comment #22) > > 2) Solve at the .tt level: i.e. use > > Koha::Template::Plugin::KohaAuthorisedValues and patch as we go along at the > > template level. > I prefer this option. This is very easily done, but we don't have a default authorized value category for public note and nonpublic note. Are these defaults we want to add?