Bug 17792 - Introduce Koha::Patron::Attribute(s)
Summary: Introduce Koha::Patron::Attribute(s)
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: Tomás Cohen Arazi
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on: 17755 17813
Blocks: 13757 17826
  Show dependency treegraph
 
Reported: 2016-12-19 21:50 UTC by Tomás Cohen Arazi
Modified: 2017-12-07 22:17 UTC (History)
4 users (show)

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


Attachments
Bug 17792: Introduce Koha::Patron::Attribute(s) (3.00 KB, patch)
2016-12-21 15:56 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17792: Introduce Koha::Patron::Attribute(s) (3.05 KB, patch)
2016-12-23 13:29 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17792: Add opac_editable and opac_display methods (5.15 KB, patch)
2016-12-23 15:09 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17792: Introduce Koha::Patron::Attribute(s) (3.09 KB, patch)
2017-01-31 17:59 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17792: Add opac_editable and opac_display methods (5.19 KB, patch)
2017-01-31 17:59 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17792: Introduce Koha::Patron::Attribute(s) (3.17 KB, patch)
2017-03-23 15:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17792: Add opac_editable and opac_display methods (5.27 KB, patch)
2017-03-23 15:55 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2016-12-19 21:50:23 UTC

    
Comment 1 Tomás Cohen Arazi 2016-12-21 15:56:06 UTC
Created attachment 58350 [details] [review]
Bug 17792: Introduce Koha::Patron::Attribute(s)

This patch introduces stub Koha::Object(s) for handling patron attributes.
Comment 2 Mark Tompsett 2016-12-23 06:15:01 UTC
Comment on attachment 58350 [details] [review]
Bug 17792: Introduce Koha::Patron::Attribute(s)

Review of attachment 58350 [details] [review]:
-----------------------------------------------------------------

::: Koha/Patron/Attribute.pm
@@ +20,5 @@
> +use base qw(Koha::Object);
> +
> +=head1 NAME
> +
> +Koha::Patron;;Category - Koha Patron;;Category Object class

Why is this ;;Category?
Comment 3 Tomás Cohen Arazi 2016-12-23 13:29:54 UTC
Created attachment 58414 [details] [review]
Bug 17792: Introduce Koha::Patron::Attribute(s)

This patch introduces stub Koha::Object(s) for handling patron attributes.

Edit: amended the POD to fix C&p mistake
Comment 4 Tomás Cohen Arazi 2016-12-23 13:30:19 UTC
(In reply to M. Tompsett from comment #2)
> Comment on attachment 58350 [details] [review] [review]
> Bug 17792: Introduce Koha::Patron::Attribute(s)
> 
> Review of attachment 58350 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: Koha/Patron/Attribute.pm
> @@ +20,5 @@
> > +use base qw(Koha::Object);
> > +
> > +=head1 NAME
> > +
> > +Koha::Patron;;Category - Koha Patron;;Category Object class
> 
> Why is this ;;Category?

It was a typo.
Comment 5 Tomás Cohen Arazi 2016-12-23 15:09:00 UTC
Created attachment 58421 [details] [review]
Bug 17792: Add opac_editable and opac_display methods

This patch adds two methods to the Koha::Patron::Attribute:

- opac_display
- opac_editable

Both method just check the corresponding Koha::Patron::Attribute::Type
and return the values for those attributes. This is useful to avoid
checking that manually on the controller scripts.

To test:
- Run:
  $ prove t/db_dependent/Koha/Patron/Attributes.t
=> SUCCESS: Tests pass!
- Sign off :-D
Comment 6 Chris Kirby 2016-12-25 01:02:48 UTC
I applied the required patches (17755, 177813), then this patch. Tests fail, "No tests run for subtest 'opac_editable() tests'".
Comment 7 Chris Kirby 2016-12-25 01:14:11 UTC
^ I applied the required patches (17755, 17813), then patch 17792. I then tested, and the tests failed:"No tests run for subtest 'opac_editable() tests'".
Comment 8 Tomás Cohen Arazi 2016-12-25 23:39:29 UTC
(In reply to Chris Kirby from comment #7)
> ^ I applied the required patches (17755, 17813), then patch 17792. I then
> tested, and the tests failed:"No tests run for subtest 'opac_editable()
> tests'".

Have you ran the updatedatabase.pl script?
Comment 9 Chris Kirby 2016-12-26 14:22:18 UTC
(In reply to Tomás Cohen Arazi from comment #8)
> (In reply to Chris Kirby from comment #7)
> > ^ I applied the required patches (17755, 17813), then patch 17792. I then
> > tested, and the tests failed:"No tests run for subtest 'opac_editable()
> > tests'".
> 
> Have you ran the updatedatabase.pl script?

I have now. After running "perl installer/data/mysql/updatedatabase.pl" in the koha-shell environment, I had the following output:

DEV atomic update: bug_17813_borrower_attributes_pk.sql
C4:::Installer::load_sql returned the following errors while attempting to load
/home/ckirby/koha/installer/data/mysql/atomicupdate/bug_17813_borrower_attributes_pk.sql:"

I had the same error when I ran "koha-upgrade-schema" on my instance.
Comment 10 Tomás Cohen Arazi 2016-12-26 15:40:58 UTC
(In reply to Chris Kirby from comment #6)
> I applied the required patches (17755, 177813), then this patch. Tests fail,
> "No tests run for subtest 'opac_editable() tests'".

Chris, please find me on IRC or email to check what's going on. Things work fine for me.
Comment 11 Tomás Cohen Arazi 2016-12-30 13:30:28 UTC
The unit tests and functionality introduced by the second patch can only be tested in conjunction with bug 13757, which adds the opac_editable attribute. Running the unit tests without 13757 will make the opac_editable() tests fail.
Comment 12 Chris Kirby 2016-12-30 15:13:55 UTC
(In reply to Tomás Cohen Arazi from comment #11)
> The unit tests and functionality introduced by the second patch can only be
> tested in conjunction with bug 13757, which adds the opac_editable
> attribute. Running the unit tests without 13757 will make the
> opac_editable() tests fail.

I applied the patch for 1357 on top of 17792. And then ran the updatedatabase.pl 
script, which errors out as follows:

DEV atomic update: bug_17813_borrower_attributes_pk.sql
C4:::Installer::load_sql returned the following errors while attempting to load
/home/ckirby/koha/installer/data/mysql/atomicupdate/bug_17813_borrower_attributes_pk.sql:"

Is it necessary to update DBIx schema files? I could not successfully run the misc/devel/update_dbix_class_files.pl script.
Comment 13 Tomás Cohen Arazi 2017-01-09 15:02:23 UTC
Note for testers: this patches need to be tested with 13757 applied on top of them. Otherwise some tests will be skipped.
Comment 14 Chris Kirby 2017-01-09 15:59:00 UTC
(In reply to Tomás Cohen Arazi from comment #13)
> Note for testers: this patches need to be tested with 13757 applied on top
> of them. Otherwise some tests will be skipped.

applied the following patches:17755, 17813 (followed by updatedatabase.pl), 
17792, (followed by updatedatabase.pl), 13757.

13757 doesn't apply cleanly over 17792: 
 https://snag.gy/q4co8z.jpg
Comment 15 Tomás Cohen Arazi 2017-01-11 12:51:16 UTC
(In reply to Chris Kirby from comment #14)
> (In reply to Tomás Cohen Arazi from comment #13)
> > Note for testers: this patches need to be tested with 13757 applied on top
> > of them. Otherwise some tests will be skipped.
> 
> applied the following patches:17755, 17813 (followed by updatedatabase.pl), 
> 17792, (followed by updatedatabase.pl), 13757.
> 
> 13757 doesn't apply cleanly over 17792: 
>  https://snag.gy/q4co8z.jpg

Chris, I fixed that already.
Comment 16 Nick Clemens 2017-01-31 17:59:28 UTC
Created attachment 59692 [details] [review]
Bug 17792: Introduce Koha::Patron::Attribute(s)

This patch introduces stub Koha::Object(s) for handling patron attributes.

Edit: amended the POD to fix C&p mistake

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 17 Nick Clemens 2017-01-31 17:59:35 UTC
Created attachment 59693 [details] [review]
Bug 17792: Add opac_editable and opac_display methods

This patch adds two methods to the Koha::Patron::Attribute:

- opac_display
- opac_editable

Both method just check the corresponding Koha::Patron::Attribute::Type
and return the values for those attributes. This is useful to avoid
checking that manually on the controller scripts.

To test:
- Run:
  $ prove t/db_dependent/Koha/Patron/Attributes.t
=> SUCCESS: Tests pass!
- Sign off :-D

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 18 Jonathan Druart 2017-03-15 12:05:48 UTC
Waiting for bug 13757 to QA it.
Comment 19 Jonathan Druart 2017-03-23 15:55:13 UTC
Created attachment 61553 [details] [review]
Bug 17792: Introduce Koha::Patron::Attribute(s)

This patch introduces stub Koha::Object(s) for handling patron attributes.

Edit: amended the POD to fix C&p mistake

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 20 Jonathan Druart 2017-03-23 15:55:17 UTC
Created attachment 61554 [details] [review]
Bug 17792: Add opac_editable and opac_display methods

This patch adds two methods to the Koha::Patron::Attribute:

- opac_display
- opac_editable

Both method just check the corresponding Koha::Patron::Attribute::Type
and return the values for those attributes. This is useful to avoid
checking that manually on the controller scripts.

To test:
- Run:
  $ prove t/db_dependent/Koha/Patron/Attributes.t
=> SUCCESS: Tests pass!
- Sign off :-D

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 21 Jonathan Druart 2017-03-23 16:02:16 UTC
QAed along withbug 17755 and bug 13757.
Please push them altogether.
Comment 22 Kyle M Hall 2017-03-24 18:44:13 UTC
Pushed to master for 17.05, thanks Tomas!
Comment 23 Katrin Fischer 2017-03-26 21:12:24 UTC
This won't get ported back to 16.11.x as it is an enhancement.