[Bug 16889] New: Move the ::columns subroutines to Koha::Objects->columns
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 Bug ID: 16889 Summary: Move the ::columns subroutines to Koha::Objects->columns Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: ASSIGNED Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart@bugs.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org Currently we have 3 subroutines to retrieve the columns of tables: C4::Members::columns, C4::Items::columns and C4::Items::biblioitems_columns. It will be easier to have a Koha::Objects->columns method to provide the different classes to retrieve their related column names. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |16846 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16846 [Bug 16846] Move patron related code to Koha::Patron -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=16889 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 53240 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53240&action=edit Bug 16889: Add Koha::Objects->columns Currently we have 3 subroutines to retrieve the columns of tables: C4::Members::columns, C4::Items::columns and C4::Items::biblioitems_columns. It is easier to have a Koha::Objects->columns method to provide the different classes to retrieve their related column names. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 53241 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53241&action=edit Bug 16889: Remove C4::Members::columns and use Koha::Patrons->columns instead Test plan: Import some patrons (tools/import_borrowers.pl) and make sure it imports the patrons correctly. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 53243 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53243&action=edit Bug 16889: Remove C4::Items::columns and use Koha::Items->columns instead Test plan: Add new rule to the "Automatic item modifications by age" tools (tools/automatic_item_modification_by_age.pl) and make sure the columns of the items table are correctly displayed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 53244 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53244&action=edit Bug 16889: Remove C4::Items::biblioitems_columns and use Koha::Biblioitems->columns instead Test plan: Add new rule to the "Automatic item modifications by age" tools (tools/automatic_item_modification_by_age.pl) and make sure the columns of the biblioitems table are correctly displayed in the 'Conditions' dropdown list. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 --- Comment #5 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 53241 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53241 Bug 16889: Remove C4::Members::columns and use Koha::Patrons->columns instead Review of attachment 53241: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=16889&attachment=53241) ----------------------------------------------------------------- ::: C4/Members.pm @@ -583,5 @@
- else { - @data = (); - } - return @data; -}
Farewell, my baby. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 --- Comment #6 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 53243 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53243 Bug 16889: Remove C4::Items::columns and use Koha::Items->columns instead Review of attachment 53243: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=16889&attachment=53243) ----------------------------------------------------------------- ::: C4/Items.pm @@ +3067,4 @@
my $dbh = C4::Context->dbh; my @errors; + my @item_columns = map { "items.$_" } Koha::Items->columns;
This should be triggered by t/db_dependent/Items/AutomaticItemModificationByAge.t as a test. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtompset@hotmail.com --- Comment #7 from M. Tompsett <mtompset@hotmail.com> --- Commit #1: Test in patch is sufficient to test. Commit #2: Yes, I can get into Tools->Import Patrons just fine. Couldn't find any other instance of C4::Members::columns, so this patch has it covered as far as I can tell. Commit #3: The Items.pm change is tested enough by prove t/db_dependent/Items/AutomaticItemModificationByAge.t The automatic item modification by age tool removing a rule is unintuitive. Code eyeballs well enough. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #53240|0 |1 is obsolete| | --- Comment #8 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 53292 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53292&action=edit Bug 16889: Add Koha::Objects->columns Currently we have 3 subroutines to retrieve the columns of tables: C4::Members::columns, C4::Items::columns and C4::Items::biblioitems_columns. It is easier to have a Koha::Objects->columns method to provide the different classes to retrieve their related column names. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #53241|0 |1 is obsolete| | --- Comment #9 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 53293 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53293&action=edit Bug 16889: Remove C4::Members::columns and use Koha::Patrons->columns instead Test plan: Import some patrons (tools/import_borrowers.pl) and make sure it imports the patrons correctly. NOTE: *sniff sniff* Good bye. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #53243|0 |1 is obsolete| | --- Comment #10 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 53294 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53294&action=edit Bug 16889: Remove C4::Items::columns and use Koha::Items->columns instead Test plan: Add new rule to the "Automatic item modifications by age" tools (tools/automatic_item_modification_by_age.pl) and make sure the columns of the items table are correctly displayed. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #53244|0 |1 is obsolete| | --- Comment #11 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 53295 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53295&action=edit Bug 16889: Remove C4::Items::biblioitems_columns and use Koha::Biblioitems->columns instead Test plan: Add new rule to the "Automatic item modifications by age" tools (tools/automatic_item_modification_by_age.pl) and make sure the columns of the biblioitems table are correctly displayed in the 'Conditions' dropdown list. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 M. Tompsett <mtompset@hotmail.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=16889 --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to M. Tompsett from comment #7)
Commit #1: Test in patch is sufficient to test.
I don't understand, what do you suggest? The method is covered by the test in Objects.t. I have arbitrary chosen to test the Patron module, but I don't see the point to add the same test for each module based on Koha::Object. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 --- Comment #13 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Jonathan Druart from comment #12)
(In reply to M. Tompsett from comment #7)
Commit #1: Test in patch is sufficient to test.
I don't understand, what do you suggest?
I was just stating what I did to test the patches. Yes, it was arbitrary that you chose Patrons, but anything would be sufficient to trigger the columns code, which is the point of the test. Nothing wrong. Just a statement. I'm hoping this moves ahead to QA without those sorts of misunderstandings. Sorry I was unclear. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |16917 CC| |kyle@bywatersolutions.com Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16917 [Bug 16917] Error when importing patrons, Column 'checkprevcheckout' cannot be null -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 Kyle M Hall <kyle@bywatersolutions.com> 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=16889 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #53292|0 |1 is obsolete| | --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 53379 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53379&action=edit Bug 16889: Add Koha::Objects->columns Currently we have 3 subroutines to retrieve the columns of tables: C4::Members::columns, C4::Items::columns and C4::Items::biblioitems_columns. It is easier to have a Koha::Objects->columns method to provide the different classes to retrieve their related column names. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #53293|0 |1 is obsolete| | Attachment #53294|0 |1 is obsolete| | Attachment #53295|0 |1 is obsolete| | Attachment #53379|0 |1 is obsolete| | --- Comment #15 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 53380 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53380&action=edit Bug 16889: Add Koha::Objects->columns Currently we have 3 subroutines to retrieve the columns of tables: C4::Members::columns, C4::Items::columns and C4::Items::biblioitems_columns. It is easier to have a Koha::Objects->columns method to provide the different classes to retrieve their related column names. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 53381 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53381&action=edit Bug 16889: Remove C4::Members::columns and use Koha::Patrons->columns instead Test plan: Import some patrons (tools/import_borrowers.pl) and make sure it imports the patrons correctly. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 --- Comment #17 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 53382 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53382&action=edit Bug 16889: Remove C4::Items::columns and use Koha::Items->columns instead Test plan: Add new rule to the "Automatic item modifications by age" tools (tools/automatic_item_modification_by_age.pl) and make sure the columns of the items table are correctly displayed. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 --- Comment #18 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 53383 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53383&action=edit Bug 16889: Remove C4::Items::biblioitems_columns and use Koha::Biblioitems->columns instead Test plan: Add new rule to the "Automatic item modifications by age" tools (tools/automatic_item_modification_by_age.pl) and make sure the columns of the biblioitems table are correctly displayed in the 'Conditions' dropdown list. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #19 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 16.11, thanks Jonathan! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |critical Priority|P5 - low |P1 - high CC| |philippe.blouin@inlibro.com --- Comment #20 from Blou <philippe.blouin@inlibro.com> --- When adding a new member, pressing save, this returns to the same page. This broke master. Not sure what the status should be. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|critical |minor Priority|P1 - high |P5 - low --- Comment #21 from Blou <philippe.blouin@inlibro.com> --- Sorry. Wrong bug, meant 16909. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|minor |blocker --- Comment #22 from Blou <philippe.blouin@inlibro.com> --- Wrote to bug 16889 by mistake. This patch broke add new member. When pressing save, we cycle back to the page. Not sure what to do with the status and importance, but put in "blocker" since, well, it is kind of a big problem. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16889 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|blocker |minor --- Comment #23 from Blou <philippe.blouin@inlibro.com> --- ... I'm an idiot. Sorry for polluting this bug again. Going to 16909 now. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org