Description
Katrin Fischer
2013-04-10 08:34:09 UTC
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 +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> |