Sending this from my phone and being lazy, sorry for the top post. ;-) Ok.
- I agree that both types of checks are required.
- I think they should be two separate entry points in setup.pl just like upgrade is now.
- At the end of a migration (or upgrade for that matter) There should be a screen that explains that we recommend running the sanity checks and provide a button to do so on that screen. Regards David G
-------- Original message -------- From: Erik Huelsmann ehuels@gmail.com Date: 4/08/2017 22:49 (GMT+08:00) To: "Yves Lavoie, GaYLi" ylavoie@yveslavoie.com Cc: devel@lists.ledgersmb.org Subject: Re: [devel] Migration and Sanity checks
On Fri, Aug 4, 2017 at 2:04 PM, Yves Lavoie, GaYLi ylavoie@yveslavoie.com wrote:
Hi all, Until now, our migration process consisted of:
Using an existing LSMB 1.2 to 1.5 or Sql-Ledger 2.8 or 3.0 database Applying a specific migration script, with various fix possibilities using specific forms
Obtaining a dataset that can be pushed into the latest LedgerSMB schema or failing completely.
During the last week, Erik and I have been working on adding different edit, update, insert, cancel and override possibilities for the migration forms, in order to ease migration and try to prevent returning the user to his old application to fix data.Yes. And looking back, we seem to be adding some checks which could be classified as "sanity checks" rather than "migration checks". The difference being that "sanity checks" are not required to pass for technical reasons (i.e. the data is acceptable for the current 1.5 or 1.6 schema); however, the data doesn't make sense: e.g. reconciliation entries on a cost account. My definition of "migration checks" is that these are checks which are required to pass in order for the target schema to be able to accept the incoming data.
Yves's idea was to add these sanity checks during migration. My idea was that adding these sanity checks to the migration is the wrong place to add them: users who migrated before the checks were added might also have functionally problematic data, but can't find out, because they can't re-run the migration, if we put them *only* in the migration. Also, for people who don't really care about all this, they're required to completely clean up their data beyond what's necessary to migrate to LedgerSMB (making the migration experience itself more painful than necessary).
Now, some of those fixing forms are specific to each migration but some are more general and should probably fit better in a new sanitization step, to be inserted between steps 2 and 3 above. As discussed with Yves on the chat channel, this is the part where we don't exactly agree yet. We completely agree that it makes sense to have sanity checks. But the questions I have are:
1. Do we require people to go through the sanitizing experience of having to comply with both the migration checks as well as the sanity checks? 2. Even if we do, how do we make the sanity checks available to other users (such as those who migrated in the past)? 3. If we decide to break out the sanity checks into a function of their own, where do we put that function? (setup.pl? something elsewhere?) 4. If we put them in a function of their own, we obviously can't put them in Upgrade_Tests.pm, but their completely parallels the upgrade tests. In terms of code re-use, do we extract out a common abstract superclass which also holds the driver(utility) functions, making the setup.pl equivalents into simple "forwarders"?
This would
benefit all previous versions prevent entering bad data in the database for which we do or not have means to fixMy idea here is that it only has meaning to have sanity checks when we have feedback to provide to the user, that is, when we can offer a resolution strategy (preferably different from "contact the LedgerSMB mailing lists").
In my eyes "bad" data isn't as bad as it looks. In the cases we've run into so far, "bad data" simply meant "functionally meaningless". provide a diagnostic of the database to the owner. Such report could also be generated on demand through the admin application. allow for more complex checks than was is strictly required by migration.And - more importantly - allow for much more complex "fix-up" operations than originally intended.
This is a change from the actual process for which your input would be welcomed.Yup. Also see my questions and comments above.