[Bug 20443] New: Move C4::Members::Attributes to Koha namespace
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Bug ID: 20443 Summary: Move C4::Members::Attributes to Koha namespace Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: josef.moravec@gmail.com QA Contact: testopia@bugs.koha-community.org -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |12159 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12159 [Bug 12159] Duplicate borrower_add_additional_fields function -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |15449 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15449 [Bug 15449] Move stuffs to the Koha namespace -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |20444 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20444 [Bug 20444] Remove C4::Members::Attributes::GetAttributes -- 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=20443 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|20444 | Depends on| |20444 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20444 [Bug 20444] Remove C4::Members::Attributes::GetAttributes -- 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=20443 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |jonathan.druart@bugs.koha-c |ity.org |ommunity.org Status|NEW |ASSIGNED CC| |jonathan.druart@bugs.koha-c | |ommunity.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=20443 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |josef.moravec@gmail.com, | |kyle@bywatersolutions.com, | |m.de.rooy@rijksmuseum.nl, | |martin.renvoize@ptfs-europe | |.com, | |nick@bywatersolutions.com, | |tomascohen@gmail.com Status|ASSIGNED |In Discussion --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Here is a first draft of that work: https://gitlab.com/joubu/Koha/commits/bug_20443 It's quite a funky move: 41 files changed, 713 insertions(+), 2017 deletions(-) However some tests are missing for trivial methods added to Koha::Patrons. Any remarks / suggestions? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Bug 20443 depends on bug 20444, which changed state. Bug 20444 Summary: Remove C4::Members::Attributes::GetAttributes https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20444 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=20443 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |21083 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21083 [Bug 21083] Batch patron modification does not allow to modify repeatable patron attributes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |8326 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8326 [Bug 8326] Allow patron attributes to be made repeatable after initial creation -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- *** Bug 20445 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- First QA comment.. and it's possibly a little mean.. You've introduced three 'filter_by_' methods: Awesome++. However, there's an inconsistency with their chainability.. I personally feel such a method should always allow chaining and as such would request, although currently, you don't use them within a chain, that `filter_by_attribute_type` and `filter_by_attribute_value` are altered such that they act upon self rather than instantiating a new object to pass as the return. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I don't see the above as a blocker however and I'm happy to submit it as a qa followup myself once there are patches here ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Remote branch has been rebased against master. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I don't think we can break GetPatronInfo (ILS-DI) like this. We should keep the current behaviour and propose a behaviour change on a separate bug. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Failed QA --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Tomás Cohen Arazi from comment #6)
I don't think we can break GetPatronInfo (ILS-DI) like this. We should keep the current behaviour and propose a behaviour change on a separate bug.
This: my $attrs = $patron->extended_attributes->search({ opac_display => 1 })->unblessed; should be: my $attrs = grep { $_->type->opac_display } @{ $patron->extended_attributes->search }; my @attrs_unblessed = map { $_->unblessed } @{ $attrs }; Highlighted by: k$ prove t/db_dependent/ILSDI_Services.t t/db_dependent/ILSDI_Services.t .. 1/4 DBD::mysql::st execute failed: Unknown column 'opac_display' in 'where clause'... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=21199 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Tomás Cohen Arazi from comment #6)
I don't think we can break GetPatronInfo (ILS-DI) like this. We should keep the current behaviour and propose a behaviour change on a separate bug.
I have pushed a follow-up to fix that. NOTES: 1. I do not think we should display info from the attribute's type 2. We should test the value of the description if linked with AV (value_description). Are we sure we want "attribute" vs "value". And "value_description"? Let's stick to current behaviours for now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=5161 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 --- Comment #10 from Josef Moravec <josef.moravec@gmail.com> --- I just rebased this on top of current code in bug 12159, see https://gitlab.com/josef.moravec/Koha/tree/bug_20443 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 --- Comment #11 from Josef Moravec <josef.moravec@gmail.com> --- Tests not passing (at least): t/db_dependent/Koha/Patrons/Import.t t/db_dependent/Auth_with_ldap.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Bug 20443 depends on bug 12159, which changed state. Bug 12159 Summary: Duplicate borrower_add_additional_fields function https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12159 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=20443 --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Hi Josef, thanks for the rebase. I have also rebased them against master (so hard!) and added a fix for the Auth_with_ldap.t failure. I am stuck with the other failure. I think we need to know if we (dev team) want this patchset. If so we will need to have a task force to have it cleaned and pushed at the beginning of the next release cycle. It will be impossible to rebase this branch for months. Who's in? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #12)
Hi Josef, thanks for the rebase. I have also rebased them against master (so hard!) and added a fix for the Auth_with_ldap.t failure. I am stuck with the other failure.
I think we need to know if we (dev team) want this patchset. If so we will need to have a task force to have it cleaned and pushed at the beginning of the next release cycle. It will be impossible to rebase this branch for months.
Who's in?
I'm in -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Do we have a choice? We decided on the move to the new namespace as a community... so I think we don't. I'll help where I can. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Remote branch rebased against master. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Lets schedule this for the beginning of month two in the next cycle.. that way it's early enough to push it and catch any fallout during the cycle, but late enough that we're not unduly making pain for the next stable maintainer whose trying to backport bugfixes ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_20_05_target -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 --- Comment #17 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Remote branch rebased against master. The failing test is fixed! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- URL| |https://gitlab.com/joubu/Ko | |ha/commits/bug_20443 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |RM_priority -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 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=20443 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |24132 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24132 [Bug 24132] search_with_library_limits vs filter_by_branch_limitations -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |24151 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24151 [Bug 24151] Add a pseudonymization process for patrons and transactions -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Michal Denar <black23@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |black23@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |5161 See Also|https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=5161 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5161 [Bug 5161] patron attributes clearing if duplicate warning -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Axelle Aix-Marseille Université <axelle.clarisse@univ-amu.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |axelle.clarisse@univ-amu.fr -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=12638 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 --- Comment #18 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Remote branch rebased against master. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #19 from Josef Moravec <josef.moravec@gmail.com> --- Hello Jonathan, thanks to rebasing this again. Firstly, there are some trivials found by qa tools: FAIL Koha/Patron/Attribute.pm FAIL pod *** ERROR: empty =head3 in file Koha/Patron/Attribute.pm *** ERROR: empty =head3 in file Koha/Patron/Attribute.pm FAIL Koha/Patron/Attributes.pm FAIL pod *** ERROR: empty =head3 in file Koha/Patron/Attributes.pm FAIL pod coverage POD is missing for merge_with POD is missing for object_class FAIL Koha/Patrons.pm FAIL spelling paramter ==> parameter Otherwise it looks nice. Note for QA: this is false positive: FAIL Koha/Patron/Attribute.pm FAIL forbidden patterns forbidden pattern: Warning: The 'sub type' may be wrong is declared in a Koha::* package (see bug 15446) (line 63) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #20 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Fixed, Josef! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #21 from Josef Moravec <josef.moravec@gmail.com> --- (In reply to Jonathan Druart from comment #20)
Fixed, Josef!
Thanks Jonathan Signed off! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |tomascohen@gmail.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #22 from Nick Clemens <nick@bywatersolutions.com> --- I tested the following, let me know if i missed elsewhere: editing patron satff editing patron opac importing patron patron creation/update via ldap Branch limitations do not save Cannot load patron if attribute value not in authorized values (OK, but should have a way to identify) Opac update of attributes fails, blank value passed to update request Patron import with 'replace only included attributes' fails Can't use string ("Koha::Patron::Attribute") as a HASH ref while "strict refs" in use at /kohadevbox/koha/Koha/Object.pm line 625 Attributes in ILSDI shows some extra info: borrowernumber, authorised_value_category - is noisier than without patch Test failures: Test Summary Report ------------------- t/00-testcritic.t (Wstat: 256 Tests: 1020 Failed: 1) Failed test: 732 Non-zero exit status: 1 t/00-valid-xml.t (Wstat: 512 Tests: 135 Failed: 2) Failed tests: 1-2 Non-zero exit status: 2 t/Logger.t (Wstat: 256 Tests: 1 Failed: 1) Failed test: 1 Non-zero exit status: 1 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #23 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Nick Clemens from comment #22) So basically nothing was working, thanks for having caught that Nick!
I tested the following, let me know if i missed elsewhere: editing patron satff editing patron opac importing patron patron creation/update via ldap
Branch limitations do not save
Fixed.
Cannot load patron if attribute value not in authorized values (OK, but should have a way to identify)
Fixed, I will also open a bug report.
Opac update of attributes fails, blank value passed to update request
Fixed.
Patron import with 'replace only included attributes' fails Can't use string ("Koha::Patron::Attribute") as a HASH ref while "strict refs" in use at /kohadevbox/koha/Koha/Object.pm line 625
Fixed.
Attributes in ILSDI shows some extra info: borrowernumber, authorised_value_category - is noisier than without patch
Yes, I let a FIXME in the code (which refers to bug 14257 comment 15). I would be happy to fix it you have an idea about what's best here.
Test failures: Test Summary Report ------------------- t/00-testcritic.t (Wstat: 256 Tests: 1020 Failed: 1) Failed test: 732 Non-zero exit status: 1 t/00-valid-xml.t (Wstat: 512 Tests: 135 Failed: 2) Failed tests: 1-2 Non-zero exit status: 2 t/Logger.t (Wstat: 256 Tests: 1 Failed: 1) Failed test: 1 Non-zero exit status: 1
They pass for me. Maybe not related to this patchset? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #24 from Nick Clemens <nick@bywatersolutions.com> --- Dies on importing wrong attribute To recreate: Create an import file like: surname,firstname,categorycode,branchcode,patron_attributes,cardnumber Acosta,Edna,T,FFL,"NOT_EXIST:1",23529001000463 Attempt to import Kaboom "Replace only included patron attributes" deletes all To test: Define attributes TEST1, TEST2, TEST3 Assign values to these for a patron, Edna for example Import a file like: surname,firstname,categorycode,branchcode,patron_attributes,cardnumber Acosta,Edna,T,FFL,"TEST1:1",23529001000463 All attributes deleted -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #25 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Nick Clemens from comment #24)
Dies on importing wrong attribute To recreate: Create an import file like: surname,firstname,categorycode,branchcode,patron_attributes,cardnumber Acosta,Edna,T,FFL,"NOT_EXIST:1",23529001000463 Attempt to import Kaboom
"Replace only included patron attributes" deletes all To test: Define attributes TEST1, TEST2, TEST3 Assign values to these for a patron, Edna for example Import a file like: surname,firstname,categorycode,branchcode,patron_attributes,cardnumber Acosta,Edna,T,FFL,"TEST1:1",23529001000463 All attributes deleted
Thanks Nick, good catch! I fixed these two and add more tests to cover them. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #26 from Nick Clemens <nick@bywatersolutions.com> --- Last changes fixed the issues, everything I have thought to test works well, tests pass, code looks good. Passing QA, let's get this in -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 --- Comment #27 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101122 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101122&action=edit Bug 20443: Revert some stuffs from 12159 These methods have been added to Koha::Patron::Attribute but are wrong, see bug 18339 We should use ->type->$method instead Moreover the tests exist in another subtest, we do not need them. Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #28 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101123 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101123&action=edit Bug 20443: Remove opac_display and opac_editable from Patron::Attribute Same as previously for methods that have been added by bug 17792. It's better to be explicite and tell we are fetch the related attribute's type Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #29 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101124 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101124&action=edit Bug 20443: Move GetBorrowerAttributeValue to Koha::Patron->get_extended_attribute_value We want to retrieve a specific patron's attribute for a given patron. We then add a new method to Koha::Patron. This patch add a getter method ->get_extended_attribute_value to use the DBIx::Class relation Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #30 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101125 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101125&action=edit Bug 20443: Move GetBorrowerAttributes to Koha::Patron - ILSDI FIXME See comment Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #31 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101126 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101126&action=edit Bug 20443: Move GetBorrowerAttributes to Koha::Patron->extended_attributes The GetBorrowerAttributes subroutine return the attributes for a given patron. Using get_extended_attributes we can acchieve it easily. The problematic here is to restore the method's name (value vs attribute, value_description vs description of the authorised value, as well as display_checkout that should not be a method of Attribute, but Attribute::Type instead) value_description was used when the attribute types were attached to an authorised value category. To avoid the necessary test in template and controller there is now a $attribute->description method that will display either the attribute's value OR the value of the authorised value when needed. We should certainly use this one from few other places. Notes: * This patch rename Koha::Patron->attributes with Koha::Patron->get_extended_attributes. It will be renamed with Koha::Patron->extended_attributes in ones of the next patches when it will become a setter as well. * GetBorrowerAttributes did not care about the library limits, we still do not * The opac_only flag was not used outside of test, we drop it off. * To maintain the existing behavior we add a default order-by clause to the search method [code, attribute] * From C4::Letters::_parseletter we always display the staff description of the AV, There is now a FIXME to warn about it * FIXMEs are not regressions, existing behaviors must be kept * TODO add a new check to bug 21010 to search for inconsistencies in patron's attributes attached to non-existent authorised values * One test has been updated in Modifications.t, order_by is now by default set to ['code', 'attribute'] Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #32 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101127 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101127&action=edit Bug 20443: Improve filtering method Koha::Patron::Attributes->search mimicks what is done in Koha::AuthorisedValues->search. But actually it should be more explicit when the caller use it. For instance filter_by_branch_limitation (see discussion on bug 11983). This will be useful for the following patches as we will need a way to replace the $no_branch_limit flag. When the $no_branch_limit flag is called, a simple ->search call should be done. When we want to limit on a specific library we can pass the branchcode in paramter of filter_by_branch_limitation (this is not used yet). If not passed the logged-in user library will be used by default. Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #33 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101128 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101128&action=edit Bug 20443: Remove DeleteBorrowerAttributes This subroutine was only used once, easy to replace. SetBorrowerAttributes must replace the attributes for the ones logged-in user is allowed to edit, that's why we filter by the library limits Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #34 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101129 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101129&action=edit Bug 20443: Remove DeleteBorrowerAttribute Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #35 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101130 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101130&action=edit Bug 20443: Remove UpdateBorrowerAttribute and SetBorrowerAttributes This patch replace Koha::Patron->get_extended_attributes with ->extended_attributes It's now a getter a setter method. It permits to replace UpdateBorrowerAttribute and use create_related from DBIx::Class Notes: * We face the same variable names difference than in a previous patch (value vs attribute) Bug 20443: Remove SetBorrowerAttributes squash + RM get_extended_attributes RM get_extended_attributes SQUASH Bug 20443: Remove UpdateBorrowerAttribute and SetBorrowerAttribute Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #36 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101131 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101131&action=edit Bug 20443: Remove CheckUniqueness There is already a method in Koha::Patron::Attribute to check the uniqueness constraint, let us it to replace CheckUniqueness Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #37 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101132 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101132&action=edit Bug 20443: Move C4::Members::AttributeTypes::GetAttributeTypes to Koha::Patron::Attribute::Types We can then now start to move methods from C4::Members::AttributeTypes as well. Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #38 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101133 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101133&action=edit Bug 20443: Remove num_patron Replace C4::Members::AttributeTypes->num_patron with Koha::Patrons->filter_by_attribute_type Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #39 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101134 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101134&action=edit Bug 20443: Remove C4::Members::AttributeTypes We do no longer need this package, we can use Koha::Patron::Attribute::Types directly instead. Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #40 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101135 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101135&action=edit Bug 20443: Remove SearchIdMatchingAttribute - prove we are not cheating Tests are still passing that way, we can continue Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #41 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101136 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101136&action=edit Bug 20443: Remove SearchIdMatchingAttribute Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #42 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101137 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101137&action=edit Bug 20443: Remove extended_attributes_merge TODO We need tests here! Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #43 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101138 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101138&action=edit Bug 20443: Remove extended_attributes_code_value_arrayref AND C4::Members::Attributes Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #44 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101139 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101139&action=edit Bug 20443: (follow-up) Move GetBorrowerAttributes to Koha::Patron - ILSDI Either we display everything, or only the info from the attribute itself (i.e. not its type) Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #45 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101140 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101140&action=edit Bug 20443: (follow-up) Fix t/db_dependent/Koha/Patrons.t Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #46 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101141 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101141&action=edit Bug 20443: Fix Auth_with_ldap.t Must be a hashref Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #47 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101142 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101142&action=edit Bug 20443: Use search_with_library_limits for attribute types Dealt with that previously in the module during the rebase. It conflicted with bug 23281. Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #48 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101143 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101143&action=edit Bug 20443: Fix Patrons/Import.t ->get_patrons has been removed but not replaced! Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #49 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101144 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101144&action=edit Bug 20443: Fix add/edit for checkboxes Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #50 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101145 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101145&action=edit Bug 20443: Fix POD Fixes the following failures: FAIL Koha/Patron/Attribute.pm FAIL pod *** ERROR: empty =head3 in file Koha/Patron/Attribute.pm *** ERROR: empty =head3 in file Koha/Patron/Attribute.pm FAIL Koha/Patron/Attributes.pm FAIL pod *** ERROR: empty =head3 in file Koha/Patron/Attributes.pm FAIL pod coverage POD is missing for merge_with POD is missing for object_class FAIL Koha/Patrons.pm FAIL spelling paramter ==> parameter except the missing pod for object_class, not needed. Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #51 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101146 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101146&action=edit Bug 20443: Fix creation of new patron with attributes Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #52 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101147 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101147&action=edit Bug 20443: Do not explode if attribute does not have a valid av Template process failed: undef error - Can't call method "lib" on an undefined value at /kohadevbox/koha/Koha/Patron/Attribute.pm line 109. at /kohadevbox/koha/C4/Templates.pm line 122 We should not crash as it if the attribute is linked to an invalid authorised value (for instance the AV has been deleted) We must provide a check in the search_for_data_inconsistencies.pl script Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #53 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101148 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101148&action=edit Bug 20443: Fix patron modification approval There was a mismatch between "value" and "attribute". Before this patchset, "value" was sometimes used, but then it comes "attribute" to match the DB column's name. We must keep both here, when an upgrade is done we could have "value" that is still in the borrower_modifications.extended_attributes JSON Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #54 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101149 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101149&action=edit Bug 20443: Handle non existent attribute when importing patrons There is much more to do here, but this patch has the same behavior than before: a warn is displayed in the log, the UI is not aware of it Signed-off-by: Signed-off-by: Nick Clemens <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=20443 --- Comment #55 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101150 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101150&action=edit Bug 20443: Fix merge_with behavior merge_with were returning Koha::Patron::Attribute for existing attribute and hashref for new attribute. Signed-off-by: Signed-off-by: Nick Clemens <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=20443 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #101122|0 |1 is obsolete| | Attachment #101123|0 |1 is obsolete| | Attachment #101124|0 |1 is obsolete| | Attachment #101125|0 |1 is obsolete| | Attachment #101126|0 |1 is obsolete| | Attachment #101127|0 |1 is obsolete| | Attachment #101128|0 |1 is obsolete| | Attachment #101129|0 |1 is obsolete| | Attachment #101130|0 |1 is obsolete| | Attachment #101131|0 |1 is obsolete| | Attachment #101132|0 |1 is obsolete| | Attachment #101133|0 |1 is obsolete| | Attachment #101134|0 |1 is obsolete| | Attachment #101135|0 |1 is obsolete| | Attachment #101136|0 |1 is obsolete| | Attachment #101137|0 |1 is obsolete| | Attachment #101138|0 |1 is obsolete| | Attachment #101139|0 |1 is obsolete| | Attachment #101140|0 |1 is obsolete| | Attachment #101141|0 |1 is obsolete| | Attachment #101142|0 |1 is obsolete| | Attachment #101143|0 |1 is obsolete| | Attachment #101144|0 |1 is obsolete| | Attachment #101145|0 |1 is obsolete| | Attachment #101146|0 |1 is obsolete| | Attachment #101147|0 |1 is obsolete| | Attachment #101148|0 |1 is obsolete| | Attachment #101149|0 |1 is obsolete| | Attachment #101150|0 |1 is obsolete| | --- Comment #56 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101209 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101209&action=edit Bug 20443: Revert some stuffs from 12159 These methods have been added to Koha::Patron::Attribute but are wrong, see bug 18339 We should use ->type->$method instead Moreover the tests exist in another subtest, we do not need them. Signed-off-by: Nick Clemens <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=20443 --- Comment #57 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101210 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101210&action=edit Bug 20443: Remove opac_display and opac_editable from Patron::Attribute Same as previously for methods that have been added by bug 17792. It's better to be explicite and tell we are fetch the related attribute's type Signed-off-by: Nick Clemens <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=20443 --- Comment #58 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101211 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101211&action=edit Bug 20443: Move GetBorrowerAttributeValue to Koha::Patron->get_extended_attribute_value We want to retrieve a specific patron's attribute for a given patron. We then add a new method to Koha::Patron. This patch add a getter method ->get_extended_attribute_value to use the DBIx::Class relation Signed-off-by: Nick Clemens <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=20443 --- Comment #59 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101212 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101212&action=edit Bug 20443: Move GetBorrowerAttributes to Koha::Patron - ILSDI FIXME See comment Signed-off-by: Nick Clemens <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=20443 --- Comment #60 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101213 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101213&action=edit Bug 20443: Move GetBorrowerAttributes to Koha::Patron->extended_attributes The GetBorrowerAttributes subroutine return the attributes for a given patron. Using get_extended_attributes we can acchieve it easily. The problematic here is to restore the method's name (value vs attribute, value_description vs description of the authorised value, as well as display_checkout that should not be a method of Attribute, but Attribute::Type instead) value_description was used when the attribute types were attached to an authorised value category. To avoid the necessary test in template and controller there is now a $attribute->description method that will display either the attribute's value OR the value of the authorised value when needed. We should certainly use this one from few other places. Notes: * This patch rename Koha::Patron->attributes with Koha::Patron->get_extended_attributes. It will be renamed with Koha::Patron->extended_attributes in ones of the next patches when it will become a setter as well. * GetBorrowerAttributes did not care about the library limits, we still do not * The opac_only flag was not used outside of test, we drop it off. * To maintain the existing behavior we add a default order-by clause to the search method [code, attribute] * From C4::Letters::_parseletter we always display the staff description of the AV, There is now a FIXME to warn about it * FIXMEs are not regressions, existing behaviors must be kept * TODO add a new check to bug 21010 to search for inconsistencies in patron's attributes attached to non-existent authorised values * One test has been updated in Modifications.t, order_by is now by default set to ['code', 'attribute'] Signed-off-by: Nick Clemens <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=20443 --- Comment #61 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101214 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101214&action=edit Bug 20443: Improve filtering method Koha::Patron::Attributes->search mimicks what is done in Koha::AuthorisedValues->search. But actually it should be more explicit when the caller use it. For instance filter_by_branch_limitation (see discussion on bug 11983). This will be useful for the following patches as we will need a way to replace the $no_branch_limit flag. When the $no_branch_limit flag is called, a simple ->search call should be done. When we want to limit on a specific library we can pass the branchcode in paramter of filter_by_branch_limitation (this is not used yet). If not passed the logged-in user library will be used by default. Signed-off-by: Nick Clemens <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=20443 --- Comment #62 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101215 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101215&action=edit Bug 20443: Remove DeleteBorrowerAttributes This subroutine was only used once, easy to replace. SetBorrowerAttributes must replace the attributes for the ones logged-in user is allowed to edit, that's why we filter by the library limits Signed-off-by: Nick Clemens <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=20443 --- Comment #63 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101216 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101216&action=edit Bug 20443: Remove DeleteBorrowerAttribute Signed-off-by: Nick Clemens <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=20443 --- Comment #64 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101217 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101217&action=edit Bug 20443: Remove UpdateBorrowerAttribute and SetBorrowerAttributes This patch replace Koha::Patron->get_extended_attributes with ->extended_attributes It's now a getter a setter method. It permits to replace UpdateBorrowerAttribute and use create_related from DBIx::Class Notes: * We face the same variable names difference than in a previous patch (value vs attribute) Bug 20443: Remove SetBorrowerAttributes squash + RM get_extended_attributes RM get_extended_attributes SQUASH Bug 20443: Remove UpdateBorrowerAttribute and SetBorrowerAttribute Signed-off-by: Nick Clemens <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=20443 --- Comment #65 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101218 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101218&action=edit Bug 20443: Remove CheckUniqueness There is already a method in Koha::Patron::Attribute to check the uniqueness constraint, let us it to replace CheckUniqueness Signed-off-by: Nick Clemens <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=20443 --- Comment #66 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101219 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101219&action=edit Bug 20443: Move C4::Members::AttributeTypes::GetAttributeTypes to Koha::Patron::Attribute::Types We can then now start to move methods from C4::Members::AttributeTypes as well. Signed-off-by: Nick Clemens <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=20443 --- Comment #67 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101220 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101220&action=edit Bug 20443: Remove num_patron Replace C4::Members::AttributeTypes->num_patron with Koha::Patrons->filter_by_attribute_type Signed-off-by: Nick Clemens <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=20443 --- Comment #68 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101221 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101221&action=edit Bug 20443: Remove C4::Members::AttributeTypes We do no longer need this package, we can use Koha::Patron::Attribute::Types directly instead. Signed-off-by: Nick Clemens <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=20443 --- Comment #69 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101222 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101222&action=edit Bug 20443: Remove SearchIdMatchingAttribute - prove we are not cheating Tests are still passing that way, we can continue Signed-off-by: Nick Clemens <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=20443 --- Comment #70 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101223 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101223&action=edit Bug 20443: Remove SearchIdMatchingAttribute Signed-off-by: Nick Clemens <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=20443 --- Comment #71 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101224 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101224&action=edit Bug 20443: Remove extended_attributes_merge TODO We need tests here! Signed-off-by: Nick Clemens <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=20443 --- Comment #72 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101225 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101225&action=edit Bug 20443: Remove extended_attributes_code_value_arrayref AND C4::Members::Attributes Signed-off-by: Nick Clemens <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=20443 --- Comment #73 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101226 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101226&action=edit Bug 20443: (follow-up) Move GetBorrowerAttributes to Koha::Patron - ILSDI Either we display everything, or only the info from the attribute itself (i.e. not its type) Signed-off-by: Nick Clemens <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=20443 --- Comment #74 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101227 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101227&action=edit Bug 20443: (follow-up) Fix t/db_dependent/Koha/Patrons.t Signed-off-by: Nick Clemens <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=20443 --- Comment #75 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101228 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101228&action=edit Bug 20443: Fix Auth_with_ldap.t Must be a hashref Signed-off-by: Nick Clemens <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=20443 --- Comment #76 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101229 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101229&action=edit Bug 20443: Use search_with_library_limits for attribute types Dealt with that previously in the module during the rebase. It conflicted with bug 23281. Signed-off-by: Nick Clemens <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=20443 --- Comment #77 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101230 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101230&action=edit Bug 20443: Fix Patrons/Import.t ->get_patrons has been removed but not replaced! Signed-off-by: Nick Clemens <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=20443 --- Comment #78 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101231 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101231&action=edit Bug 20443: Fix add/edit for checkboxes Signed-off-by: Nick Clemens <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=20443 --- Comment #79 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101232 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101232&action=edit Bug 20443: Fix POD Fixes the following failures: FAIL Koha/Patron/Attribute.pm FAIL pod *** ERROR: empty =head3 in file Koha/Patron/Attribute.pm *** ERROR: empty =head3 in file Koha/Patron/Attribute.pm FAIL Koha/Patron/Attributes.pm FAIL pod *** ERROR: empty =head3 in file Koha/Patron/Attributes.pm FAIL pod coverage POD is missing for merge_with POD is missing for object_class FAIL Koha/Patrons.pm FAIL spelling paramter ==> parameter except the missing pod for object_class, not needed. Signed-off-by: Nick Clemens <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=20443 --- Comment #80 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101233 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101233&action=edit Bug 20443: Fix creation of new patron with attributes Signed-off-by: Nick Clemens <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=20443 --- Comment #81 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101234 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101234&action=edit Bug 20443: Do not explode if attribute does not have a valid av Template process failed: undef error - Can't call method "lib" on an undefined value at /kohadevbox/koha/Koha/Patron/Attribute.pm line 109. at /kohadevbox/koha/C4/Templates.pm line 122 We should not crash as it if the attribute is linked to an invalid authorised value (for instance the AV has been deleted) We must provide a check in the search_for_data_inconsistencies.pl script Signed-off-by: Nick Clemens <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=20443 --- Comment #82 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101235 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101235&action=edit Bug 20443: Fix patron modification approval There was a mismatch between "value" and "attribute". Before this patchset, "value" was sometimes used, but then it comes "attribute" to match the DB column's name. We must keep both here, when an upgrade is done we could have "value" that is still in the borrower_modifications.extended_attributes JSON Signed-off-by: Nick Clemens <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=20443 --- Comment #83 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101236 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101236&action=edit Bug 20443: Handle non existent attribute when importing patrons There is much more to do here, but this patch has the same behavior than before: a warn is displayed in the log, the UI is not aware of it Signed-off-by: Nick Clemens <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=20443 --- Comment #84 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101237 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101237&action=edit Bug 20443: Fix merge_with behavior merge_with were returning Koha::Patron::Attribute for existing attribute and hashref for new attribute. Signed-off-by: Nick Clemens <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=20443 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |20.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 --- Comment #85 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 --- Comment #86 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Ohhh -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 --- Comment #87 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101472 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101472&action=edit Bug 20443: Add missing use statement This is a follow-up for commit 836e950d25148ca04ed0561c487772e3b0f7f8ee Bug 20443: Remove SearchIdMatchingAttribute We removed the use of C4::Members::Attributes, that was using Text::CSV -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joy@bywatersolutions.com --- Comment #88 from Joy Nelson <joy@bywatersolutions.com> --- not backported to 19.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_20_05_candidate, | |RM_priority | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 --- Comment #89 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 104519 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104519&action=edit Bug 20443: (follow-up) Remove C4::Members::AttributeTypes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 --- Comment #90 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Jonathan Druart from comment #89)
Created attachment 104519 [details] [review] Bug 20443: (follow-up) Remove C4::Members::AttributeTypes
Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |25567 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25567 [Bug 25567] borrower_attribute_types.category_code must be set to undef if not set -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |27957 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27957 [Bug 27957] Attributes removed in LDAP -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |28293 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28293 [Bug 28293] Wrong key used in Patrons::Import->generate_patron_attributes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |28345 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28345 [Bug 28345] Patron attributes no longer have option to select empty class -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |35817 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35817 [Bug 35817] Wrong hint on patron's category when batch update patron -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20443 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=30657 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org