[Bug 27857] New: Koha::Patron->extended_attributes skips checks
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Bug ID: 27857 Summary: Koha::Patron->extended_attributes skips checks Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org Patron attributes can have some requirements: - Be unique across the DB - (Not) bd repeatable This is correctly handled by both - Koha::Patron::Attribute->store - Koha::Patron->add_extended_attribute But then, there's Koha::Patron->extended_attributes, that can overwrite them all at once, but doesn't rely on those methods, and uses DBIC relations instead, so no checks on attribute constraints are made. We need to fix it. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |27853 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27853 [Bug 27853] POST /patrons doesn't enforce mandatory extended attributes -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |27833 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27833 [Bug 27833] Koha::Exceptions::Patron::Attribute::* should have parameters -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |27858 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27858 [Bug 27858] Make Koha::Patron::Attribute->store raise an exception on invalid type/code -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 117709 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117709&action=edit Bug 27857: Move extended_attributes tests to Koha/Patron.t They really belong here. both Patron.t and Patrons.t got lengthy and it is not intuitive to look for tests for a Koha::Patron method in the Patrons.t file. To test: 1. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t \ t/db_dependent/Koha/Patrons.t => SUCCESS: Tests pass! 2. Apply this patch 3. Repeat 1 => SUCCESS: Tests still pass, no new stuff 4. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 117710 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117710&action=edit Bug 27857: Regression tests This patch adds regression tests for the changed behavior. Koha::Patron->extended_attributes will now throw exceptions and rollback on invalid attributes. The existing tests are amended so they don't try to deal with skipping bad attributes. Specific subtests are added for error situations. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t => FAIL: Tests fail because the new behavior is not implemented. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 117711 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117711&action=edit Bug 27857: Make Koha::Patron->extended_attributes validate data This patch makes the extended_attributes method use Koha::Patron->add_extended_attribute inside the loop, instead of direct DB access. This way, it takes advantage of the validation that is done there. All happens inside a transaction, so if 'add_extended_attribute' throws an exception, everything is rolled back. To test: 1. Apply the previous patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t => FAIL: The behavior is not the correct one, tests fail 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|27833 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27833 [Bug 27833] Koha::Exceptions::Patron::Attribute::* should have parameters -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Whilst I'm reviewing this.. * We need to add handling for mandatory attributes - Working on this * Why do we filter the delete by the logged in users branch (not the patrons branch.. or no filtering at all?) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- # Get a list of required attributes my $required_attribute_types = Koha::Patron::Attribute::Types->search_with_library_limits( { mandatory => 1 }, {}, $self->library->branchcode )->get_column('type'); We got stuck at deciding what mandatory actually means.. right now we delete all attributes for the patron at the library the staff member is logged in at (the patron may well have a different home library). Should we be checking against their home library for mandatory attributes.. or the staff users library? The above query gets the patrons home libraries mandatory attribute list.. but until we know the answer to this intermediary question it's hard to know how/when/what error to throw. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com | |, katrin.fischer@bsz-bw.de, | |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- The more I think about this, the more I think perhaps that 'mandatory' is a purely UI concept in this case. I can see all sorts of reasons you may have added mandatory attributes.. but actually enforcing them would likely need to happen at whatever point the attribute should be getting looked at rather than at the point at which it's entered. Examples * An attribute that denotes permission to access certain resources at a certain branch. - This attribute is 'mandatory' as such at the point at which the patron wants to gain access to the resource.. they're either given permission or not. This is at a branch level and is mandatory in so much as it's required prior to being able to access the resource. We don't tie any logic to that mandatory for that action so in reality, it's only at the data entry via the existing UI that will take any notice of the mandatory flag.. I think the API can and probably should safely ignore the mandatory cases when they're not set as globally mandatory. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117938 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117938&action=edit Bug 27857: Add handling for globally mandatory attributes This patch adds handling for globally mandatory extended attributes and the corresponding unit tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117709|0 |1 is obsolete| | --- Comment #8 from David Nind <david@davidnind.com> --- Created attachment 117945 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117945&action=edit Bug 27857: Move extended_attributes tests to Koha/Patron.t They really belong here. both Patron.t and Patrons.t got lengthy and it is not intuitive to look for tests for a Koha::Patron method in the Patrons.t file. To test: 1. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t \ t/db_dependent/Koha/Patrons.t => SUCCESS: Tests pass! 2. Apply this patch 3. Repeat 1 => SUCCESS: Tests still pass, no new stuff 4. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117710|0 |1 is obsolete| | --- Comment #9 from David Nind <david@davidnind.com> --- Created attachment 117946 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117946&action=edit Bug 27857: Regression tests This patch adds regression tests for the changed behavior. Koha::Patron->extended_attributes will now throw exceptions and rollback on invalid attributes. The existing tests are amended so they don't try to deal with skipping bad attributes. Specific subtests are added for error situations. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t => FAIL: Tests fail because the new behavior is not implemented. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117711|0 |1 is obsolete| | --- Comment #10 from David Nind <david@davidnind.com> --- Created attachment 117947 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117947&action=edit Bug 27857: Make Koha::Patron->extended_attributes validate data This patch makes the extended_attributes method use Koha::Patron->add_extended_attribute inside the loop, instead of direct DB access. This way, it takes advantage of the validation that is done there. All happens inside a transaction, so if 'add_extended_attribute' throws an exception, everything is rolled back. To test: 1. Apply the previous patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t => FAIL: The behavior is not the correct one, tests fail 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117938|0 |1 is obsolete| | --- Comment #11 from David Nind <david@davidnind.com> --- Created attachment 117948 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117948&action=edit Bug 27857: Add handling for globally mandatory attributes This patch adds handling for globally mandatory extended attributes and the corresponding unit tests. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117948|0 |1 is obsolete| | --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117953 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117953&action=edit Bug 27857: Add handling for globally mandatory attributes This patch adds handling for globally mandatory extended attributes and the corresponding unit tests. To test: 1. Apply the patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117945|0 |1 is obsolete| | --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117954 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117954&action=edit Bug 27857: Move extended_attributes tests to Koha/Patron.t They really belong here. both Patron.t and Patrons.t got lengthy and it is not intuitive to look for tests for a Koha::Patron method in the Patrons.t file. To test: 1. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t \ t/db_dependent/Koha/Patrons.t => SUCCESS: Tests pass! 2. Apply this patch 3. Repeat 1 => SUCCESS: Tests still pass, no new stuff 4. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117946|0 |1 is obsolete| | --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117955 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117955&action=edit Bug 27857: Regression tests This patch adds regression tests for the changed behavior. Koha::Patron->extended_attributes will now throw exceptions and rollback on invalid attributes. The existing tests are amended so they don't try to deal with skipping bad attributes. Specific subtests are added for error situations. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t => FAIL: Tests fail because the new behavior is not implemented. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117947|0 |1 is obsolete| | --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117956 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117956&action=edit Bug 27857: Make Koha::Patron->extended_attributes validate data This patch makes the extended_attributes method use Koha::Patron->add_extended_attribute inside the loop, instead of direct DB access. This way, it takes advantage of the validation that is done there. All happens inside a transaction, so if 'add_extended_attribute' throws an exception, everything is rolled back. To test: 1. Apply the previous patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t => FAIL: The behavior is not the correct one, tests fail 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117953|0 |1 is obsolete| | --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117957 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117957&action=edit Bug 27857: Add handling for globally mandatory attributes This patch adds handling for globally mandatory extended attributes and the corresponding unit tests. To test: 1. Apply the patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I'm happy to PQA on Tomas's patches but would like him to verify my final followup. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #18 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Martin Renvoize from comment #17)
I'm happy to PQA on Tomas's patches but would like him to verify my final followup.
I like the follow-up. Go ahead! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Excellent :).. PQA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #20 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Aren't we going to adjust the controllers? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #21 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Jonathan Druart from comment #20)
Aren't we going to adjust the controllers?
I had a brief check of these at the time.. I believe currently the case is that the controller already check these things prior to passing use the data and so don't need changes to try/catch the exceptions... as such I felt it made sense to not touch them at this time and rather do that as part of migrating UI to using the API's. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #22 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- It does not convince me. members/memberentry.pl 592 $patron->extended_attributes($extended_patron_attributes); may raise an uncaught exceptions. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #23 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #22)
It does not convince me.
members/memberentry.pl 592 $patron->extended_attributes($extended_patron_attributes);
may raise an uncaught exceptions.
I'll take a look now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #24 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Tomás Cohen Arazi from comment #18)
(In reply to Martin Renvoize from comment #17)
I'm happy to PQA on Tomas's patches but would like him to verify my final followup.
I like the follow-up. Go ahead!
Should we rename the id => patron_extended_attribute_id ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #25 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #22)
It does not convince me.
members/memberentry.pl 592 $patron->extended_attributes($extended_patron_attributes);
may raise an uncaught exceptions.
Do you think we should wrap everything inside a transaction? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #26 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I don't know exactly what needs to be done, but there are check above, see the block with "ERROR_extended_unique_id_failed_code". Also the repeatable is not checked in the controller, only from the template. Now that we have it we should deal with it. Basically it's: - insert - catch "must unique" => propagate and display nice to the template - catch other failures => "something went wrong" is enough (they hack the form) And remove the existing block. We should continue on a dedicated bug report :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |27956 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27956 [Bug 27956] Catch extended attributes exceptions in controllers -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |23666 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23666 [Bug 23666] Add API route for additional patron attributes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |28009 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28009 [Bug 28009] Provide better feedback on import -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |28056 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28056 [Bug 28056] Add Koha::Exceptions::Patron::MissingMandatoryExtendedAttribute -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #27 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 119009 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=119009&action=edit Bug 27857: (use-it-in-real-life follow-up) Use a more fine-grained exception for mandatory attributes Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #28 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 119038 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=119038&action=edit Bug 27857: Get the right column This patch makes the search for mandatory attribute types actually use 'code' instead of 'class' for comparisson. Tests are added to cover this behavior. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> https://bugs.koha-community.org/show_bug.cgi?id=27587 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #119009|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #29 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 119040 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=119040&action=edit Bug 27857: (follow-up) Use a more fine-grained exception for mandatory attributes On using this dev for the API, I found that the original FKConstraint exception was too generic, and could be confused with those risen by Koha::Patron->store. So this patch changes the exception that is thrown in Koha::Patron->extended_attributes The change is reflected on the test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t => SUCCESS: Tests pass, and they include the new exception 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |21.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #30 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 21.05, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #31 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 119443 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=119443&action=edit Bug 27857: Fix Pseudonymization.t t/db_dependent/Koha/Pseudonymization.t .. 2/3 # Failed test 'Attribute 1 should be retrieved correctly' # at t/db_dependent/Koha/Pseudonymization.t line 198. # Structures begin differing at: # $got->{borrowernumber} = Does not exist # $expected->{borrowernumber} = '58' # Failed test 'Attribute 2 should be retrieved correctly' # at t/db_dependent/Koha/Pseudonymization.t line 204. # Structures begin differing at: # $got->{borrowernumber} = Does not exist # $expected->{borrowernumber} = '58' # Looks like you failed 2 tests of 3. t/db_dependent/Koha/Pseudonymization.t .. 3/3 Highlighted by commit 7b40ee22035210e3fbdfe75785d68ca314285409 Bug 27857: Make Koha::Patron->extended_attributes validate data Koha::Patron->add_extended_attributes should not modify the $attribute hashref parameter we passed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #32 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Last patch pushed to master. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #33 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 119937 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=119937&action=edit Bug 27857: Start a transaction foreach subtest There is a random failure # Looks like you planned 15 tests but ran 2. # Failed test 'extended_attributes' # at t/db_dependent/Koha/Patron.t line 664. Missing mandatory extended attribute (type=B0vCQxlI)# Looks like your test exited with 255 just after 7. [17:00:24] t/db_dependent/Koha/Patron.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #34 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 119938 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=119938&action=edit Bug 27857: Remove unecessary âttribute_type3 var It could be created with mandatory=1 and will make tests fail -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #35 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Last two patches pushed to master. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #36 from Fridolin Somers <fridolin.somers@biblibre.com> --- Depends on Bug 27858 and Bug 28056 both not in 20.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Eric Phetteplace <ephetteplace@cca.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |ephetteplace@cca.edu -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #37 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Hum, failure on Jenkins Master_D9 run 1663 11:41:11 koha_1 | [0m[92mKoha::Exceptions::Patron::MissingMandatoryExtendedAttribute[0m { 11:41:11 koha_1 | Parents [92mKoha::Exceptions::Patron[0m 11:41:11 koha_1 | public methods (3) : [92mdescription[0m, [92mFields[0m, [92mtype[0m 11:41:11 koha_1 | private methods (0) 11:41:11 koha_1 | internals: { 11:41:11 koha_1 | [35megid[0m "[93m1000 1000[0m", 11:46:27 koha_1 | [35meuid[0m [94m1001[0m, 11:46:27 koha_1 | [35mgid[0m "[93m1000 1000[0m", 11:46:27 koha_1 | [35mmessage[0m "[93m[0m", 11:46:27 koha_1 | [35mpid[0m [94m4522[0m, 11:46:27 koha_1 | [35mtime[0m [94m1621935671[0m, 11:46:27 koha_1 | [35mtrace[0m [92mDevel::StackTrace[0m, 11:46:27 koha_1 | [35mtype[0m "[93mVfu0SZ[0m", 11:46:27 koha_1 | [35muid[0m [94m1001[0m 11:46:27 koha_1 | } 11:46:27 koha_1 | } 11:46:27 koha_1 | Missing mandatory extended attribute (type=Vfu0SZ) at /kohadevbox/koha/Koha/Patrons/Import.pm line 420, <$fh> line 2. 11:46:27 koha_1 | 11:46:27 koha_1 | # Failed test at t/db_dependent/Koha/Patrons/Import.t line 902. 11:46:27 koha_1 | # got: '0' 11:46:27 koha_1 | # expected: '1' 11:46:27 koha_1 | # Looks like you planned 17 tests but ran 1. 11:46:27 koha_1 | # Looks like you failed 1 test of 1 run. 11:46:27 koha_1 | 11:46:27 koha_1 | # Failed test 'patron_attributes' 11:46:27 koha_1 | # at t/db_dependent/Koha/Patrons/Import.t line 1114. 11:46:27 koha_1 | Can't call method "extended_attributes" on an undefined value at t/db_dependent/Koha/Patrons/Import.t line 905, <$fh> line 2. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #38 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Got it locally t/db_dependent/Koha/Patrons.t ........................................... 28/42 # No tests run! # Failed test 'No tests run for subtest "extended attributes"' # at t/db_dependent/Koha/Patrons.t line 1770. # Looks like you failed 1 test of 110. t/db_dependent/Koha/Patrons.t ........................................... 33/42 # Failed test 'Test Koha::Patrons::merge' # at t/db_dependent/Koha/Patrons.t line 1774. Missing mandatory extended attribute (type=hGA0QWrb6)# Looks like your test exited with 2 just after 33. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 --- Comment #39 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Master_D9_My8 586 Docker_7 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |28516 --- Comment #40 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Random failure reported on bug 28516 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28516 [Bug 28516] Koha/Patrons/Import.t is failing randomly -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |31005 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31005 [Bug 31005] Cannot edit patrons in other categories if an extended attribute is mandatory and limited to a category -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |31032 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31032 [Bug 31032] Patron attributes branch limitations are unclear -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org