impactoss / impactoss-server

IMPACT OSS - server-side application & API
https://demo.impactoss.org
MIT License
3 stars 8 forks source link

Make various reference and title fields mandatory #412

Closed lukearndt closed 2 months ago

lukearndt commented 2 months ago

Context:

https://github.com/impactoss/impactoss-server/issues/403

Changes:

This commit makes the following fields mandatory (table: field):

measures: reference
indicators: reference
pages: title
categories: title
progress_reports: title

It also updates the model validations and specs accordingly.

Considerations:

The data migration sets any currently null values to "" rather than applying a more nuanced strategy. I'd like to confirm that this is okay before we go ahead.

tmfrnz commented 2 months ago

The data migration sets any currently null values to "" rather than applying a more nuanced strategy. I'd like to confirm that this is okay before we go ahead.

great point! can we alternatively store random (unique) values on migration to ensure validity?

lukearndt commented 2 months ago

The data migration sets any currently null values to "" rather than applying a more nuanced strategy. I'd like to confirm that this is okay before we go ahead.

great point! can we alternatively store random (unique) values on migration to ensure validity?

The empty string values will technically be valid, but we can store some other values if we want.

What pattern do you want to use to generate them? It's worth noting that I'm not 100% clear on what these fields are actually for, so I wouldn't want to guess at the data they should contain.

tmfrnz commented 2 months ago

thanks @lukearndt, let's go with the empty strings for now (although in the future we may want to consider requiring actual values for these mandatory fields)