[Bug 17828] New: Koha::Patron::Attribute-> store should raise an exception if unique_id is being broken
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 Bug ID: 17828 Summary: Koha::Patron::Attribute->store should raise an exception if unique_id is being broken Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org The new Koha::Patron::Attribute should check the unique_id property and raise an exception if storing the new attribute would break it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |17792 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17792 [Bug 17792] Introduce Koha::Patron::Attribute(s) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 58526 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58526&action=edit Attachment to Bug 17828 - Koha::Patron::Attribute->store should raise an exception if unique_id is being broken Bug 17828: Unit tests for Koha::Patron::Attribute->store Unit tests for an overloaded Koha::Patron::Attribute->store that checks attribute type's uniqueness property and raises an exception conveniently. Test plan on the implementing patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 58527 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58527&action=edit Bug 17828: Overload Koha::Patron::Attribute->store to check for unique_id This patch introduces a Koha::Patron::Attribute->store method that does the required check for the corresponding Koha::Patron::Attribute::Type and raises an exception if trying to store more than one unique attribute. This patch introduces an exception definition for that purpose. To test: - Apply the unit tests patches - Run: $ prove t/db_dependent/Koha/Patron/Attributes.t => FAIL: Tests fail - Apply this patch - Run: $ prove t/db_dependent/Koha/Patron/Attributes.t => SUCCESS: Tests pass! - 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=17828 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=17828 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |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=17828 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58526|Attachment to Bug 17828 - |Bug 17828: Unit tests for description|Koha::Patron::Attribute->st |Koha::Patron::Attribute->st |ore should raise an |ore |exception if unique_id is | |being broken | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- This patch was wrong, re-doing it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |jonathan.druart@bugs.koha-c |y.org |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58526|0 |1 is obsolete| | --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 58557 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58557&action=edit Bug 17828: Unit tests for Koha::Patron::Attribute->store Unit tests for an overloaded Koha::Patron::Attribute->store that checks attribute type's uniqueness property and raises an exception conveniently. It also tests for repeatable attribute type's property. Test plan on the implementing patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58527|0 |1 is obsolete| | --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 58558 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58558&action=edit Bug 17828: Overload Koha::Patron::Attribute->store to check for unique_id and repeatable This patch introduces a Koha::Patron::Attribute->store method that does the required check for the corresponding Koha::Patron::Attribute::Type and raises an exception if trying to store more than one unique attribute code/value combination, and another exception for non-repeatable attribute types being stored while already existing on the DB. This patch introduces exception definitions for that purpose. To test: - Apply the unit tests patches - Run: $ prove t/db_dependent/Koha/Patron/Attributes.t => FAIL: Tests fail - Apply this patch - Run: $ prove t/db_dependent/Koha/Patron/Attributes.t => SUCCESS: Tests pass! - 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=17828 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 Josef Moravec <josef.moravec@gmail.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=17828 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58557|0 |1 is obsolete| | --- Comment #6 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 61505 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61505&action=edit [SIGNED-OFF] Bug 17828: Unit tests for Koha::Patron::Attribute->store Unit tests for an overloaded Koha::Patron::Attribute->store that checks attribute type's uniqueness property and raises an exception conveniently. It also tests for repeatable attribute type's property. Test plan on the implementing patch. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58558|0 |1 is obsolete| | --- Comment #7 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 61506 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61506&action=edit [SIGNED-OFF] Bug 17828: Overload Koha::Patron::Attribute->store to check for unique_id and repeatable This patch introduces a Koha::Patron::Attribute->store method that does the required check for the corresponding Koha::Patron::Attribute::Type and raises an exception if trying to store more than one unique attribute code/value combination, and another exception for non-repeatable attribute types being stored while already existing on the DB. This patch introduces exception definitions for that purpose. To test: - Apply the unit tests patches - Run: $ prove t/db_dependent/Koha/Patron/Attributes.t => FAIL: Tests fail - Apply this patch - Run: $ prove t/db_dependent/Koha/Patron/Attributes.t => SUCCESS: Tests pass! - Sign off :-D Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 61547 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61547&action=edit Bug 17828: (followup) Add Test::Exception as dependency Test::Exception is available on Jessie and already installed when koha-common gets installed 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=17828 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|17792 |13757 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13757 [Bug 13757] Make patron attributes editable in the opac if set to 'editable in OPAC' https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17792 [Bug 17792] Introduce Koha::Patron::Attribute(s) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- The 2 Koha::Database->new->schema->resultset( $self->_type )->search call sounds weird to me. It looks like you need to create a new method to return the type of the attribute (so return a the Koha::Patron::Attribute::Type related to $self->code). Don't you think? That will simplify the code and make it more readable. Switch back to SO if you disagree or if I am missing something. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 61644 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61644&action=edit Bug 17828: (QA followup) Add ->type and reuse it 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=17828 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |18339 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18339 [Bug 18339] Koha::Patron::Attribute ->opac_editable and ->opac_display should be removed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 61662 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61662&action=edit Bug 17828: Use Koha::Patron::Attributes instead of _resultset -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Not sure about _check_repeatable and _check_unique_id, I will think a bit more about it. Note that one returns $self not the other one -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #12)
Not sure about _check_repeatable and _check_unique_id, I will think a bit more about it. Note that one returns $self not the other one
Boo, I forgot that line! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #61505|0 |1 is obsolete| | Attachment #61506|0 |1 is obsolete| | Attachment #61547|0 |1 is obsolete| | Attachment #61644|0 |1 is obsolete| | Attachment #61662|0 |1 is obsolete| | --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 61703 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61703&action=edit Bug 17828: Unit tests for Koha::Patron::Attribute->store Unit tests for an overloaded Koha::Patron::Attribute->store that checks attribute type's uniqueness property and raises an exception conveniently. It also tests for repeatable attribute type's property. Test plan on the implementing patch. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #61703|0 |1 is obsolete| | --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 61704 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61704&action=edit Bug 17828: Unit tests for Koha::Patron::Attribute->store Unit tests for an overloaded Koha::Patron::Attribute->store that checks attribute type's uniqueness property and raises an exception conveniently. It also tests for repeatable attribute type's property. Test plan on the implementing patch. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 --- Comment #16 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 61705 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61705&action=edit Bug 17828: Overload Koha::Patron::Attribute->store to check for unique_id and repeatable This patch introduces a Koha::Patron::Attribute->store method that does the required check for the corresponding Koha::Patron::Attribute::Type and raises an exception if trying to store more than one unique attribute code/value combination, and another exception for non-repeatable attribute types being stored while already existing on the DB. This patch introduces exception definitions for that purpose. To test: - Apply the unit tests patches - Run: $ prove t/db_dependent/Koha/Patron/Attributes.t => FAIL: Tests fail - Apply this patch - Run: $ prove t/db_dependent/Koha/Patron/Attributes.t => SUCCESS: Tests pass! - Sign off :-D Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 --- Comment #17 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 61706 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61706&action=edit Bug 17828: (followup) Add Test::Exception as dependency Test::Exception is available on Jessie and already installed when koha-common gets installed Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 --- Comment #18 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 61707 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61707&action=edit Bug 17828: (QA followup) Add ->type and reuse it Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 --- Comment #19 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 61708 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61708&action=edit Bug 17828: Use Koha::Patron::Attributes instead of _resultset Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 --- Comment #20 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 61709 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61709&action=edit Bug 17828: return $self to be consistent Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Status|Passed QA |Pushed to Master --- Comment #21 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 17.05, thanks Tomas, Jonathan! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 Bug 17828 depends on bug 13757, which changed state. Bug 13757 Summary: Make patron attributes editable in the opac if set to 'editable in OPAC' https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13757 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED CC| |katrin.fischer@bsz-bw.de --- Comment #22 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Dependency is not in 16.11.x, so this should not be needed there. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17828 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |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=17828 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |28031 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28031 [Bug 28031] Koha::Patron::Attribute->_check_repeatable doesn't exclude the object's ID -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org