While updating our database documentation I found ethnicity and I suspect, that this is no longer used in current versions of Koha. Related code I found so far: - database table ethnicity - database columns ethnicity and ethnotes in borrowers and deletedborrowers - ethnicitycategories in C4/Members.pm - some code in memberentry.pl I can see no way in current versions for a library to enter data in the related fields and tables. Also we have a very flexible feature for patron attributes now, that I think, should make it possible to replicate this feature, if a library wanted to use it.
Note: We need to check the table and columns are empty before dropping them. If possible migrate data to patron attributes automatically.
(In reply to comment #1) > Note: We need to check the table and columns are empty before dropping them. > If possible migrate data to patron attributes automatically. If non empty we must migrate them, we can't throw away users data.
(In reply to Chris Cormack from comment #2) > (In reply to comment #1) > > Note: We need to check the table and columns are empty before dropping them. > > If possible migrate data to patron attributes automatically. > > If non empty we must migrate them, we can't throw away users data. My inclination is to treat this like bug 7372: i.e., create an ETHNICITY authval, migrate the ethnicity table to that, then drop it. Moving the ethnicity column to a patron attribute can come later, if it all.
+1
Created attachment 37639 [details] [review] Bug 10020: Remove code related to ethnicity This is a(nother) vestige of Koha (2.2?). This patch removes unused code related to the 'ethnicity'. In detail: There is no way to fill the ethnicity table. There is no way to fill the borrowers.ethnicity and borrowers.ethnotes. BUT if borrowers.ethnicity exists, the value is displayed on members/moremember.pl (and only here). Test plan: Apply this patch and confirm there is no regression on adding/updating/deleting patrons.
Created attachment 37640 [details] [review] Bug 10020: Remove DB table and fields related to ethnicity This patch remove the ethnicity table and the ethnicity and ethnotes fields from the 3 tables: borrowers, deletedborrowers and borrower_modifications IF no data exist. Test plan: 0/ Apply this patch and update the Koha::Schema (misc/devel/update_dbix_class_files.pl). 1/ Make sure the ethnicity has at least 1 entry 2/ Launch the updatedb entry and confirm you get a warning message and the DB structure is not updated. 3/ truncate the ethnicity table and fill ethnicity (or ethnotes) for at least 1 row of the borrowers table. 4/ Launch the updatedb entry and confirm you get a warning message and the DB structure is not updated. 5/ Make sure the ethnicity table is empty as well as the ethniciy and the ethnotes fields of borrowers, deletedborrowers and borrower_modifications 6/ Launch the updatedb entry and confirm you don't get the warning message and the DB structure is updated (ie. table and fields dropped).
Created attachment 37641 [details] [review] Bug 10020: Remove code related to ethnicity This is a(nother) vestige of Koha (2.2?). This patch removes unused code related to the 'ethnicity'. In detail: There is no way to fill the ethnicity table. There is no way to fill the borrowers.ethnicity and borrowers.ethnotes. BUT if borrowers.ethnicity exists, the value is displayed on members/moremember.pl (and only here). Test plan: Apply this patch and confirm there is no regression on adding/updating/deleting patrons. Note that you don't see the ethnicity value on the moremember.pl page even if a patron has it.
Test plan successful for first patch, when checking for regression I cannot delete a patron Software error: DBIx::Class::Storage::DBI::_dbh_execute(): Unknown column 'me.ethnicity' in 'field list' at /home/kidclamp/kohaclone/C4/Members.pm line 1864 For help, please send mail to the webmaster ([no address given]), giving this error message and the time and date of the error.
(In reply to Nick Clemens from comment #9) > Test plan successful for first patch, when checking for regression I cannot > delete a patron > > Software error: > > DBIx::Class::Storage::DBI::_dbh_execute(): Unknown column 'me.ethnicity' in > 'field list' at /home/kidclamp/kohaclone/C4/Members.pm line 1864 > For help, please send mail to the webmaster ([no address given]), giving > this error message and the time and date of the error. I have retested and it works. Did you update the schema (step 0 of first patch)? You need to edit the file and set $db_name, $db_user and $db_password
Created attachment 37984 [details] [review] [SIGNED OFF] Bug 10020: Remove DB table and fields related to ethnicity This patch remove the ethnicity table and the ethnicity and ethnotes fields from the 3 tables: borrowers, deletedborrowers and borrower_modifications IF no data exist. Test plan: 0/ Apply this patch and update the Koha::Schema (misc/devel/update_dbix_class_files.pl). 1/ Make sure the ethnicity has at least 1 entry 2/ Launch the updatedb entry and confirm you get a warning message and the DB structure is not updated. 3/ truncate the ethnicity table and fill ethnicity (or ethnotes) for at least 1 row of the borrowers table. 4/ Launch the updatedb entry and confirm you get a warning message and the DB structure is not updated. 5/ Make sure the ethnicity table is empty as well as the ethniciy and the ethnotes fields of borrowers, deletedborrowers and borrower_modifications 6/ Launch the updatedb entry and confirm you don't get the warning message and the DB structure is updated (ie. table and fields dropped). Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Created attachment 37985 [details] [review] [SIGNED OFF] Bug 10020: Remove code related to ethnicity This is a(nother) vestige of Koha (2.2?). This patch removes unused code related to the 'ethnicity'. In detail: There is no way to fill the ethnicity table. There is no way to fill the borrowers.ethnicity and borrowers.ethnotes. BUT if borrowers.ethnicity exists, the value is displayed on members/moremember.pl (and only here). Test plan: Apply this patch and confirm there is no regression on adding/updating/deleting patrons. Note that you don't see the ethnicity value on the moremember.pl page even if a patron has it. Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Created attachment 38400 [details] [review] [PASSED QA] Bug 10020: Remove DB table and fields related to ethnicity This patch remove the ethnicity table and the ethnicity and ethnotes fields from the 3 tables: borrowers, deletedborrowers and borrower_modifications IF no data exist. Test plan: 0/ Apply this patch and update the Koha::Schema (misc/devel/update_dbix_class_files.pl). 1/ Make sure the ethnicity has at least 1 entry 2/ Launch the updatedb entry and confirm you get a warning message and the DB structure is not updated. 3/ truncate the ethnicity table and fill ethnicity (or ethnotes) for at least 1 row of the borrowers table. 4/ Launch the updatedb entry and confirm you get a warning message and the DB structure is not updated. 5/ Make sure the ethnicity table is empty as well as the ethniciy and the ethnotes fields of borrowers, deletedborrowers and borrower_modifications 6/ Launch the updatedb entry and confirm you don't get the warning message and the DB structure is updated (ie. table and fields dropped). Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 38401 [details] [review] [PASSED QA] Bug 10020: Remove code related to ethnicity This is a(nother) vestige of Koha (2.2?). This patch removes unused code related to the 'ethnicity'. In detail: There is no way to fill the ethnicity table. There is no way to fill the borrowers.ethnicity and borrowers.ethnotes. BUT if borrowers.ethnicity exists, the value is displayed on members/moremember.pl (and only here). Test plan: Apply this patch and confirm there is no regression on adding/updating/deleting patrons. Note that you don't see the ethnicity value on the moremember.pl page even if a patron has it. Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
I agree with Galen here: I think we need to create extended patron attributes for storing this. It can be linked to an authorized value category or not, but I don't think we should leave cruft on the database (even with a warning message: the users will read it, but won't be capable of doing anything).
(In reply to Tomás Cohen Arazi from comment #15) > I agree with Galen here: I think we need to create extended patron > attributes for storing this. It can be linked to an authorized value > category or not, but I don't think we should leave cruft on the database > (even with a warning message: the users will read it, but won't be capable > of doing anything). IMO it does not worth spending time on this. The patch removes the tables/fields if data exist (which is certainly not the case), or leave them. How do you see the data migration? Just create 2 attributes "ethnicity" and "ethnotes"? I don't understand why we should need an authorized value here.
I agree with Jonathan, I think creating authorized values is not necessary. If we are going with extended patron attributes, then I'd just copy the values.
(In reply to Jonathan Druart from comment #16) > The patch removes the tables/fields if data exist (which is certainly not > the case), or leave them. Hum, trying again... The patch removes the tables/fields when they don't contain any data (which is certainly the case) or keep them if they are not empty.
(In reply to Jonathan Druart from comment #18) > (In reply to Jonathan Druart from comment #16) > > The patch removes the tables/fields if data exist (which is certainly not > > the case), or leave them. > > Hum, trying again... > The patch removes the tables/fields when they don't contain any data (which > is certainly the case) or keep them if they are not empty. And I think it's enough, don't you think? Back to the RM queue for reconsidering it.
Please, small rebase needed. Thanks!
Created attachment 43103 [details] [review] Bug 10020: Remove DB table and fields related to ethnicity This patch remove the ethnicity table and the ethnicity and ethnotes fields from the 3 tables: borrowers, deletedborrowers and borrower_modifications IF no data exist. Test plan: 0/ Apply this patch and update the Koha::Schema (misc/devel/update_dbix_class_files.pl). 1/ Make sure the ethnicity has at least 1 entry 2/ Launch the updatedb entry and confirm you get a warning message and the DB structure is not updated. 3/ truncate the ethnicity table and fill ethnicity (or ethnotes) for at least 1 row of the borrowers table. 4/ Launch the updatedb entry and confirm you get a warning message and the DB structure is not updated. 5/ Make sure the ethnicity table is empty as well as the ethniciy and the ethnotes fields of borrowers, deletedborrowers and borrower_modifications 6/ Launch the updatedb entry and confirm you don't get the warning message and the DB structure is updated (ie. table and fields dropped). Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 43104 [details] [review] Bug 10020: Remove code related to ethnicity This is a(nother) vestige of Koha (2.2?). This patch removes unused code related to the 'ethnicity'. In detail: There is no way to fill the ethnicity table. There is no way to fill the borrowers.ethnicity and borrowers.ethnotes. BUT if borrowers.ethnicity exists, the value is displayed on members/moremember.pl (and only here). Test plan: Apply this patch and confirm there is no regression on adding/updating/deleting patrons. Note that you don't see the ethnicity value on the moremember.pl page even if a patron has it. Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 43174 [details] [review] Bug 10020: (RM followup) DBIx update Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Patch pushed to master. Thanks Jonathan!
Created attachment 43384 [details] [review] Bug 10020: (RM followup) DBIx update / delete leftover file Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>