Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.This is a follow-up issue for #2860038: Add a comment field to each time slot (D8).
ATM, comments can be added to a time slot. However, when the day is closed, and a comment is added, the comment disappears upon saving the entity.
Can this be fixed?
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | office-hours.patch | 2.71 KB | Geolim4 |
| #5 | office_hours-2860041-5.patch | 787 bytes | achton |
Comments
Comment #2
johnvSome debugging shows that you may not save empty values in the starthours and enhours columns.
The starting point is in OfficeHoursWidgetBase:massageFormValues(). If you allow an empty slot with comment there, then the hours are filled with value '0' instead of '', resulting in an opening time of '00:00 - 24:00'.
Comment #3
blacklabel_tom CreditAttribution: blacklabel_tom at Reason Digital commentedBumping this one as I've found it too.
Comment #4
blacklabel_tom CreditAttribution: blacklabel_tom at Reason Digital commentedComment #5
achtonPresumably, this could work if changing the property definition for starthours and endhours to string, as suggested in the comment in
OfficeHoursWidgetBase:massageFormValues(). I think you'd be compromising a bit of efficiency though, since the values right now are optimized for integer storage. Haven't tested that assumption though.I opted for a bit of "magic" behaviour, where we store a negative value if the timeslot is supposed to be empty. This allows the storage handler to correctly store our comment.
A simple PoC patch is attached, which works for me (my use case is exposing the data via a REST/GraphQL endpoint). However, when Drupal renders the fields, it still attempts to translate those values to a valid time (I get the time '10:46'). So if this is a viable approach, it still needs to take negative time values into account whereever these are rendered, and I don't know enough about this module to quickly achieve this.
Comments on this approach?
Comment #6
johnvI will review / test your solution.
Perhaps moving to string is the best option. So an empty value is not allowed?
I did also a grep for 'mpty' to see if an isEmpty() function is used.
Using the negative time is perhaps an option for other requests in the queue like 'open without closing time' .
Comment #7
Geolim4 CreditAttribution: Geolim4 as a volunteer commentedHello, is this issue planned to be fixed/merged ?
I facing the same problem too, thanks !
Edit: The patch above isn't working as the formatting of a closed becomes 00-00 24h00 :/
Comment #8
Geolim4 CreditAttribution: Geolim4 as a volunteer commentedI finally managed how to fix that, may this patch helps you.
Comment #9
johnvThanks @achton, @Geolim4,
Indeed, changing the database format to string would be the best option, but I do not want to risk any damage to existing installations.
@ Geolim4, I used another method to manipuate the formatter.