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.
It would be good tone to handle the case when $feed_element contains empty strings to not produce the wrong results on import.
If the $sub_field is equal to 'hours' and $feed_element contains an empty string this produces the 0 values for 'start_hour' and 'end_hour' which leads to round-the-clock values for office_hour field.
Any thoughts/objections against that idea?
Thanks in advance!
Comment | File | Size | Author |
---|---|---|---|
#2 | Handling-empty-hours-import-values-1965948-2.patch | 1.1 KB | ruslan.muradov |
#1 | Handling-empty-hours-import-values-1965948-1.patch | 1.1 KB | ruslan.muradov |
Comments
Comment #1
ruslan.muradov CreditAttribution: ruslan.muradov commentedIn case if anybody interested in solving this minor issue here is the patch that I have made. Hope it will helps
Comment #2
ruslan.muradov CreditAttribution: ruslan.muradov commentedHere is the "clean" version of patch - trailing whitespace is removed and the comments are compliant to drupal.org standards
Comment #3
johnvThanks for the patch. Correcting status.
Be aware that the patch seems to be implemented on top of #1965966: On feeds import, wrong deltas when entity has multiple 'office_hours'-fields.
Comment #4
ruslan.muradov CreditAttribution: ruslan.muradov commentedHmm. Not sure what you mean..
I have made the patch against the "clean" version of office_hours.feeds.inc file which I have obtained from 7.x-dev branch of project git-repo.
Cheers.
Comment #5
johnvYes, you are right, I was confused with the other patch.
This patch contains a 'return '. Which is quiet confusing for the quick reader.
Can you change it to a 'normal if-then-else?
Comment #6
ruslan.muradov CreditAttribution: ruslan.muradov commentedI was decided to use if() clause with return statement to keep the code as clean as possible. it-then-else produce less clean code in that case. But this is only IMO. Of course you can rewrite the code in any way you think is needed!
Thanks for the support and quick responses! Cheers!
Comment #7
johnvThis needs some more tests, and add more exeptions .
What happens (before and after this patch) if the following is imported:
- 09:00 - 19:00 (normal situation)
- - (this patch ? )
- 00:00 - 00:00 (this patch ? )
- 00:00 - 24:00 (around-the-clock: for now: 24:00 must be stored as 00:00, which is corrected in formatter)
- - 18:00 (error message?)
- 18:00 - (error message?)
Comment #8
ruslan.muradov CreditAttribution: ruslan.muradov commentedYou are right! There are a lot of cases when module does not handle the edge case conditions.
In that case there is three options
1) the user must control the correctness of incoming data
2) the module should contain most robust code which will guard against the data integrity
3) handle the cases through custom tampers
I have achieved correct behavior using the 3-rd way.
Comment #9
johnvFor better statistics, let's call this a feature request :-)
Comment #10
johnvComment #11
johnvComment #12
johnvComment #13
johnvGiven the life cycle of D7, this issue is considered closed. However, patches are appreciated.