Bug 30291 - Rename recalls.* column names
Summary: Rename recalls.* column names
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Aleisha Amohia
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 19532
Blocks: 30946
  Show dependency treegraph
 
Reported: 2022-03-14 13:07 UTC by Tomás Cohen Arazi
Modified: 2023-06-08 22:26 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00


Attachments
Bug 30291: Database changes to recalls table (7.55 KB, patch)
2022-03-29 03:47 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30291: [DBIx] Updated schema files (11.25 KB, patch)
2022-03-29 03:47 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30291: [DBIx] Updating boolean flags in Recall schema file (872 bytes, patch)
2022-03-29 03:47 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30291: [DBIx] Package fixes in Recall schema file (1.41 KB, patch)
2022-03-29 03:47 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30291: Changes to tests (40.28 KB, patch)
2022-03-29 03:47 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30291: Changes to controller scripts (24.34 KB, patch)
2022-03-29 03:47 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30291: Changes to staff client files (42.63 KB, patch)
2022-03-29 03:47 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30291: Changes to OPAC files (14.54 KB, patch)
2022-03-29 03:47 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30291: Database changes to recalls table (7.58 KB, patch)
2022-04-29 11:33 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30291: Update boolean flags in Recall schema file (915 bytes, patch)
2022-04-29 11:33 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30291: Changes to tests (40.29 KB, patch)
2022-04-29 11:33 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30291: Changes to controller scripts (24.35 KB, patch)
2022-04-29 11:33 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30291: Changes to staff client files (42.56 KB, patch)
2022-04-29 11:33 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30291: Changes to OPAC files (14.55 KB, patch)
2022-04-29 11:33 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30291: Fix permissions (683 bytes, patch)
2022-04-29 11:33 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30291: (QA follow-up) Keep recall_id column name (2.87 KB, patch)
2022-04-29 11:33 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30291: Updated schema files [DO NOT PUSH] (11.96 KB, patch)
2022-04-29 11:34 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30291: Database changes to recalls table (7.65 KB, patch)
2022-04-29 15:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30291: Update boolean flags in Recall schema file (980 bytes, patch)
2022-04-29 15:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30291: Changes to tests (40.35 KB, patch)
2022-04-29 15:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30291: Changes to controller scripts (24.41 KB, patch)
2022-04-29 15:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30291: Changes to staff client files (42.62 KB, patch)
2022-04-29 15:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30291: Changes to OPAC files (14.61 KB, patch)
2022-04-29 15:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30291: Fix permissions (748 bytes, patch)
2022-04-29 15:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30291: (QA follow-up) Keep recall_id column name (2.94 KB, patch)
2022-04-29 15:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30291: Updated schema files [DO NOT PUSH] (12.02 KB, patch)
2022-04-29 15:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30291: (QA follow-up) Fix for revert to recall_id (1.38 KB, patch)
2022-05-05 10:15 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30291: (QA follow-up) Fix for revert to recall_id (4.37 KB, patch)
2022-05-05 10:29 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2022-03-14 13:07:13 UTC
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.
Comment 1 Aleisha Amohia 2022-03-23 03:55:28 UTC
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
Comment 2 Tomás Cohen Arazi 2022-03-23 11:01:29 UTC
(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
Comment 3 Aleisha Amohia 2022-03-29 03:47:13 UTC
Created attachment 132379 [details] [review]
Bug 30291: Database changes to recalls table
Comment 4 Aleisha Amohia 2022-03-29 03:47:18 UTC
Created attachment 132380 [details] [review]
Bug 30291: [DBIx] Updated schema files
Comment 5 Aleisha Amohia 2022-03-29 03:47:22 UTC
Created attachment 132381 [details] [review]
Bug 30291: [DBIx] Updating boolean flags in Recall schema file
Comment 6 Aleisha Amohia 2022-03-29 03:47:26 UTC
Created attachment 132382 [details] [review]
Bug 30291: [DBIx] Package fixes in Recall schema file
Comment 7 Aleisha Amohia 2022-03-29 03:47:31 UTC
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
Comment 8 Aleisha Amohia 2022-03-29 03:47:35 UTC
Created attachment 132384 [details] [review]
Bug 30291: Changes to controller scripts
Comment 9 Aleisha Amohia 2022-03-29 03:47:40 UTC
Created attachment 132385 [details] [review]
Bug 30291: Changes to staff client files
Comment 10 Aleisha Amohia 2022-03-29 03:47:44 UTC
Created attachment 132386 [details] [review]
Bug 30291: Changes to OPAC files
Comment 11 Aleisha Amohia 2022-03-29 03:49:10 UTC
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.
Comment 12 Tomás Cohen Arazi 2022-04-29 11:33:16 UTC
Created attachment 134359 [details] [review]
Bug 30291: Database changes to recalls table

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 Tomás Cohen Arazi 2022-04-29 11:33:22 UTC
Created attachment 134360 [details] [review]
Bug 30291: Update boolean flags in Recall schema file

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 14 Tomás Cohen Arazi 2022-04-29 11:33:27 UTC
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>
Comment 15 Tomás Cohen Arazi 2022-04-29 11:33:33 UTC
Created attachment 134362 [details] [review]
Bug 30291: Changes to controller scripts

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 Tomás Cohen Arazi 2022-04-29 11:33:40 UTC
Created attachment 134363 [details] [review]
Bug 30291: Changes to staff client files

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 17 Tomás Cohen Arazi 2022-04-29 11:33:45 UTC
Created attachment 134364 [details] [review]
Bug 30291: Changes to OPAC files

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 18 Tomás Cohen Arazi 2022-04-29 11:33:50 UTC
Created attachment 134365 [details] [review]
Bug 30291: Fix permissions

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 19 Tomás Cohen Arazi 2022-04-29 11:33:55 UTC
Created attachment 134366 [details] [review]
Bug 30291: (QA follow-up) Keep recall_id column name

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 20 Tomás Cohen Arazi 2022-04-29 11:34:01 UTC
Created attachment 134367 [details] [review]
Bug 30291: Updated schema files [DO NOT PUSH]

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 21 Martin Renvoize 2022-04-29 15:34:24 UTC
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>
Comment 22 Martin Renvoize 2022-04-29 15:34:28 UTC
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>
Comment 23 Martin Renvoize 2022-04-29 15:34:33 UTC
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>
Comment 24 Martin Renvoize 2022-04-29 15:34:38 UTC
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>
Comment 25 Martin Renvoize 2022-04-29 15:34:42 UTC
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>
Comment 26 Martin Renvoize 2022-04-29 15:34:47 UTC
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>
Comment 27 Martin Renvoize 2022-04-29 15:34:51 UTC
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>
Comment 28 Martin Renvoize 2022-04-29 15:34:56 UTC
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>
Comment 29 Martin Renvoize 2022-04-29 15:35:00 UTC
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>
Comment 30 Martin Renvoize 2022-04-29 15:36:38 UTC
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.
Comment 31 Fridolin Somers 2022-05-03 19:55:36 UTC
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.
Comment 32 Martin Renvoize 2022-05-05 10:15:48 UTC
Created attachment 134629 [details] [review]
Bug 30291: (QA follow-up) Fix for revert to recall_id
Comment 33 Martin Renvoize 2022-05-05 10:29:57 UTC
Created attachment 134630 [details] [review]
Bug 30291: (QA follow-up) Fix for revert to recall_id
Comment 34 Martin Renvoize 2022-05-05 10:31:42 UTC
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)
Comment 35 Fridolin Somers 2022-05-05 21:20:49 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄