The original patchset on bug 19532 replicated what was in the 'reserves' table, and thus introduced several column names that we should better avoid, to adhere to the coding guidelines.
Hey Tomás Happy to work on this. Can you please advise what changes are to be made? The coding guidelines didn't seem to have what I was looking for https://wiki.koha-community.org/wiki/Coding_Guidelines#Database and the Bug 25260 report is hard to follow. Aleisha
(In reply to Aleisha Amohia from comment #1) > Hey Tomás > > Happy to work on this. Can you please advise what changes are to be made? > The coding guidelines didn't seem to have what I was looking for > https://wiki.koha-community.org/wiki/Coding_Guidelines#Database and the Bug > 25260 report is hard to follow. > > Aleisha Look at the column names here: https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123072
Created attachment 132379 [details] [review] Bug 30291: Database changes to recalls table
Created attachment 132380 [details] [review] Bug 30291: [DBIx] Updated schema files
Created attachment 132381 [details] [review] Bug 30291: [DBIx] Updating boolean flags in Recall schema file
Created attachment 132382 [details] [review] Bug 30291: [DBIx] Package fixes in Recall schema file
Created attachment 132383 [details] [review] Bug 30291: Changes to tests t/db_dependent/Circulation.t t/db_dependent/Circulation/CalcFine.t t/db_dependent/Circulation/transferbook.t t/db_dependent/Holds.t t/db_dependent/Koha/Biblio.t t/db_dependent/Koha/Item.t t/db_dependent/Koha/Patron.t t/db_dependent/Koha/Recall.t t/db_dependent/Koha/Recalls.t t/db_dependent/XSLT.t
Created attachment 132384 [details] [review] Bug 30291: Changes to controller scripts
Created attachment 132385 [details] [review] Bug 30291: Changes to staff client files
Created attachment 132386 [details] [review] Bug 30291: Changes to OPAC files
Please take a look at the patches and ensure I caught all of the cases, and do a test of the recalls workflows to make sure everything is as normal with the updated column names.
Created attachment 134359 [details] [review] Bug 30291: Database changes to recalls table Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134360 [details] [review] Bug 30291: Update boolean flags in Recall schema file Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134361 [details] [review] Bug 30291: Changes to tests t/db_dependent/Circulation.t t/db_dependent/Circulation/CalcFine.t t/db_dependent/Circulation/transferbook.t t/db_dependent/Holds.t t/db_dependent/Koha/Biblio.t t/db_dependent/Koha/Item.t t/db_dependent/Koha/Patron.t t/db_dependent/Koha/Recall.t t/db_dependent/Koha/Recalls.t t/db_dependent/XSLT.t Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134362 [details] [review] Bug 30291: Changes to controller scripts Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134363 [details] [review] Bug 30291: Changes to staff client files Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134364 [details] [review] Bug 30291: Changes to OPAC files Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134365 [details] [review] Bug 30291: Fix permissions Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134366 [details] [review] Bug 30291: (QA follow-up) Keep recall_id column name Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134367 [details] [review] Bug 30291: Updated schema files [DO NOT PUSH] Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134389 [details] [review] Bug 30291: Database changes to recalls table Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 134390 [details] [review] Bug 30291: Update boolean flags in Recall schema file Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 134391 [details] [review] Bug 30291: Changes to tests t/db_dependent/Circulation.t t/db_dependent/Circulation/CalcFine.t t/db_dependent/Circulation/transferbook.t t/db_dependent/Holds.t t/db_dependent/Koha/Biblio.t t/db_dependent/Koha/Item.t t/db_dependent/Koha/Patron.t t/db_dependent/Koha/Recall.t t/db_dependent/Koha/Recalls.t t/db_dependent/XSLT.t Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 134392 [details] [review] Bug 30291: Changes to controller scripts Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 134393 [details] [review] Bug 30291: Changes to staff client files Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 134394 [details] [review] Bug 30291: Changes to OPAC files Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 134395 [details] [review] Bug 30291: Fix permissions Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 134396 [details] [review] Bug 30291: (QA follow-up) Keep recall_id column name Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 134397 [details] [review] Bug 30291: Updated schema files [DO NOT PUSH] Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
The QA script is happy and this all appears to still be working as expected... so Passing QA. However, building the DBIC Schema appears to spit out lots of overloaded method name errors (at the top as you run the QA script).. I think they're harmless in this case but it's a little disturbing to see.
Test suite fails for me : # Failed test 'Recalls tests' # at /kohadevbox/koha/t/db_dependent/Koha/Item.t line 1421. C4::Letters::_substitute_tables(): DBI Exception: DBD::mysql::st execute failed: Unknown column 'id' in 'where clause' at /kohadevbox/koha/C4/Letters.pm line 646 Looks like this is not enought : Bug 30291: (QA follow-up) Keep recall_id column name There are more places with recall_id => id, begining with C4::Letters::_parseletter_sth Maybe squash this revert with main patches.
Created attachment 134629 [details] [review] Bug 30291: (QA follow-up) Fix for revert to recall_id
Created attachment 134630 [details] [review] Bug 30291: (QA follow-up) Fix for revert to recall_id
Follow-up added to take care of the missing id -> recall_id changes. Beware, the 'DO NOT PUSH' patch contains some changes below the dbic build marker line.. so you'll need to apply those yourself after building the schema.. (or just use the patch and rename the commit message)
Pushed to master for 22.05, thanks to everybody involved [U+1F984]