Bug 20067 - Wrong display of authorised value for items.materials on staff detail page
Summary: Wrong display of authorised value for items.materials on staff detail page
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Katrin Fischer
QA Contact: Testopia
URL:
Keywords:
Depends on: 17250
Blocks:
  Show dependency treegraph
 
Reported: 2018-01-22 19:48 UTC by Marjorie Barry-Vila
Modified: 2019-10-14 19:56 UTC (History)
8 users (show)

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


Attachments
notice view (184.21 KB, image/png)
2018-01-22 19:48 UTC, Marjorie Barry-Vila
Details
item table (61.84 KB, image/png)
2018-01-22 19:50 UTC, Marjorie Barry-Vila
Details
authorized values (109.39 KB, image/png)
2018-01-22 19:50 UTC, Marjorie Barry-Vila
Details
Bug 20067: Fix false display of authorised value for materials on staff detail page (1.54 KB, patch)
2018-02-23 08:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20067: Fix other checks for linked authorised value categories (7.50 KB, patch)
2018-02-23 08:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20067: Fix false display of authorised value for materials on staff detail page (1.61 KB, patch)
2018-02-23 13:30 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20067: Fix other checks for linked authorised value categories (7.68 KB, patch)
2018-02-23 13:30 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20067: Fix false display of authorised value for materials on staff detail page (1.66 KB, patch)
2018-03-16 08:34 UTC, Jesse Maseto
Details | Diff | Splinter Review
Bug 20067: Fix other checks for linked authorised value categories (7.70 KB, patch)
2018-03-16 08:34 UTC, Jesse Maseto
Details | Diff | Splinter Review
Bug 20067: Fix false display of authorised value for materials on staff detail page (1.72 KB, patch)
2018-03-23 14:05 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20067: Fix other checks for linked authorised value categories (7.76 KB, patch)
2018-03-23 14:05 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marjorie Barry-Vila 2018-01-22 19:48:46 UTC
Created attachment 70807 [details]
notice view

Hi,

Is it normal contents of authorised values of the ORDER_CANCELLATION category are displayed in items table for the materials specified fields?

Test plan:
1. Fill with 2 in a subfield 952 $3 and save item
2. Display item table in normal view
3. See column Materials specified which display authorised values instead of number 2

See attached images

Regards
Marjorie
Comment 1 Marjorie Barry-Vila 2018-01-22 19:50:14 UTC
Created attachment 70808 [details]
item table
Comment 2 Marjorie Barry-Vila 2018-01-22 19:50:43 UTC
Created attachment 70809 [details]
authorized values
Comment 3 Katrin Fischer 2018-02-19 07:12:05 UTC
Hi Marjorie, can you check in your framework for 952$3 which authorised value it's mapped to?
Comment 4 Marjorie Barry-Vila 2018-02-19 14:15:06 UTC
(In reply to Katrin Fischer from comment #3)
> Hi Marjorie, can you check in your framework for 952$3 which authorised
> value it's mapped to?

Hi Katrin,
There is no mapped authorised value to the 952 $ 3 field.
have you been able to reproduce the problem of your board?

Regards,
Marjorie
Comment 5 Katrin Fischer 2018-02-19 14:23:13 UTC
Hi Marjorie, are you sure? Your screenshots shows numeric values for materials, that's why I was asking. Do you enter them manually?
Comment 6 Marjorie Barry-Vila 2018-02-19 14:44:34 UTC
(In reply to Katrin Fischer from comment #5)
> Hi Marjorie, are you sure? Your screenshots shows numeric values for
> materials, that's why I was asking. Do you enter them manually?

Oh yes, i entered them manually. If i write "vol.1", it's correct but 
if I write only the numbers it displays the text of the authorized value of ORDER_CANCELLATION_REASON

Marjorie
Comment 7 Katrin Fischer 2018-02-19 14:57:00 UTC
Last question: Which version exactly? I will try to test :)
Comment 8 Marjorie Barry-Vila 2018-02-19 15:32:24 UTC
(In reply to Katrin Fischer from comment #7)
> Last question: Which version exactly? I will try to test :)

Tested on 16.11.00 and 17.05.05.

Thanks!

Marjorie
Comment 9 Katrin Fischer 2018-02-22 22:24:09 UTC
Hi Marjorie,

can confirm this with the sample data on master!

To test:
- make sure sample data and configuration is loaded
- edit any item
- write 2 in the $3 materials specificed
- check the details page normal view - it will display 'restocking'
Comment 10 Katrin Fischer 2018-02-22 22:28:24 UTC
I think the error is in detail.pl around here:

$mss = Koha::MarcSubfieldStructures->search({ frameworkcode => $fw, kohafield => 'items.materials', authorised_value => { not => undef } });
my %materials_map;
use Data::Dumper; warn Dumper $mss->count;
if ($mss->count) {
    my $materials_authvals = GetAuthorisedValues($mss->next->authorised_value);
    if ($materials_authvals) {
        foreach my $value (@$materials_authvals) {
            $materials_map{$value->{authorised_value}} = $value->{lib};
        }
    }
}
Comment 11 Jonathan Druart 2018-02-22 22:58:26 UTC
Caused by commit 0b6f129ac32e5ea3a2d1357d8e19ba3e24519757
    Bug 17250: Do not retrieve marc subfield structure when the authorised value is not defined
Comment 12 Jonathan Druart 2018-02-22 22:59:57 UTC
(In reply to Jonathan Druart from comment #11)
> Caused by commit 0b6f129ac32e5ea3a2d1357d8e19ba3e24519757
>     Bug 17250: Do not retrieve marc subfield structure when the authorised
> value is not defined

Or maybe before...
Comment 13 Katrin Fischer 2018-02-22 23:24:07 UTC
What we need is:
select authorised_value from marc_subfield_structure where frameworkcode = "BKS" and kohafield = "items.materials" and authorised_value is not null and authorised_value != "";

But I am failing to translate this correctly at the moment.
Comment 14 Katrin Fischer 2018-02-23 08:23:01 UTC
Created attachment 72100 [details] [review]
Bug 20067: Fix false display of authorised value for materials on staff detail page

Koha didn't check for a linked authorised value category for
items.materials correctly which led to displaying false information
on the detail page.

To test:
- before applying the patch:
- make sure sample data and configuration is loaded
- edit any item
- write 2 in the $3 materials specificed
- check the details page normal view - it will display 'restocking'
- apply patch
- check display, it should now show "2"
- link 952$3 (MARC21) to ORDER_CANCELLATION_REASON
- check display again, it should now show 'restocking'
Comment 15 Katrin Fischer 2018-02-23 08:23:05 UTC
Created attachment 72101 [details] [review]
Bug 20067: Fix other checks for linked authorised value categories

The same pattern was used in other files, this patch fixes it.
Comment 16 Katrin Fischer 2018-02-23 08:23:43 UTC
After getting some sleep... :)
Comment 17 Jonathan Druart 2018-02-23 12:38:36 UTC
you defined twice the same key "!=" in the condition hashref.
Comment 18 Jonathan Druart 2018-02-23 12:42:05 UTC
Koha::MarcSubfieldStructures->search(
    {   
        frameworkcode => '', 
        kohafield     => 'items.materials',
        -or           => [
            { authorised_value => { not  => undef } },
            { authorised_value => { '!=' => '' } } 
        ]   
    }   
);

will generate 
SELECT `me`.`tagfield`, `me`.`tagsubfield`, `me`.`liblibrarian`, `me`.`libopac`, `me`.`repeatable`, `me`.`mandatory`, `me`.`kohafield`, `me`.`tab`, `me`.`authorised_value`, `me`.`authtypecode`, `me`.`value_builder`, `me`.`isurl`, `me`.`hidden`, `me`.`frameworkcode`, `me`.`seealso`, `me`.`link`, `me`.`defaultvalue`, `me`.`maxlength` FROM `marc_subfield_structure` `me` WHERE ( ( ( `authorised_value` IS NOT NULL OR `authorised_value` != ? ) AND `frameworkcode` = ? AND `kohafield` = ? ) ): '', '', 'items.materials'

Which is what we want.
Comment 19 Katrin Fischer 2018-02-23 13:30:49 UTC
Created attachment 72120 [details] [review]
Bug 20067: Fix false display of authorised value for materials on staff detail page

Koha didn't check for a linked authorised value category for
Koha didn't check for a linked authorised value category for
items.materials correctly which led to displaying false information
on the detail page.

To test:
- before applying the patch:
- make sure sample data and configuration is loaded
- edit any item
- write 2 in the $3 materials specificed
- check the details page normal view - it will display 'restocking'
- apply patch
- check display, it should now show "2"
- link 952$3 (MARC21) to ORDER_CANCELLATION_REASON
- check display again, it should now show 'restocking'
Comment 20 Katrin Fischer 2018-02-23 13:30:52 UTC
Created attachment 72121 [details] [review]
Bug 20067: Fix other checks for linked authorised value categories

The same pattern was used in other files, this patch fixes it.
Comment 21 delaye 2018-03-14 15:00:12 UTC
the tile of the ticket is not good.
it is rather 'Display the description authorised valeur is you mapping with items.materials
Comment 22 Jesse Maseto 2018-03-16 08:34:36 UTC
Created attachment 72991 [details] [review]
Bug 20067: Fix false display of authorised value for materials on staff detail page

Koha didn't check for a linked authorised value category for
Koha didn't check for a linked authorised value category for
items.materials correctly which led to displaying false information
on the detail page.

To test:
- before applying the patch:
- make sure sample data and configuration is loaded
- edit any item
- write 2 in the $3 materials specificed
- check the details page normal view - it will display 'restocking'
- apply patch
- check display, it should now show "2"
- link 952$3 (MARC21) to ORDER_CANCELLATION_REASON
- check display again, it should now show 'restocking'

Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>
Comment 23 Jesse Maseto 2018-03-16 08:34:57 UTC
Created attachment 72992 [details] [review]
Bug 20067: Fix other checks for linked authorised value categories

The same pattern was used in other files, this patch fixes it.

Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>
Comment 24 Nick Clemens 2018-03-23 14:05:43 UTC
Created attachment 73181 [details] [review]
Bug 20067: Fix false display of authorised value for materials on staff detail page

Koha didn't check for a linked authorised value category for
Koha didn't check for a linked authorised value category for
items.materials correctly which led to displaying false information
on the detail page.

To test:
- before applying the patch:
- make sure sample data and configuration is loaded
- edit any item
- write 2 in the $3 materials specificed
- check the details page normal view - it will display 'restocking'
- apply patch
- check display, it should now show "2"
- link 952$3 (MARC21) to ORDER_CANCELLATION_REASON
- check display again, it should now show 'restocking'

Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 25 Nick Clemens 2018-03-23 14:05:47 UTC
Created attachment 73182 [details] [review]
Bug 20067: Fix other checks for linked authorised value categories

The same pattern was used in other files, this patch fixes it.

Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 26 Jonathan Druart 2018-03-26 20:37:37 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 27 Nick Clemens 2018-04-24 01:35:06 UTC
Awesome work all, backported to stable for 17.11.05
Comment 28 Fridolin Somers 2018-04-24 15:06:42 UTC
Conflict for 17.05.x in circ/circulation.pl.
I will not risk to release in 17.05.11