Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EDTF fields are not validated in migration #86

Open
bseeger opened this issue Mar 4, 2021 · 7 comments
Open

EDTF fields are not validated in migration #86

bseeger opened this issue Mar 4, 2021 · 7 comments

Comments

@bseeger
Copy link

bseeger commented Mar 4, 2021

EDTF fields have no content validated when they are migrated. If an invalid EDTF value slips in the system via migration, the system will throw an error later on when someone views the record.

There is, too, the bigger question of: what, if any, content validation happens during migration and what could/should we do for validation during migration?

@bseeger
Copy link
Author

bseeger commented Mar 18, 2021

Here's an example of some work that was done by the Islandora community to validate EDTF dates.

Islandora/migrate_islandora_csv#41

This combined with turning on the validate flag in our yml files might cover most validation needs.

@birkland
Copy link

birkland commented Jun 9, 2021

It's possible this may be solved merely by a config change (enabling validation).

@jordandukart
Copy link

jordandukart commented Jun 16, 2021

Just some general notes / thoughts:

  • Islandora's EDTF field definition doesn't provide a constraint plugin and relies on widget validation currently. In terms of the "correct" approach adding a constraint to the field is probably the way to go. With that in mind, I'm not sure why that field never had one defined to begin with.
  • The migrate plugin mentioned above will perform validation that the widget does via the UI albeit it's not necessarily fully complete EDTF validation to specification. If the intent was to mirror what the field accepts via the UI leveraging this plugin would be sufficient.
  • However, there is an ongoing effort to potentially refine the above but don't believe there is an official timeline for completion. The meta issue may provide some more insight. If this further work comes to fruition and the overall invocation does not change this plugin would not need to be updated to take advantage of it.

In terms of solutions and their timelines there would be two options (imo):

  1. Leveraging the plugin would be as trivial as adding the below to any migration utilizing an EDTF field:
- plugin: valid_edtf
   sets: {true|false depending on the field config}
   intervals: {true|false depending on the field config}
  1. In the event the constraint work should be undertook the actual constraint plugin would likely take only a few hours. However, potential time it may take to have it merged into Islandora core would be a bit of question mark as well some further research into why the constraint approach wasn't undertook in the original implementation. If this path was chosen leveraging validate: true for the entity should cause this validation to occur. Would also likely look out how to roll out an update to existing fields.

@birkland
Copy link

Blocked awaiting advice on required validation for launch

@birkland
Copy link

birkland commented Aug 3, 2021

Follow-up: can we confirm from Katie/michelle/stakeholders that we want the plugin enabled, owing to a lack of a constraint?

@birkland birkland removed the vendor label Aug 3, 2021
@birkland
Copy link

birkland commented Aug 3, 2021

Michelle: Yes, we need this plugin enabled

@birkland
Copy link

@mjanowiecki will investigate whether this needs to be implemented for launch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants