Bugzilla – Attachment 183088 Details for
Bug 39971
Patron attribute types form logic should be reusable
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 39971: Move patron_attributes_form method to a class
Bug-39971-Move-patronattributesform-method-to-a-cl.patch (text/plain), 9.48 KB, created by
David Nind
on 2025-06-07 01:20:29 UTC
(
hide
)
Description:
Bug 39971: Move patron_attributes_form method to a class
Filename:
MIME Type:
Creator:
David Nind
Created:
2025-06-07 01:20:29 UTC
Size:
9.48 KB
patch
obsolete
>From b30aab116f1996729aa90cec59fd5b11003d1d8a Mon Sep 17 00:00:00 2001 >From: Pedro Amorim <pedro.amorim@openfifth.co.uk> >Date: Wed, 21 May 2025 12:52:56 +0000 >Subject: [PATCH] Bug 39971: Move patron_attributes_form method to a class > >This makes it reusable, documentable, and testable. > >This is changing code without adding a new feature or fixing a bug. >The first test plan should be to just make sure that the existing functionality is kept intact, i.e. rendering and saving the patron attribute types in the form. > >As to why we need this, we're maintaining the ILL Actions plugin where we have a feature to quickly add a patron. This is designed to be as minimal as possible, but if there are any mandatory patron attribute types, those need to be considered. >To test this: >1) Apply patches and enable ILLModule syspref. >2) Install the plugin: >https://github.com/openfifth/koha-plugin-ill-actions/releases/tag/v2.4.0 >3) Add a mandatory patron attribute type, you can edit the existing one in k-t-d, visit: > <staff_url>/cgi-bin/koha/admin/patron-attr-types.pl?op=edit_attribute_type&code=SHOW_BCODE >5) Tick the 'Staff interface mandatory' checkbox >6) Create a new ILL request, visit: > <staff_url>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=Standard >7) Notice there's a '+Add new user' link next to the cardnumber input at the bottom of the form. Click that. >8) Notice there's a 'Additional attributes and identifiers' section. This is only possible (without writing it again) because of the patches here. > >Signed-off-by: David Nind <david@davidnind.com> >--- > Koha/Patron/Attribute/Types.pm | 85 ++++++++++++++++++++++++++++++++++ > members/memberentry.pl | 75 +----------------------------- > 2 files changed, 86 insertions(+), 74 deletions(-) > >diff --git a/Koha/Patron/Attribute/Types.pm b/Koha/Patron/Attribute/Types.pm >index e131e73db7..5a35882083 100644 >--- a/Koha/Patron/Attribute/Types.pm >+++ b/Koha/Patron/Attribute/Types.pm >@@ -18,6 +18,7 @@ package Koha::Patron::Attribute::Types; > use Modern::Perl; > > use Koha::Patron::Attribute::Type; >+use C4::Koha qw( GetAuthorisedValues ); > > use base qw(Koha::Objects Koha::Objects::Limit::Library); > >@@ -27,6 +28,90 @@ Koha::Patron::Attribute::Types Object set class > > =head1 API > >+=head2 Class Methods >+ >+=cut >+ >+=head3 patron_attributes_form >+ >+ my $patron_attributes_form = Koha::Patron::Attribute::Types::patron_attributes_form >+ >+ Static method that returns patron attribute types to be rendered in a form >+ >+=cut >+ >+sub patron_attributes_form { >+ my $template = shift; >+ my $attributes = shift; >+ my $op = shift; >+ >+ my $library_id = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef; >+ my $attribute_types = Koha::Patron::Attribute::Types->search_with_library_limits( {}, {}, $library_id ); >+ if ( $attribute_types->count == 0 ) { >+ $template->param( no_patron_attribute_types => 1 ); >+ return; >+ } >+ >+ # map patron's attributes into a more convenient structure >+ my %attr_hash = (); >+ foreach my $attr (@$attributes) { >+ push @{ $attr_hash{ $attr->{code} } }, $attr; >+ } >+ >+ my @attribute_loop = (); >+ my $i = 0; >+ my %items_by_class; >+ while ( my ($attr_type) = $attribute_types->next ) { >+ my $entry = { >+ class => $attr_type->class(), >+ code => $attr_type->code(), >+ description => $attr_type->description(), >+ repeatable => $attr_type->repeatable(), >+ category => $attr_type->authorised_value_category(), >+ category_code => $attr_type->category_code(), >+ mandatory => $attr_type->mandatory(), >+ is_date => $attr_type->is_date(), >+ }; >+ if ( exists $attr_hash{ $attr_type->code() } ) { >+ foreach my $attr ( @{ $attr_hash{ $attr_type->code() } } ) { >+ my $newentry = {%$entry}; >+ $newentry->{value} = $attr->{attribute}; >+ $newentry->{use_dropdown} = 0; >+ if ( $attr_type->authorised_value_category() ) { >+ $newentry->{use_dropdown} = 1; >+ $newentry->{auth_val_loop} = >+ C4::Koha::GetAuthorisedValues( $attr_type->authorised_value_category(), $attr->{attribute} ); >+ } >+ $i++; >+ undef $newentry->{value} if ( $attr_type->unique_id() && $op eq 'duplicate' ); >+ $newentry->{form_id} = "patron_attr_$i"; >+ push @{ $items_by_class{ $attr_type->class() } }, $newentry; >+ } >+ } else { >+ $i++; >+ my $newentry = {%$entry}; >+ if ( $attr_type->authorised_value_category() ) { >+ $newentry->{use_dropdown} = 1; >+ $newentry->{auth_val_loop} = C4::Koha::GetAuthorisedValues( $attr_type->authorised_value_category() ); >+ } >+ $newentry->{form_id} = "patron_attr_$i"; >+ push @{ $items_by_class{ $attr_type->class() } }, $newentry; >+ } >+ } >+ for my $class ( sort keys %items_by_class ) { >+ my $av = Koha::AuthorisedValues->search( { category => 'PA_CLASS', authorised_value => $class } ); >+ my $lib = $av->count ? $av->next->lib : $class; >+ push @attribute_loop, { >+ class => $class, >+ items => $items_by_class{$class}, >+ lib => $lib, >+ }; >+ } >+ >+ $template->param( patron_attributes => \@attribute_loop ); >+ >+} >+ > =head2 Internal methods > > =cut >diff --git a/members/memberentry.pl b/members/memberentry.pl >index 5846b3b1c6..a81ac3a7fc 100755 >--- a/members/memberentry.pl >+++ b/members/memberentry.pl >@@ -29,7 +29,6 @@ use CGI qw ( -utf8 ); > use C4::Auth qw( get_template_and_user haspermission ); > use C4::Context; > use C4::Output qw( output_and_exit output_and_exit_if_error output_html_with_http_headers ); >-use C4::Koha qw( GetAuthorisedValues ); > use C4::Letters qw( GetPreparedLetter EnqueueLetter SendQueuedMessages ); > use C4::Form::MessagingPreferences; > use Koha::AuthUtils; >@@ -834,7 +833,7 @@ if ( C4::Context->preference('uppercasesurnames') ) { > } > > if ( C4::Context->preference('ExtendedPatronAttributes') ) { >- patron_attributes_form( $template, $extended_patron_attributes, $op ); >+ Koha::Patron::Attribute::Types::patron_attributes_form( $template, $extended_patron_attributes, $op ); > } > > if ( C4::Context->preference('EnhancedMessagingPreferences') ) { >@@ -920,78 +919,6 @@ sub parse_extended_patron_attributes { > return \@attr; > } > >-sub patron_attributes_form { >- my $template = shift; >- my $attributes = shift; >- my $op = shift; >- >- my $library_id = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef; >- my $attribute_types = Koha::Patron::Attribute::Types->search_with_library_limits( {}, {}, $library_id ); >- if ( $attribute_types->count == 0 ) { >- $template->param( no_patron_attribute_types => 1 ); >- return; >- } >- >- # map patron's attributes into a more convenient structure >- my %attr_hash = (); >- foreach my $attr (@$attributes) { >- push @{ $attr_hash{ $attr->{code} } }, $attr; >- } >- >- my @attribute_loop = (); >- my $i = 0; >- my %items_by_class; >- while ( my ($attr_type) = $attribute_types->next ) { >- my $entry = { >- class => $attr_type->class(), >- code => $attr_type->code(), >- description => $attr_type->description(), >- repeatable => $attr_type->repeatable(), >- category => $attr_type->authorised_value_category(), >- category_code => $attr_type->category_code(), >- mandatory => $attr_type->mandatory(), >- is_date => $attr_type->is_date(), >- }; >- if ( exists $attr_hash{ $attr_type->code() } ) { >- foreach my $attr ( @{ $attr_hash{ $attr_type->code() } } ) { >- my $newentry = {%$entry}; >- $newentry->{value} = $attr->{attribute}; >- $newentry->{use_dropdown} = 0; >- if ( $attr_type->authorised_value_category() ) { >- $newentry->{use_dropdown} = 1; >- $newentry->{auth_val_loop} = >- GetAuthorisedValues( $attr_type->authorised_value_category(), $attr->{attribute} ); >- } >- $i++; >- undef $newentry->{value} if ( $attr_type->unique_id() && $op eq 'duplicate' ); >- $newentry->{form_id} = "patron_attr_$i"; >- push @{ $items_by_class{ $attr_type->class() } }, $newentry; >- } >- } else { >- $i++; >- my $newentry = {%$entry}; >- if ( $attr_type->authorised_value_category() ) { >- $newentry->{use_dropdown} = 1; >- $newentry->{auth_val_loop} = GetAuthorisedValues( $attr_type->authorised_value_category() ); >- } >- $newentry->{form_id} = "patron_attr_$i"; >- push @{ $items_by_class{ $attr_type->class() } }, $newentry; >- } >- } >- for my $class ( sort keys %items_by_class ) { >- my $av = Koha::AuthorisedValues->search( { category => 'PA_CLASS', authorised_value => $class } ); >- my $lib = $av->count ? $av->next->lib : $class; >- push @attribute_loop, { >- class => $class, >- items => $items_by_class{$class}, >- lib => $lib, >- }; >- } >- >- $template->param( patron_attributes => \@attribute_loop ); >- >-} >- > sub add_guarantors { > my ( $patron, $input ) = @_; > >-- >2.39.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 39971
:
182739
|
182740
|
182741
|
182742
|
183087
| 183088 |
183089
|
183090