Decimal Values Ignored when Shifting Datetimes via Bulk Editing

[tn3rb]

steps to reproduce

• using new EDTR, add/edit a date/ticket and set start date to 9:00 am
• change to table view and click "show bulk actions"
• select one or more entities using checkboxes
• choose "edit date/ticket details" from dropdown and click "apply"
• scroll down to the "Shift dates" section
• using the number input and dropdowns enter 1.5 hours later
• click submit
• start date will have changed by only 1 hour instead of 1 and a half

[alexkuc]

Issue successfully reproduced locally, will proceed to work on the bug fix

[alexkuc]

@tn3rb In the process of this bug fix, I will make this field required

Because currently, if you omit this field, but specify time, e.g. 1.5 hours, nothing will happen i.e. Submit button works without asking for that earlier/later but nothing actually happens to time. Not great UX, imho.

[alexkuc]

Summary of my understanding of GraphQL implementation on EE:

• plugin event-espresso-core uses WPGraphQL to implement GQL
• the domain data is defined at cafe/plugins/event-espresso-core/core/services/graphql
• frontend uses Apollo GraphQL client
• to connect backend to frontend, monorepo cafe exposes global window variable called eventEspressoData
• there are classes extend JsonDataNode that define data for eventEspressoData following DDD paradigm
• contents of those classes is printed using class JsonDataNodeHandler using method printDataNode()

[alexkuc]

It looks like bug is caused by this utility function

https://github.com/eventespresso/barista/blob/aeba6e81028b49d12c94032164406a637454481f/packages/dates/src/utils/misc.ts#L32-L44

[alexkuc]

@tn3rb bug happens for both operations, earlier and later so I'll be fixing both

[alexkuc]

Further inspection of the source code reveals the following

https://github.com/eventespresso/barista/blob/aeba6e81028b49d12c94032164406a637454481f/packages/dates/src/utils/addSub.ts#L1-L13

https://github.com/eventespresso/barista/blob/aeba6e81028b49d12c94032164406a637454481f/packages/dates/src/utils/addSub.ts#L40-L43

https://github.com/eventespresso/barista/blob/aeba6e81028b49d12c94032164406a637454481f/packages/dates/src/utils/addSub.ts#L45-L52

The functions are not isomorphic even though date-fns supports subtraction function, e.g. subHours. Perhaps it is worthwhile to make it so.

Both functions use the same rounding logic which causes the bug:

Positive decimals will be rounded using Math.floor, decimals less than zero will be rounded using Math.ceil.

[alexkuc]

Due to non-accurate representation of floating numbers in JS, I'll be using the following logic

• use modulo to determine if number is a whole, e.g. 5 % 1; // 0 or 5.23 % 1; // 0.23000000000000043
• if number is whole i.e. integer proceed as it is
• if number has fraction, store that fraction to a variable
• since all time units are sexagesimal or base 60, we would multiple the fraction and using previous time unit from the integer, e.g. if we are dealing with hours, the fraction is minutes; if we are dealing with days, the fraction is hours, etc.

[alexkuc]

Function add is used only by misc.ts so scope for change is minimal but adjustment of unit tests might be necessary

[alexkuc]

Same goes for sub function which is only used by misc.ts

[alexkuc]

Inspecting unit tests suggest that ability to support fractions was never intended

[alexkuc]

Apparently, the value of 0.5 is not supported

Edit: when trying to set value of 0.5 the warning above is shown and the value is changed to 1

[alexkuc]

TODOs

• [x] add new utility function that would support fractional values
• [x] add unit tests for new function
• [x] add support for 0.5 value on input field

Edit: TODO for field validation is now tracked via #1196

[tn3rb]

thank you for the thorough description of the issue 🙇🏻

[tn3rb]

re: earlier/later input

I will make this field required

is/was there a default set in the submit handler for that functionality?

ie: if the default was later then maybe we can simply set that as the default for the input, then remove the null option, so that the input will always have a value

[alexkuc]

There is no default currently. When editor loads, it looks like this:

We could set it to later by default but that might lead to some users overlooking it and changing event to a wrong time.

[alexkuc]

Using simple console.log I can see that function receives a Date object for its argument date

https://github.com/eventespresso/barista/blob/c9ddddde495eb7d6d3d851547f378c2e94a8542b/packages/dates/src/utils/misc.ts#L32-L44

[alexkuc]

After quick chat with @tn3rb on Slack, it became obvious that support for floats leads to not-so-trivial rounding rules. For example, if shifting date by 5.23 days, how do you round that? Do you round it as 5 days 6 hours or as 5 days, 5 hours, 31 minutes, 12 seconds? While technically feasible, it will require time to write this logic, write unit tests, do the actual testing, etc. From a business point of view, it is not entirely clear how valuable it is to support ability to shift date by 5.23 so instead a different approach will be taken. Support for floats will be removed and each unit will be presented as a standalone field. Wireframe follows:

[alexkuc]

After further code review, I have decided to finish the feature since it was actually 80% complete. The logic for calculating time is as follows:

• get integer (whole) of the given value and add it to Date object using date-fns function
• calculate remainder
• get "previous" time unit for the remainder, e.g. hours for days, days for weeks, weeks for month, etc.
• calculate new date
• repeat process until the remainder is zero