Bug 17767

Summary: Let Koha::Patron::Modification handle extended attributes
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart, katrin.fischer, kyle
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 13757, 21134    
Attachments: Bug 17767: Add borrower_modification.extended_attributes
Bug 17767: DBIC update
Bug 17767: Make Koha::Patron::Modification handle extended attributes
Bug 17767: Unit tests
Bug 17767: (followup) Rename test file
Bug 17767: Add borrower_modification.extended_attributes
Bug 17767: DBIC update
Bug 17767: Make Koha::Patron::Modification handle extended attributes
Bug 17767: Unit tests
Bug 17767: (followup) Rename test file
[SIGNED-OFF] Bug 17767: Add borrower_modification.extended_attributes
[SIGNED-OFF] Bug 17767: DBIC update
[SIGNED-OFF] Bug 17767: Make Koha::Patron::Modification handle extended attributes
[SIGNED-OFF] Bug 17767: Unit tests
[SIGNED-OFF] Bug 17767: (followup) Rename test file
Bug 17767: Add borrower_modification.extended_attributes
Bug 17767: DBIC update
Bug 17767: Make Koha::Patron::Modification handle extended attributes
Bug 17767: Unit tests
Bug 17767: (followup) Rename test file

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.