|
Description
Tomás Cohen Arazi (tcohen)
2017-02-27 12:39:42 UTC
address2, address3 and address4 are used if they exist on the DB, by the template. They are concatenated using \n and splitted back on save... Created attachment 100861 [details] [review] Bug 18177: remove aqbooksellers.othersupplier This patchset removes some columns from the aqbooksellers table that are not used: * booksellerfax * booksellerurl * bookselleremail * currency The first 3 ones are certainly leftover from bug 10402. Test plan: For each for these fields you will make sure that they cannot be edited from the interface. Created attachment 100862 [details] [review] Bug 18177: remove aqbooksellers.booksellerfax Created attachment 100863 [details] [review] Bug 18177: remove aqbooksellers.booksellerurl Created attachment 100864 [details] [review] Bug 18177: remove aqbooksellers.bookselleremail Created attachment 100865 [details] [review] Bug 18177: remove aqbooksellers.currency Created attachment 100866 [details] [review] Bug 18177: DB changes Created attachment 100867 [details] [review] Bug 18177: DBIC changes Created attachment 100868 [details] [review] Bug 18177: remove aqbooksellers.othersupplier This patchset removes some columns from the aqbooksellers table that are not used: * booksellerfax * booksellerurl * bookselleremail * othersupplier * currency The first 3 ones are certainly leftover from bug 10402. Test plan: For each for these fields you will make sure that they cannot be edited from the interface. Created attachment 100869 [details] [review] Bug 18177: remove aqbooksellers.booksellerfax Created attachment 100870 [details] [review] Bug 18177: remove aqbooksellers.booksellerurl Created attachment 100871 [details] [review] Bug 18177: remove aqbooksellers.bookselleremail Created attachment 100872 [details] [review] Bug 18177: remove aqbooksellers.currency Created attachment 100873 [details] [review] Bug 18177: DB changes Created attachment 100874 [details] [review] Bug 18177: DBIC changes The 4 fields address[1-4] are built from the "Physical address" input. However, if the address entered has more than 4 lines, the following ones are dropped. That is for a separate bug report. When we migrated from your previous ILS (Virtua), I kept some vendor contact information in booksellerfax, bookselleremail and currency. This means that we are will lose this information when you drop them from DB. Created attachment 101612 [details] [review] Bug 18177: Do not remove columns if at least one value exists (In reply to Frank Hansen from comment #17) > When we migrated from your previous ILS (Virtua), I kept some vendor contact > information in booksellerfax, bookselleremail and currency. This means that > we are will lose this information when you drop them from DB. Thanks Frank for this feedback? What about this last patch? We are going to remove the column only if there is no data, otherwise a warning will be displayed. (In reply to Frank Hansen from comment #17) > When we migrated from your previous ILS (Virtua), I kept some vendor contact > information in booksellerfax, bookselleremail and currency. This means that > we are will lose this information when you drop them from DB. Thanks Frank for this feedback! What about this last patch? We are going to remove the column only if there is no data, otherwise a warning will be displayed. (In reply to Jonathan Druart from comment #20) > (In reply to Frank Hansen from comment #17) > > When we migrated from your previous ILS (Virtua), I kept some vendor contact > > information in booksellerfax, bookselleremail and currency. This means that > > we are will lose this information when you drop them from DB. > > Thanks Frank for this feedback! > > What about this last patch? We are going to remove the column only if there > is no data, otherwise a warning will be displayed. Maybe we can give some advise for migrating that data? (In reply to Jonathan Druart from comment #20) > (In reply to Frank Hansen from comment #17) > > When we migrated from your previous ILS (Virtua), I kept some vendor contact > > information in booksellerfax, bookselleremail and currency. This means that > > we are will lose this information when you drop them from DB. > > Thanks Frank for this feedback! > > What about this last patch? We are going to remove the column only if there > is no data, otherwise a warning will be displayed. Yes, this will at least keep the data in those columns so they're not getting lost. But is the plan to remove those fields completely from UI as well? If that is the case, we have no use for them. We can probably live without those columns anyway. Unused should not be in the GUI - if they are, we might need to keep some! They are not displayed (view or edit) anywhere. (In reply to Jonathan Druart from comment #24) > They are not displayed (view or edit) anywhere. When I dig a little deeper into how we migrated, it turned out that we copied the same value to multiple columns. For example, currency is the same as invoiceprice; booksellerfax is the same as fax; and bookselleremail was also inserted in aqcontacts.email. So this means that we will not lose any data at all. The columns othersupplier and booksellerurl is not used here at Lund University. Created attachment 101687 [details] [review] Bug 18177: remove aqbooksellers.othersupplier This patchset removes some columns from the aqbooksellers table that are not used: * booksellerfax * booksellerurl * bookselleremail * othersupplier * currency The first 3 ones are certainly leftover from bug 10402. Test plan: For each for these fields you will make sure that they cannot be edited from the interface. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 101688 [details] [review] Bug 18177: remove aqbooksellers.booksellerfax Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 101689 [details] [review] Bug 18177: remove aqbooksellers.booksellerurl Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 101690 [details] [review] Bug 18177: remove aqbooksellers.bookselleremail Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 101691 [details] [review] Bug 18177: remove aqbooksellers.currency Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 101692 [details] [review] Bug 18177: DB changes Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 101693 [details] [review] Bug 18177: DBIC changes Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 101694 [details] [review] Bug 18177: Do not remove columns if at least one value exists Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Looks like I overlapped with incoming comments.. really pleased to see this cleanup being done. I've signed off as the patches all do as intended and I'm happy to agree that these fields have not been used in the codebase for some time. The final patch is nice to have in cases where people have migrated data as a 'just in case'.. Signing off. QAing Created attachment 101958 [details] [review] Bug 18177: remove aqbooksellers.othersupplier This patchset removes some columns from the aqbooksellers table that are not used: * booksellerfax * booksellerurl * bookselleremail * othersupplier * currency The first 3 ones are certainly leftover from bug 10402. Test plan: For each for these fields you will make sure that they cannot be edited from the interface. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 101959 [details] [review] Bug 18177: remove aqbooksellers.booksellerfax Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 101960 [details] [review] Bug 18177: remove aqbooksellers.booksellerurl Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 101961 [details] [review] Bug 18177: remove aqbooksellers.bookselleremail Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 101962 [details] [review] Bug 18177: remove aqbooksellers.currency Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 101963 [details] [review] Bug 18177: DB changes Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 101964 [details] [review] Bug 18177: DBIC changes Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 101965 [details] [review] Bug 18177: Do not remove columns if at least one value exists Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> + warn "Warning - Cannot remove column aqbooksellers.$column. At least one value exists"; Does this warn find its way to the interface. Probably not. Print it ? I will pass QA. Feel free to add follow-up. @RM: Watch for DBIx changes (In reply to Marcel de Rooy from comment #44) > + warn "Warning - Cannot remove column aqbooksellers.$column. > At least one value exists"; > > > Does this warn find its way to the interface. Probably not. Print it ? > I will pass QA. Feel free to add follow-up. > > @RM: Watch for DBIx changes warnings should be seen from the interface, maybe at the bottom in the red part? They will indeed be displayed at the bottom of the page in red when I tested :) Nice work everyone! Pushed to master for 20.05 Created attachment 102011 [details] [review] Bug 18177: (follow-up) remove aqbooksellers.currency There was an occurrence in tests enhancement not backported to 19.11.x |