sfbrigade / bats-server

Routed is an app to help ambulances direct non-critical patients to hospital emergency rooms with the most availability.
https://routedapp.org/
GNU Affero General Public License v3.0
18 stars 11 forks source link

Pelumi/feat/add mfa to data model #294

Closed olupelumi closed 1 year ago

olupelumi commented 1 year ago

For Sutter Health, we're looking to add the ability for an admin to enable two-factor authentication. The first step here (and in this PR) is to create an 'isMfaEnabled' column in the organization model/table that'll act as an indicator for knowing if multifactor authentication should be activated on the server side.

Also included is an addition of the shared folder to yarn workspaces in the top level package.json

How I tested Tested that:

francisli commented 1 year ago

So, the errors I see in the CircleCI log and when I run locally are:

column "ismfaenabled" does not exist

Our database schema was originally separately designed by a data model specialist BEFORE the server codebase was even started, and he had a particular naming convention/style to his schema where, if you inspect the rest of the schema, all column names are lowercased.

So, in shared/FieldMetadata.js, you'll see that the field name (i.e. 'isMfaEnabled') is being lowercased (i.e. into 'ismfaenabled') by default.

For consistency with the rest of the db schema, I'd recommend undoing your migration, editing the migration file so that the column name is all lowercase, and running the migration again.

fwextensions commented 1 year ago

Just a note on FieldMetadata: it defaults to lowercasing the name to get the colName, but that can be overridden in the metadata, and many of the Organization fields have different column names. I don't know what the history there is, but wanted to capture it in the metadata framework.

I think, ideally, there'd be an easy way to use the metadata to do a migration, so we're not duplicating the column names, types, etc. in multiple places, and therefore avoiding these bugs. But there isn't one currently.