Generate patient fields from metadata shared between server and client

[fwextensions]

• src/metadata/patient.js contains a more compact specification of the fields in the patient model. Values that can be generated, like the colName coming from the name.toLowerCase(), are removed. The list of fields that appear as parameters is generated programmatically.
• This metadata is used in Ringdown.js to attach getters and setters for each patient field in the payload. It's also used to generate the React propTypes.
• The shared files are under src/ because CRA won't allow imports outside of that path. They're in CJS format so they can be easily required in Node, and webpack will convert them to modules for the client.
• Client- or server-specific modules, like PropTypes or sequelize.DataTypes, aren't imported into the metadata so it can be shared more easily between the two packages.

@francisli would love to get your thoughts on whether this seems like a productive direction. The names and types of data fields aren't getting repeated so many times, and using the metadata to generate things like getters/setters and prop-types saves ~225 LOC in Ringdown.js.

In PatientFields.js, the metadata is used to generate the text area, numeric input and checkbox fields (doesn't handle comboboxes or radio buttons yet, but it could). Lots of repeated props are avoided by sharing the ringdown data and onChange handler via a context, and then getting names, labels, units, etc. from the metadata. A downside may be that those strings aren't directly visible in the form layout code, but the upside is much less verbose code and more consistent labels for things when the same data is used in multiple places.

I'm not wedded to the names of things or the file structure. "Metadata" is pretty generic and overused, but "model" already has a specific meaning, so open to suggestions. Maybe MetaModel and MetaForm?

[fwextensions]

@francisli any thoughts on this? I know there are a bunch of changes, but it would be helpful to see if this seems like a reasonable approach.

[francisli]

I think this looks like good approach at a first glance and I will spend some more time with this branch next, I wanted to get some of the smaller quick PRs merged in first so that they weren't hanging around too long...

[francisli]

@fwextensions so, I guess we'll want to finish re-writing all the models into metadata, so that we don't have two different ways of defining a model...? I can help with that if so...

Do you think it is worth it to see if we can tweak things so that the constants and metadata directories can live outside of the React source? It definitely fields weird to me, and will be even weirder after we proceed with the client/server directory refactor in the other PR. There are ways to tweak or completely remove the restriction without ejecting it from CRA here:

https://stackoverflow.com/questions/44114436/the-create-react-app-imports-restriction-outside-of-src-directory

[fwextensions]

Yeah, I'm about half way through converting the models. Will finish it up and remove the draft label.

Importing from outside src will require adding something like craco as well as another lib. I agree it's somewhat weird, but I'd like to get things working with the current paths and then look at adding other solutions. Or just eject. I've never really used CRA so I'm not sure how much we benefit from keeping it.

[fwextensions]

@francisli let me know if you want me to squash all these commits.

I moved the new files into /src/shared so they're at least all in one place and somewhat separate from the client source. We can look at moving those higher up as part of the file reorg.

[francisli]

@fwextensions no need to squash or rebase, that will happen at merge time...! I'll start looking this over tonight...

[francisli]

@fwextensions one place where I think I'll make a change/suggestion is with the db migration file. DB migration files are timestamped and should reflect always the changes made to the database at that point in time. But, right now, that will change if the definition of those columns change later in the metadata. Once a db migration is deployed, it should never be changed, because then it no longer reflects what the actual status of the db is based on when the migration was originally run (if the columns change definition later, then a new migration needs to be made). So, I think the migration will remain the one place where we continue to just manually write the changes directly into the file, not connected to the metadata. Does that make sense...? Can discuss on the call if you like...

[fwextensions]

I'd forgotten about the migration file. Think it's still worth looping over the field metadata to avoid all that duplication. But could just copy and paste the new fields into a literal in the migration file instead of calling getFieldHash(). That way the definition won't change. Can't make the call tonight...

[francisli]

I'd forgotten about the migration file. Think it's still worth looping over the field metadata to avoid all that duplication. But could just copy and paste the new fields into a literal in the migration file instead of calling getFieldHash(). That way the definition won't change. Can't make the call tonight...

Hmm, let me think on that and see... I think there's something to a migration file that is fully self-contained and easily readable at a glance to know exactly the structure/schema of the db. Also, being dependent upon a another layer of translation logic (i.e. from our abstract field definition to Sequelize) makes it susceptible to being changed again if that logic gets tweaked later...

[fwextensions]

What I was thinking of was something like the below, with the metadata copied and pasted. But I guess there is still a needed translation to the sequelize format. Not the end of the world to write it manually, though wouldn't be hard to spit the migration file out programmatically...

const newFields = [
  {
    name: 'glasgowComaScale',
    type: 'integer',
    label: 'GCS',
    unit: '/ 15',
    range: { min: 3, max: 15 },
  },
  {
    name: 'otherObservationNotes',
    type: 'text',
    label: 'Other',
  },
];

module.exports = {
  async up(queryInterface, Sequelize) {
    await queryInterface.sequelize.transaction(async (transaction) => {
      for (const newField of newFields) {
        const { field, type } = convertToSequelizeField(newField)

        await queryInterface.addColumn(tableName, field, type, { transaction });
      }
    });
  },
...

[francisli]

I think creating a script that would could be run to generate the migration script is the way to go, let's put that on the list for the next time we have data model changes to make...

Otherwise, I think this looks good to go...

[fwextensions]

Thanks for plowing through that!