Bug 20444 - Remove C4::Members::Attributes::GetAttributes
Summary: Remove C4::Members::Attributes::GetAttributes
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Josef Moravec
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 20443
  Show dependency treegraph
 
Reported: 2018-03-19 08:40 UTC by Josef Moravec
Modified: 2019-10-14 19:56 UTC (History)
3 users (show)

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


Attachments
Bug 20444: Use Koha::Patron::Attribute::Types object for getting patron attributes in letter.pl (1.91 KB, patch)
2018-03-19 09:35 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20444: Update test (2.36 KB, patch)
2018-03-19 09:35 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20444: Remove sub GetAttributes (1.17 KB, patch)
2018-03-19 09:35 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20444: Use Koha::Patron::Attribute::Types object for getting patron attributes in letter.pl (1.98 KB, patch)
2018-04-24 08:15 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20444: Update test (2.36 KB, patch)
2018-04-24 08:15 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20444: Remove sub GetAttributes (1.17 KB, patch)
2018-04-24 08:15 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 20444: Use Koha::Patron::Attribute::Types object for getting patron attributes in letter.pl (2.13 KB, patch)
2018-04-24 09:34 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 20444: Update test (2.50 KB, patch)
2018-04-24 09:34 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 20444: Remove sub GetAttributes (1.32 KB, patch)
2018-04-24 09:34 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 20444: Use Koha::Patron::Attribute::Types object for getting patron attributes in letter.pl (2.22 KB, patch)
2018-04-27 08:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 20444: Update test (2.60 KB, patch)
2018-04-27 08:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 20444: Remove sub GetAttributes (1.41 KB, patch)
2018-04-27 08:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Josef Moravec 2018-03-19 08:40:04 UTC
It is not used in the code.
Comment 1 Josef Moravec 2018-03-19 09:30:29 UTC
(In reply to Josef Moravec from comment #0)
> It is not used in the code.

Actually it is used on letter.pl
Comment 2 Josef Moravec 2018-03-19 09:35:45 UTC
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
Comment 3 Josef Moravec 2018-03-19 09:35:48 UTC
Created attachment 73084 [details] [review]
Bug 20444: Update test

0) Apply this patch
1) prove t/db_dependent/Members/Attributes.t
should return green
Comment 4 Josef Moravec 2018-03-19 09:35:51 UTC
Created attachment 73085 [details] [review]
Bug 20444: Remove sub GetAttributes

0) Apply this patch
1) git grep GetAttributes should return no occurencies
Comment 5 Mark Tompsett 2018-03-22 01:24:59 UTC
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...
Comment 6 Josef Moravec 2018-04-24 08:15:11 UTC
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
Comment 7 Josef Moravec 2018-04-24 08:15:14 UTC
Created attachment 74789 [details] [review]
Bug 20444: Update test

0) Apply this patch
1) prove t/db_dependent/Members/Attributes.t
should return green
Comment 8 Josef Moravec 2018-04-24 08:15:17 UTC
Created attachment 74790 [details] [review]
Bug 20444: Remove sub GetAttributes

0) Apply this patch
1) git grep GetAttributes should return no occurencies
Comment 9 Josef Moravec 2018-04-24 08:15:51 UTC
(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
Comment 10 Séverine Queune 2018-04-24 09:34:27 UTC
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>
Comment 11 Séverine Queune 2018-04-24 09:34:30 UTC
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>
Comment 12 Séverine Queune 2018-04-24 09:34:33 UTC
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>
Comment 13 Séverine Queune 2018-04-24 09:39:36 UTC
I tried with several notices (predue, suggestion accepted, article request completed) and it always worked.
Comment 14 Marcel de Rooy 2018-04-27 08:39:37 UTC
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>
Comment 15 Marcel de Rooy 2018-04-27 08:39:42 UTC
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>
Comment 16 Marcel de Rooy 2018-04-27 08:39:46 UTC
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>
Comment 17 Jonathan Druart 2018-05-03 16:41:18 UTC
Pushed to master for 18.05, thanks to everybody involved!