It is not used in the code.
(In reply to Josef Moravec from comment #0) > It is not used in the code. Actually it is used on letter.pl
Created attachment 73083 [details] [review] Bug 20444: Use Koha::Patron::Attribute::Types object for getting patron attributes in letter.pl Test plan: 0) Apply the patch 1) Do not have any patron attribute types defined 2) Go to Tools -> Notices and slips 3) Edit any slip/letter which uses table borrower 4) Confirm the editing works, and the slip/letter itself works 5) Add some patron attributes types and define them a value for any patron 6) Go to Tools -> Notices and slips and edit any slip/letter which uses table borrower 7) Confirm, the attributes are listed in available fields 8) Confirm the slip/letter is working when you add a attribute to it
Created attachment 73084 [details] [review] Bug 20444: Update test 0) Apply this patch 1) prove t/db_dependent/Members/Attributes.t should return green
Created attachment 73085 [details] [review] Bug 20444: Remove sub GetAttributes 0) Apply this patch 1) git grep GetAttributes should return no occurencies
Comment on attachment 73083 [details] [review] Bug 20444: Use Koha::Patron::Attribute::Types object for getting patron attributes in letter.pl Review of attachment 73083 [details] [review]: ----------------------------------------------------------------- ::: tools/letter.pl @@ +463,4 @@ > } > } > if ($table eq 'borrowers') { > + my $attribute_types = Koha::Patron::Attribute::Types->search; This let's you loop like an array, but it isn't ordered by code as far as I can tell. Feel free to correct me. Does this need to be code equivalent? Or just similar functionally? @@ -462,4 @@ > } > } > if ($table eq 'borrowers') { > - if ( my $attributes = C4::Members::Attributes::GetAttributes() ) { This is a sorted array of codes...
Created attachment 74788 [details] [review] Bug 20444: Use Koha::Patron::Attribute::Types object for getting patron attributes in letter.pl Test plan: 0) Apply the patch 1) Do not have any patron attribute types defined 2) Go to Tools -> Notices and slips 3) Edit any slip/letter which uses table borrower 4) Confirm the editing works, and the slip/letter itself works 5) Add some patron attributes types and define them a value for any patron 6) Go to Tools -> Notices and slips and edit any slip/letter which uses table borrower 7) Confirm, the attributes are listed in available fields 8) Confirm the slip/letter is working when you add a attribute to it
Created attachment 74789 [details] [review] Bug 20444: Update test 0) Apply this patch 1) prove t/db_dependent/Members/Attributes.t should return green
Created attachment 74790 [details] [review] Bug 20444: Remove sub GetAttributes 0) Apply this patch 1) git grep GetAttributes should return no occurencies
(In reply to M. Tompsett from comment #5) > Comment on attachment 73083 [details] [review] [review] > Bug 20444: Use Koha::Patron::Attribute::Types object for getting patron > attributes in letter.pl > > Review of attachment 73083 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: tools/letter.pl > @@ +463,4 @@ > > } > > } > > if ($table eq 'borrowers') { > > + my $attribute_types = Koha::Patron::Attribute::Types->search; > > This let's you loop like an array, but it isn't ordered by code as far as I > can tell. Feel free to correct me. Does this need to be code equivalent? Or > just similar functionally? > > @@ -462,4 @@ > > } > > } > > if ($table eq 'borrowers') { > > - if ( my $attributes = C4::Members::Attributes::GetAttributes() ) { > > This is a sorted array of codes... Fixed for preserving functionality
Created attachment 74793 [details] [review] Bug 20444: Use Koha::Patron::Attribute::Types object for getting patron attributes in letter.pl Test plan: 0) Apply the patch 1) Do not have any patron attribute types defined 2) Go to Tools -> Notices and slips 3) Edit any slip/letter which uses table borrower 4) Confirm the editing works, and the slip/letter itself works 5) Add some patron attributes types and define them a value for any patron 6) Go to Tools -> Notices and slips and edit any slip/letter which uses table borrower 7) Confirm, the attributes are listed in available fields 8) Confirm the slip/letter is working when you add a attribute to it Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 74794 [details] [review] Bug 20444: Update test 0) Apply this patch 1) prove t/db_dependent/Members/Attributes.t should return green Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 74795 [details] [review] Bug 20444: Remove sub GetAttributes 0) Apply this patch 1) git grep GetAttributes should return no occurencies Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
I tried with several notices (predue, suggestion accepted, article request completed) and it always worked.
Created attachment 74906 [details] [review] Bug 20444: Use Koha::Patron::Attribute::Types object for getting patron attributes in letter.pl Test plan: 0) Apply the patch 1) Do not have any patron attribute types defined 2) Go to Tools -> Notices and slips 3) Edit any slip/letter which uses table borrower 4) Confirm the editing works, and the slip/letter itself works 5) Add some patron attributes types and define them a value for any patron 6) Go to Tools -> Notices and slips and edit any slip/letter which uses table borrower 7) Confirm, the attributes are listed in available fields 8) Confirm the slip/letter is working when you add a attribute to it Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 74907 [details] [review] Bug 20444: Update test 0) Apply this patch 1) prove t/db_dependent/Members/Attributes.t should return green Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 74908 [details] [review] Bug 20444: Remove sub GetAttributes 0) Apply this patch 1) git grep GetAttributes should return no occurencies Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 18.05, thanks to everybody involved!