Bug 17767 - Let Koha::Patron::Modification handle extended attributes
Summary: Let Koha::Patron::Modification handle extended attributes
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 13757 21134
  Show dependency treegraph
 
Reported: 2016-12-13 13:32 UTC by Tomás Cohen Arazi
Modified: 2018-07-31 15:43 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 17767: Add borrower_modification.extended_attributes (1.95 KB, patch)
2016-12-16 14:36 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17767: DBIC update (1.58 KB, patch)
2016-12-16 14:36 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17767: Make Koha::Patron::Modification handle extended attributes (8.64 KB, patch)
2016-12-16 14:36 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17767: Unit tests (11.56 KB, patch)
2016-12-16 14:36 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17767: (followup) Rename test file (683 bytes, patch)
2016-12-16 14:36 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17767: Add borrower_modification.extended_attributes (1.94 KB, patch)
2016-12-21 16:02 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17767: DBIC update (1.58 KB, patch)
2016-12-21 16:02 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17767: Make Koha::Patron::Modification handle extended attributes (9.33 KB, patch)
2016-12-21 16:02 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17767: Unit tests (11.56 KB, patch)
2016-12-21 16:02 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17767: (followup) Rename test file (682 bytes, patch)
2016-12-21 16:03 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17767: Add borrower_modification.extended_attributes (2.00 KB, patch)
2016-12-21 19:59 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17767: DBIC update (1.64 KB, patch)
2016-12-21 19:59 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17767: Make Koha::Patron::Modification handle extended attributes (9.40 KB, patch)
2016-12-21 19:59 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17767: Unit tests (11.62 KB, patch)
2016-12-21 19:59 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17767: (followup) Rename test file (745 bytes, patch)
2016-12-21 19:59 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 17767: Add borrower_modification.extended_attributes (2.05 KB, patch)
2016-12-27 15:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17767: DBIC update (1.69 KB, patch)
2016-12-27 15:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17767: Make Koha::Patron::Modification handle extended attributes (9.44 KB, patch)
2016-12-27 15:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17767: Unit tests (11.67 KB, patch)
2016-12-27 15:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17767: (followup) Rename test file (795 bytes, patch)
2016-12-27 15:41 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-13 13:32:29 UTC
This is actually needed in order to implement bug 13757, because the underlying code doesn't handle extended attributes.
Comment 1 Tomás Cohen Arazi 2016-12-16 14:36:01 UTC
Created attachment 58256 [details] [review]
Bug 17767: Add borrower_modification.extended_attributes

This patch changes the DB structure adding borrower_modifications.extended_attributes
Comment 2 Tomás Cohen Arazi 2016-12-16 14:36:06 UTC
Created attachment 58257 [details] [review]
Bug 17767: DBIC update

This patch can be skipped by the RM, it just updates the DBIC schema files.
Comment 3 Tomás Cohen Arazi 2016-12-16 14:36:12 UTC
Created attachment 58258 [details] [review]
Bug 17767: Make Koha::Patron::Modification handle extended attributes

This patch makes Koha::Patron::Modification aware of the new extended_attributes
column, which is expected to contain valid JSON data.

The ->store method is modified so it validates the field value (i.e. the content
is decoded using the JSON library) and raises a convenient exception in case of
failure.

This behaviour change is covered by the provided unit tests.

To test:
- Apply the patchset
- Run:
  $ prove t/db_dependent/Koha/Patron/Modifications.t
=> SUCCESS: Tests make sense, and they pass
- Sign off :-D
Comment 4 Tomás Cohen Arazi 2016-12-16 14:36:22 UTC
Created attachment 58259 [details] [review]
Bug 17767: Unit tests

This patch introduces tests for the new behaviour, and also
enhances the existing ones aiming to reach full coverage.
Comment 5 Tomás Cohen Arazi 2016-12-16 14:36:27 UTC
Created attachment 58260 [details] [review]
Bug 17767: (followup) Rename test file
Comment 6 Tomás Cohen Arazi 2016-12-21 16:02:41 UTC
Created attachment 58358 [details] [review]
Bug 17767: Add borrower_modification.extended_attributes

This patch changes the DB structure adding borrower_modifications.extended_attributes
Comment 7 Tomás Cohen Arazi 2016-12-21 16:02:46 UTC
Created attachment 58359 [details] [review]
Bug 17767: DBIC update

This patch can be skipped by the RM, it just updates the DBIC schema files.
Comment 8 Tomás Cohen Arazi 2016-12-21 16:02:51 UTC
Created attachment 58360 [details] [review]
Bug 17767: Make Koha::Patron::Modification handle extended attributes

This patch makes Koha::Patron::Modification aware of the new extended_attributes
column, which is expected to contain valid JSON data.

The ->store method is modified so it validates the field value (i.e. the content
is decoded using the JSON library) and raises a convenient exception in case of
failure.

This behaviour change is covered by the provided unit tests.

To test:
- Apply the patchset
- Run:
  $ prove t/db_dependent/Koha/Patron/Modifications.t
=> SUCCESS: Tests make sense, and they pass
- Sign off :-D
Comment 9 Tomás Cohen Arazi 2016-12-21 16:02:56 UTC
Created attachment 58361 [details] [review]
Bug 17767: Unit tests

This patch introduces tests for the new behaviour, and also
enhances the existing ones aiming to reach full coverage.
Comment 10 Tomás Cohen Arazi 2016-12-21 16:03:01 UTC
Created attachment 58362 [details] [review]
Bug 17767: (followup) Rename test file
Comment 11 Owen Leonard 2016-12-21 19:59:11 UTC
Created attachment 58365 [details] [review]
[SIGNED-OFF] Bug 17767: Add borrower_modification.extended_attributes

This patch changes the DB structure adding borrower_modifications.extended_attributes

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 12 Owen Leonard 2016-12-21 19:59:18 UTC
Created attachment 58366 [details] [review]
[SIGNED-OFF] Bug 17767: DBIC update

This patch can be skipped by the RM, it just updates the DBIC schema files.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 13 Owen Leonard 2016-12-21 19:59:26 UTC
Created attachment 58367 [details] [review]
[SIGNED-OFF] Bug 17767: Make Koha::Patron::Modification handle extended attributes

This patch makes Koha::Patron::Modification aware of the new extended_attributes
column, which is expected to contain valid JSON data.

The ->store method is modified so it validates the field value (i.e. the content
is decoded using the JSON library) and raises a convenient exception in case of
failure.

This behaviour change is covered by the provided unit tests.

To test:
- Apply the patchset
- Run:
  $ prove t/db_dependent/Koha/Patron/Modifications.t
=> SUCCESS: Tests make sense, and they pass
- Sign off :-D

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 14 Owen Leonard 2016-12-21 19:59:32 UTC
Created attachment 58368 [details] [review]
[SIGNED-OFF] Bug 17767: Unit tests

This patch introduces tests for the new behaviour, and also
enhances the existing ones aiming to reach full coverage.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 15 Owen Leonard 2016-12-21 19:59:38 UTC
Created attachment 58369 [details] [review]
[SIGNED-OFF] Bug 17767: (followup) Rename test file

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 16 Owen Leonard 2016-12-21 20:00:26 UTC
Passes the test plan, in which it passes the test.
Comment 17 Jonathan Druart 2016-12-27 15:41:05 UTC
Created attachment 58437 [details] [review]
Bug 17767: Add borrower_modification.extended_attributes

This patch changes the DB structure adding borrower_modifications.extended_attributes

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 18 Jonathan Druart 2016-12-27 15:41:12 UTC
Created attachment 58438 [details] [review]
Bug 17767: DBIC update

This patch can be skipped by the RM, it just updates the DBIC schema files.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 19 Jonathan Druart 2016-12-27 15:41:16 UTC
Created attachment 58439 [details] [review]
Bug 17767: Make Koha::Patron::Modification handle extended attributes

This patch makes Koha::Patron::Modification aware of the new extended_attributes
column, which is expected to contain valid JSON data.

The ->store method is modified so it validates the field value (i.e. the content
is decoded using the JSON library) and raises a convenient exception in case of
failure.

This behaviour change is covered by the provided unit tests.

To test:
- Apply the patchset
- Run:
  $ prove t/db_dependent/Koha/Patron/Modifications.t
=> SUCCESS: Tests make sense, and they pass
- Sign off :-D

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 20 Jonathan Druart 2016-12-27 15:41:22 UTC
Created attachment 58440 [details] [review]
Bug 17767: Unit tests

This patch introduces tests for the new behaviour, and also
enhances the existing ones aiming to reach full coverage.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 21 Jonathan Druart 2016-12-27 15:41:26 UTC
Created attachment 58441 [details] [review]
Bug 17767: (followup) Rename test file

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 22 Jonathan Druart 2016-12-27 15:43:36 UTC
It would have been good to use the column_exists subroutine added by 17234 (but not pushed yet...)
Comment 23 Kyle M Hall 2016-12-28 14:00:54 UTC
Pushed to master for 17.05, thanks Tomas!
Comment 24 Katrin Fischer 2017-01-04 21:52:40 UTC
This won't get ported back to 16.11.x as it is an enhancement.