Skip to content

7993/fix end event date required with open end option checked#7995

Open
TimoBroeskamp wants to merge 4 commits intomainfrom
7993/fixEndEventDateWithOpenEnd
Open

7993/fix end event date required with open end option checked#7995
TimoBroeskamp wants to merge 4 commits intomainfrom
7993/fixEndEventDateWithOpenEnd

Conversation

@TimoBroeskamp
Copy link

@TimoBroeskamp TimoBroeskamp commented Mar 12, 2026

Caution

The Volto Team has suspended its review of new pull requests from first-time contributors until the release of Plone 7, which is preliminarily scheduled for the second quarter of 2026.
Read details.



If your pull request closes an open issue, include the exact text below, immediately followed by the issue number. When your pull request gets merged, then that issue will close automatically.

Is partly closing #7993


If your pull request includes changes to the documentation—either in narrative documentation, Storybook, or configuration—then a pull request preview will be generated and a link will populate in the description of your pull request below.
By clicking that link, you can use the visual diff menu in the upper right corner to navigate to pages that have changes, then display the diff by checking the Show diff checkbox.

@boring-cyborg
Copy link

boring-cyborg bot commented Mar 12, 2026

Caution

The Volto Team has suspended its review of new pull requests from first-time contributors until the release of Plone 7, which is preliminarily scheduled for the second quarter of 2026.
Read details.

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, read
Plone's Code of Conduct,
Contributing to Plone,
First-time contributors, and
Contributing to Volto,
as this will greatly help the review process.

Welcome to the Plone community! 🎉

@TimoBroeskamp TimoBroeskamp marked this pull request as draft March 12, 2026 13:38
@stevepiercy
Copy link
Collaborator

Please edit the description to include closes #7995 so that when this is merged, GitHub will automatically close the issue.

Also please add a change log entry per https://6.docs.plone.org/contributing/index.html#contributing-change-log-label

@TimoBroeskamp TimoBroeskamp marked this pull request as ready for review March 12, 2026 14:54
@jackahl
Copy link
Member

jackahl commented Mar 12, 2026

Please edit the description to include closes #7995 so that when this is merged, GitHub will automatically close the issue.

Also please add a change log entry per https://6.docs.plone.org/contributing/index.html#contributing-change-log-label

@stevepiercy I think this will most likely not fix #7993 as there are also backend changes to be made as noted in plone/plone.app.event#97

@TimoBroeskamp
Copy link
Author

@stevepiercy @jackahl Okay now it's updated. When checking "open end" the formData.end is set to formData.start and the validation accepts that only when "open end" is checked too so otherwise you still need the end date to be at a later point.

@jackahl
Copy link
Member

jackahl commented Mar 13, 2026

@TimoBroeskamp this should work. But I just had another thought: This might become annoying if someone checks the box and then decides to use an end-date anyway. To avoid this we could make the change ob the request only, but not the actual formdata. Though I am unsure, if we have a good place to apply such transformations...

@sneridagh @pnicolli is there a place where we can apply transformations on the formdata, before it is send? Or is this a bad idea in general, as then the database contents would differ from the formdata

@stevepiercy
Copy link
Collaborator

Sorry, I misinterpreted the icalendar standard, per a further discussion at collective/icalendar#1275 (comment). Please see my revised comment at #7993 (comment).

This might become annoying if someone checks the box and then decides to use an end-date anyway.

@jackahl I don't understand why that's annoying. I can check the Open End box, then uncheck it and add an Event Ends date or datetime. Can you elaborate?

Or is this a bad idea in general, as then the database contents would differ from the formdata

I'd transform the data in the client form before submittal and validate it both on the frontend and backend. Transformation on the backend has a funky smell to me.

Copy link
Member

@gyst gyst left a comment

Choose a reason for hiding this comment

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

Some pointers about making this consistent with backend.

}
// If open_end is checked set formData.end to formData.start
if (formData?.open_end === true) {
formData.end = formData.start;
Copy link
Member

Choose a reason for hiding this comment

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

const isValid =
value && formData.end && new Date(value) < new Date(formData.end);
(value && formData.end && new Date(value) < new Date(formData.end)) ||
(formData.open_end && formData.start === formData.end);
Copy link
Member

Choose a reason for hiding this comment

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

This breaks with the backend defaults, see above. Better is to require that start <= end in case of open_end.

const isValid =
value && formData.start && new Date(value) > new Date(formData.start);
(value && formData.start && new Date(value) > new Date(formData.start)) ||
(formData.open_end && formData.start === formData.end);
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

@stevepiercy
Copy link
Collaborator

I was in error yet again. Please see the amended note in #7993 (comment).

@TimoBroeskamp
Copy link
Author

@stevepiercy Okay just to clarify so that I know I understood it right.
#7993 (comment)
When emptying the "Event Ends" field it should get the value Null or None and the same should happen when I check the "opne end" checkbox right? Therefore you get no entry in the DTEND property for the .ics file.

The third point about events without an end or recurring events is something that needs to be changed in the backend so its not related directly to my pr right?

@stevepiercy
Copy link
Collaborator

When emptying the "Event Ends" field it should get the value Null or None and the same should happen when I check the "opne end" checkbox right? Therefore you get no entry in the DTEND property for the .ics file.

Ummm... that begs the question, why do we have two inputs for the same purpose?

I added a comment to the original issue. Let's keep the discussion about how to proceed in #7993. I neglected to complete the draft of a PLIP, which I think is necessary because it touches multiple repositories, but I hope to finish it tonight.

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.

4 participants