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

Rsdev 326 backend time validation #7

Merged

Conversation

rs-fraser
Copy link
Contributor

@rs-fraser rs-fraser commented Aug 16, 2024

This PR adds a new subclass of ExtraField so that we can apply validation specific to time formats. Previously any time fields were treated purely as text and allowed any values.

related rspace-web PR: rspace-os/rspace-web#88

@mKowalski256
Copy link
Contributor

Hmm, what this PR is doing it's actually adding a new type of ExtraField, while I think the goal of the RSDEV-326 ticket was to add validation to existing InventoryTimeField and InventoryDateField which are extensions of SampleField.

ExtraFields (named 'Custom Fields' in the UI) are a functionality where to any individual inventory item (sample/subsample/container) user can add any number of additional fields to store arbitrary more data. Extra fields (custom fields) have just two types: TEXT and NUMBER, and while user can store data/time in ExtraTextField, there is no validation needed.

Extra Fields / Custom Fields in the UI:
image

Now SampleFields, even though I see are named 'Custom Fields' in the UI, are something specific to Samples and Sample Templates. When creating a Sample Template user can say this template will have a number of Custom Fields that are for specific data type: choice/date/number/time/uri/text/etc. These types are only available for selection when editing a Sample Template, and are only available for filling the data when using the Sample that is based on Sample Template.

Sample Fields / Custom Fields in the UI:
image

I hope that's clearer now, the validation code and test from this PR is probably reusable, but should end up in/around the existing InventoryTimeField class, rather than in new subclass of ExtraField.

Copy link
Contributor

@mKowalski256 mKowalski256 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, adding a time field to ComplexSampleTemplate is a good improvement, and I can no longer use API to create sample with incorrect date/time fields:

image

@rs-fraser rs-fraser merged commit b615f25 into rspace-os:main Aug 27, 2024
@rs-fraser rs-fraser deleted the rsdev-326-backend-time-validation branch August 27, 2024 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants