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

event.js: Fix test for matching the start-date datepicker #524

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

zosterops-lateralis
Copy link

Hello,

From what I could test from the current version of the plugin, when editing a recurring event, it seems that the recurrence end date (or “until” date) isn't updated when setting the event start date to a later date.

To reproduce:

  1. Create a new event.
  2. Leave everything as set by default in the editor: the start and end date for the event should be the current date (e.g., 2022-10-07 in yyyy-mm-dd format).
  3. Pick a regular recurrence (daily or weekly, for instance), so that the “until” date shows in the form. This date should also be the same as the start and end dates above (i.e., 2022-10-07, following the same example).
  4. Set the start date to a later date (e.g., 2022-10-31).
  5. The “until” date hasn't changed (it is still 2022-10-07).

However, one would expect the “until” date to be updated as well, since it now falls earlier than the event start date. This behavior is actually expected, as per the comment on line 268 of js/event.js:

Event-Organiser/js/event.js

Lines 265 to 271 in 65c4e3b

date = $.datepicker.parseDate(instance.settings.dateFormat || $.datepicker._defaults.dateFormat, selectedDate, instance.settings);
dates.not(this).datepicker("option", option, date);
if (this.id == "from_date") {
//If updating start date, ensure recurrence end falls after this
schedule_last.datepicker("option", "minDate", date);
}

From what I could read from the code, it seems that this bug comes from the fact that the element ID of the start-date datepicker in the event editor has changed: it is not from_date (as written in the test on line 267 above), but eo-start-date.

A simple fix would be to write eo-start-date instead of from_date in the above test. However, since the role of each datepicker is now accessible using $(element).data('eo-datepicker'), it seems to me that matching this string should provide a more robust test for figuring out if we are currently handling events for a particular datepicker. The exact same test is already performed on line 263 of the same file, in the same function:

var option = ( 'start' == $(this).data('eo-datepicker')? "minDate": "maxDate" ),

This is therefore the solution I'm proposing in this PR. From what I could test, this fixes the issue.

Another solution would be to use the jQuery selector views.start_date, retrieve the element matching the query, and check that it is the same as this. Something like

if (this === $(views.start_date)[0]) {
	...
}

Of course, I'll gladly update my PR if you think this is a better solution!

I hope this will help! :)
In any case, thank you very much!

Zosterops

The element ID of the start-date datepicker in the event editor
has changed: it is not `from_date` anymore, but `eo-start-date`.

However, since the role of each datepicker is now accessible
using `$(element).data('eo-datepicker')`, matching this string
should provide a more robust test for figuring out if we are
currently handling events for a particular datepicker.
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.

None yet

1 participant