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

Implement flexible format validators #426

Merged

Conversation

open-dynaMIX
Copy link
Member

This commit adds flexible format validators.

There is one new query: all_format_validators that enables querying
all available format validators.

A text- or textarea-question can have one or multiple format_validators
assigned.

When querying a question, there is a new FormatValidatorsConnection,
containing all FormatValidator-objects assigned on the question.

For this there is a new QuestionValidator that ensures, referenced
format_validators exist.

translate_value() has been moved to core.utils, as this is used in the
data_source- and form-app.

There is a new extension-point format_validators.py.

When adding the tests for this, it became obvious, that there was something
wrong with the parameters for DataSource. As I was touching those tests
anyway, I fixed this as well.

Added commented out extension volume to docker-compose.yml. Also added
forgotten data_sources extension volume.

Additionally this commit also cleans up the snapshots for document and
question as there were quite a lot old unused snapshots.

Closes #360

@open-dynaMIX
Copy link
Member Author

@czosel @winged What do you think?

Let's agree on the design, before I go ahead and implement the other base FormatValidators.

@open-dynaMIX open-dynaMIX force-pushed the add_flexible_format_validators branch 2 times, most recently from a1ead19 to 9660e82 Compare May 6, 2019 13:32
Copy link
Contributor

@winged winged left a comment

Choose a reason for hiding this comment

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

I really like the declarative syntax. From my POV, go ahead with the rest of the implementation for the remaining types.

Minor nitpick for the docs: I'd add a comment noting that it's OK to overwrite the validate() method for more complex validations, and what you can do with it. (Along with the note to have a corresponding implementation in the frontend as well, of course)

@czosel
Copy link
Contributor

czosel commented May 6, 2019

The API looks great to me as well! 👍

@open-dynaMIX open-dynaMIX force-pushed the add_flexible_format_validators branch from 9660e82 to 533d02a Compare May 8, 2019 05:44
This commit adds flexible format validators.

There is one new query: `all_format_validators` that enables querying
all available format validators.

A text- or textarea-question can have one or multiple format_validators
assigned.

When querying a question, there is a new `FormatValidatorsConnection`,
containing all FormatValidator-objects assigned on the question.

For this there is a new `QuestionValidator` that ensures, referenced
format_validators exist.

`translate_value()` has been moved to `core.utils`, as this is used in the
`data_source`- and `form`-app.

There is a new extension-point `format_validators.py`.

When adding the tests for this, it became obvious, that there was something
wrong with the parameters for `DataSource`. As I was touching those tests
anyway, I fixed this as well.

Added commented out extension volume to docker-compose.yml. Also added
forgotten data_sources extension volume.

Additionally this commit also cleans up the snapshots for document and
question as there were quite a lot old unused snapshots.

Closes projectcaluma#360
@open-dynaMIX open-dynaMIX force-pushed the add_flexible_format_validators branch from 533d02a to 8011518 Compare May 8, 2019 06:55
@open-dynaMIX open-dynaMIX requested review from winged and czosel May 8, 2019 07:00
@open-dynaMIX
Copy link
Member Author

I've added the info about overriding validate() to the README and implemented the other base validator (there was just the phone-number left :)).

Copy link
Contributor

@winged winged left a comment

Choose a reason for hiding this comment

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

Nice work, looks good!

@winged winged merged commit 419391b into projectcaluma:master May 8, 2019
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.

Flexible format validations
3 participants