Frames used in migrations

[alxndrsn]

"frames" reflect the current data model, which can change, whereas database migrations operate on historic models which should be fixed to the specific migration.

Observed

$ git grep frames -- lib/model/migrations/
lib/model/migrations/20200220-01-repair-submission-parsing.js:const { Submission } = require('../frames');
lib/model/migrations/20210120-01-instance-names.js:const { Submission } = require('../frames');
lib/model/migrations/20211008-01-track-select-many-options.js:const { Form } = require('../frames');

Expected

Migrations should not rely on application code for their understanding of the database schema.

[matthew-white]

IIRC we've had a previous round of removing frames from migrations. I think most of the ones that remain are there for XML-parsing, e.g., Submission.fromXml().

[alxndrsn]

👍

Could construct(Form.Field) in lib/model/migrations/20211008-01-track-select-many-options.js still be an issue?

central-backend/lib/model/migrations/20211008-01-track-select-many-options.js

Lines 57 to 61 in 7a1e080

	const allFields = await db.select('formId', 'path')
	.from('form_fields')
	.where({ selectMultiple: true })
	.groupBy('formId', 'path')
	.then(map(construct(Form.Field)));

[alxndrsn]

It looks like Submission.fromXml() is still creating a Submission frame internally, so presumably could still present issues in the future.

[matthew-white]

Could construct(Form.Field) in lib/model/migrations/20211008-01-track-select-many-options.js still be an issue?

I think this is similar to Submission.fromXml() in that Form.Field is used for purposes of XML-parsing. The fields are passed to getSelectMultipleResponses(), which expects an array of frames.

It looks like Submission.fromXml() is still creating a Submission frame internally, so presumably could still present issues in the future.

Conceivably! Though it's a pretty simple use of the frame. Would you want to refactor the XML-parsing so that it doesn't touch frames at all?

[alxndrsn]

Would you want to refactor the XML-parsing so that it doesn't touch frames at all?

Yes, I think the ideal is that database migrations are independent of application code, because there is no guarantee which version of the application a particular migration is run with/by. This is especially risky for the application's code defining database structure (in this case frames).

[matthew-white]

I'm still not sure how much risk there is in this case in practice, but I agree that what you describe would be ideal. And actually, looking at Submission.fromXml(), it does seem very possible to separate the XML-parsing from the frame itself. Submission.fromXml() only assembles a Submission.Partial at the very end, so I could imagine a separate function to do the parsing (which the migration could use), then Submission.fromXml() would just call that function and do its assembling. There are other cases where we have XML-parsing that's completely separate from frames, e.g., getDataset().

The Form.Field frame looks very simple, so I'm not even sure why it's needed in the migration. Maybe because something is calling the frame's isStructural() method? That's probably not essential though / there's got to be an alternative.