dev/core#2846 Towards validation of start and end dates. Forms.

[JKingsnorth]

Please see https://lab.civicrm.org/dev/core/-/issues/2846#note_65250

First step towards improving validation of entities with 'start' and 'end' dates, ensuring they are consistent. And applying consistent checking location. First step is applying form-level validation.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[JKingsnorth]

Test failures are related; and I think I should be adding extra tests to the affected forms to cover the validation.

It would be good to get a 'concept approved' on this via https://lab.civicrm.org/dev/core/-/issues/2846 before I proceed though.

[JKingsnorth]

Tests added, this is ready for review. @eileenmcnaughton , I know how much you like PRs that add tests. (And I'm very happy to take feedback on the approaches used, if you think I can improve =] )

[eileenmcnaughton]

Nice - I've given it the all-important pink label so that will stay on my view of the PR queue

[BettyDolfing]

test this please

[JKingsnorth]

Jenkins test this please

[jaapjansma]

@BettyDolfing and I are reviewing PR's and we tried to reproduce the issue. And we found that the issues where too different from each other so we suggested to create separate issues for each specific use case, and separate PR's for them. See https://lab.civicrm.org/dev/core/-/issues/2846#note_68143

@JKingsnorth are you still willing to work on this issue?

On a side note we also saw a lot of warnings in the test environment of this PR. See screenshot below.

[JKingsnorth]

@jaapjansma / @BettyDolfing Thanks for taking a look.

I agree that trying to tackle the form, APIs, BAO layers should all be done in separate PRs.

But this PR I think is a 'complete' solution for improving validation and error messages in the form layer, ie: when using the front end form features.

This PR does not aim to resolve it in all places, and the actual code is fairly minimal (a lot of the lines changes are due to tests which have been added).

I do not think those warnings are related to this merge request, because I do not touch the .hlp.php files in this merge request.

So I still think this PR is ready, and is a first step towards https://lab.civicrm.org/dev/core/-/issues/2846

[BettyDolfing]

Test this please

[jaapjansma]

@JKingsnorth thanks for your response. @BettyDolfing and I are not able to review this PR as we don't feel comfortable in testing it because we think this PR tries to solve too many different issues (although they are related). Maybe someone else is able to review this.

[mattwire]

@JKingsnorth needs rebase

[eileenmcnaughton]

@JKingsnorth I'm sorry I've only just looked at this - I agree it improves the date validation - making it impossible to have the start date before the end date in more places & requiring date be set if time is - and the tests are yay yay yay

[JKingsnorth]

Thanks very much for taking the time to look at this @eileenmcnaughton !

[eileenmcnaughton]

@JKingsnorth it's a good change