/svc/cataloguing/framework fetches the frameworks and needed authorised values. Currently the authorised values are fetched incorrectly fields suing authorised values don't generate the correct helpers To recreate: 1 - Map a marc field to an authorised value in the default framework - say 300$c -> CCODE 2 - Open the advanced cataloguing editor 3 - Create a new field 300$c - note there is no dropdown 4 - browse to: /cgi-bin/koha/svc/cataloguing/framework?callback=define 5 - Note the many instance of Koha::Schema::ResultSet::AuthorisedValueCategory->HASH...
Created attachment 75291 [details] [review] Bug 20760: Correctly fetch authorised values for rancor To test: 1 - Map a marc field to an authorised value in the default framework - say 300$c -> CCODE 2 - Open the advanced cataloguing editor 3 - Create a new field 300$c - note there is no dropdown 4 - browse to: /cgi-bin/koha/svc/cataloguing/framework?callback=define 5 - Note the many instance of Koha::Schema::ResultSet::AuthorisedValueCategory->HASH... 6 - Apply patch 7 - Restart memcached and plack 8 - reload/recreate record in rancor 9 - Note that 300$c is now a dropdown as expected 10 - repeate 4 11 - note the authorised values look correct in response
Comment on attachment 75291 [details] [review] Bug 20760: Correctly fetch authorised values for rancor Review of attachment 75291 [details] [review]: ----------------------------------------------------------------- ::: svc/cataloguing/framework @@ +53,4 @@ > } > > my $branch_limit = C4::Context->userenv ? C4::Context->userenv->{"branch"} : ""; > +my $results = Koha::AuthorisedValues->search({ branchcode => $branch_limit },{ order_by => "lib" }); Will this behave the same if branch_limit is q{}? Perhaps an if to define the passed hashref would be safer?
Putting to In Discussion until comment #2 is answered.
I think you should "just" call C4::Koha::GetAuthorisedValues, it deals with library limitations, and results are cached.
Created attachment 75779 [details] [review] [ALTERNATIVE PATCH] Bug 20760: Fill authorised values in svc framework
I still do not see the dropdown lists but I think it's the way to go.
Comment on attachment 75779 [details] [review] [ALTERNATIVE PATCH] Bug 20760: Fill authorised values in svc framework Review of attachment 75779 [details] [review]: ----------------------------------------------------------------- ::: svc/cataloguing/framework @@ +53,5 @@ > } > > +my $avs = C4::Koha::GetAuthorisedValues(); > +for my $av ( @$avs ) { > + push @{ $authorised_values->{$av->{category}} }, { value => $av->{authorised_value}, lib => $av->{lib} }; The other version had order_by lib. This uses order by category first. Perhaps add a sort?
(In reply to M. Tompsett from comment #7) > Comment on attachment 75779 [details] [review] [review] > [ALTERNATIVE PATCH] Bug 20760: Fill authorised values in svc framework > > Review of attachment 75779 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: svc/cataloguing/framework > @@ +53,5 @@ > > } > > > > +my $avs = C4::Koha::GetAuthorisedValues(); > > +for my $av ( @$avs ) { > > + push @{ $authorised_values->{$av->{category}} }, { value => $av->{authorised_value}, lib => $av->{lib} }; > > The other version had order_by lib. This uses order by category first. > Perhaps add a sort? I do not see the point, read again the 3 lines you are quoting ;)
(In reply to M. Tompsett from comment #7) > Comment on attachment 75779 [details] [review] [review] > [ALTERNATIVE PATCH] Bug 20760: Fill authorised values in svc framework > > Review of attachment 75779 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: svc/cataloguing/framework > @@ +53,5 @@ > > } > > > > +my $avs = C4::Koha::GetAuthorisedValues(); > > +for my $av ( @$avs ) { > > + push @{ $authorised_values->{$av->{category}} }, { value => $av->{authorised_value}, lib => $av->{lib} }; > > The other version had order_by lib. This uses order by category first. > Perhaps add a sort? I do not think it is needed. We are pushing into @{$hash->{$category}}, and template site we are using the keys of this hash separately. I will not help to sort by category.
Created attachment 76003 [details] [review] Bug 20760: Fill authorised values in svc framework To test: 1 - Map a marc field to an authorised value in the default framework - say 300$c -> CCODE 2 - Open the advanced cataloguing editor 3 - Create a new field 300$c - note there is no dropdown 4 - browse to: /cgi-bin/koha/svc/cataloguing/framework?callback=define 5 - Note the many instance of Koha::Schema::ResultSet::AuthorisedValueCategory->HASH... 6 - Apply patch 7 - Restart memcached and plack 8 - reload/recreate record in rancor 9 - Note that 300$c is now a dropdown as expected 10 - repeate 4 11 - note the authorised values look correct in response Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
I agree with Joubu, we don't need the sort and his patch is better (as usual, *sigh*)
Created attachment 76004 [details] [review] Bug 20760: Fill authorised values in svc framework To test: 1 - Map a marc field to an authorised value in the default framework - say 300$c -> CCODE 2 - Open the advanced cataloguing editor 3 - Create a new field 300$c - note there is no dropdown 4 - browse to: /cgi-bin/koha/svc/cataloguing/framework?callback=define 5 - Note the many instance of Koha::Schema::ResultSet::AuthorisedValueCategory->HASH... 6 - Apply patch 7 - Restart memcached and plack 8 - reload/recreate record in rancor 9 - Note that 300$c is now a dropdown as expected 10 - repeate 4 11 - note the authorised values look correct in response Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Works as expected and passes all tests. I'm happy with it and Passing QA
Awesome work all! Pushed to master for 18.11
Pushed to 18.05.x for 18.05.01
Pushed to 17.11.x for 17.11.07