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

Rely on fields when validating submitted values on sample creation #2307

Merged
merged 12 commits into from
May 11, 2023

Conversation

xispa
Copy link
Member

@xispa xispa commented May 9, 2023

Description of the issue/feature this PR addresses

This Pull Request ensures that values submitted in sample add form are validated in accordance with the field types and the validators assigned.

Although the Date/Time widget does not display future dates when the widget is configured with the option no_future, user can easily bypass this by typing a future date manually in the input field:

Captura de 2023-05-09 14-44-18

This Pull Request guarantees that even if the date is typed manually, the entered value is consistent with the widget configuration.

Current behavior before PR

User can manually enter future dates for e.g. DateSampled in Add sample form

Desired behavior after PR is merged

System complains if the date entered is not valid

--
I confirm I have tested this PR thoroughly and coded it according to PEP8
and Plone's Python styleguide standards.

@xispa xispa changed the title Validate non-future and non-past dates on add sample submit Validate non-future and non-past dates on add sample May 9, 2023
return datetime.now()
two_months = getattr(widget, "datepicker_2months", False)
if self.is_true(two_months):
return datetime.now() + relativedelta(months=2)
Copy link
Contributor

Choose a reason for hiding this comment

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

Where do we have this in place? Not sure if this is possible at all with the current native HTML 5 date widget.

Copy link
Member Author

Choose a reason for hiding this comment

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

Was present in 1.3.x, not in 2.x. Just removed this with 9823c4c

@ramonski
Copy link
Contributor

ramonski commented May 9, 2023

Wouldn't it make more sense to call a field validator for these date fields and move the logic in there?

The reason is that field validators should be called from the JSON API as well:
https://github.com/senaite/senaite.jsonapi/blob/2.x/src/senaite/jsonapi/fieldmanagers.py#L107

@xispa
Copy link
Member Author

xispa commented May 9, 2023

Wouldn't it make more sense to call a field validator for these date fields and move the logic in there?

The reason is that field validators should be called from the JSON API as well: https://github.com/senaite/senaite.jsonapi/blob/2.x/src/senaite/jsonapi/fieldmanagers.py#L107

Probably yes. Let me try and see

@xispa xispa changed the title Validate non-future and non-past dates on add sample Verify submitted values by relying on field validators on sample creation May 10, 2023
@xispa xispa changed the title Verify submitted values by relying on field validators on sample creation Rely on fields when validating submitted values on sample creation May 10, 2023
return ansi
return "{}{:02d}{:02d}{:02d}".format(ansi, dt.hour, dt.minute, dt.second)


Copy link
Contributor

Choose a reason for hiding this comment

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

Better use strftime from the datetime module:

In [1]: from datetime import datetime

In [2]: now = datetime.now()

In [3]: now.strftime("%Y%m%d%H%M")
Out[3]: '202305110732'

# is offset-aware. We need to add the TZ, otherwise we get a:
# TypeError: can't compare offset-naive and offset-aware datetimes
if dtime.to_ansi(value) >= dtime.to_ansi(self.min):
return None
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, ANSI times can be compared as strings, then it's fine like this.

datetime.datetime(1, 1, 1, 0, 0)

>>> dtime.to_ansi(min_date)
'00010101000000'
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, got it. Therefore you format the date manually:

In [9]: dt = datetime.strptime("1000-01-01", "%Y-%m-%d")

In [10]: dt.strftime("%Y%m%d%H%M")
ValueError: year=1000 is before 1900; the datetime strftime() methods require year >= 1900

@ramonski ramonski merged commit 9a1936a into 2.x May 11, 2023
@ramonski ramonski deleted the validate-past-future branch May 11, 2023 05:58
ramonski pushed a commit that referenced this pull request May 17, 2023
…#2308)

* Validate no-future dates when submitting Add sample

* Validate both past and future dates

* Include 2.x's `datetime_ng` (just in case)

* Changelog

* Use field validator instead

* Return immediately if we have no value and the field is not required
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants