CalDAV/PropFilter: set empty array as default value for time-range #1300
Conversation
|
See also #1275. |
phil-davis
force-pushed
the
patch/caldav-time-range
branch
from
12e2f35
to
66f410c
Compare
Codecov Report
@@ Coverage Diff @@
## master #1300 +/- ##
=========================================
Coverage 97.37% 97.37%
Complexity 2817 2817
=========================================
Files 174 174
Lines 7996 7996
=========================================
Hits 7786 7786
Misses 210 210
Continue to review full report at Codecov.
|
|
I rebased and fixed the tests. |
|
@DeepDiver1975 @staabm opinion please on this PR and PR #1364 |
| 'is-not-defined' => false, | ||
| 'param-filters' => [], | ||
| 'text-match' => null, | ||
| 'time-range' => false, | ||
| 'time-range' => [], | ||
| ]; |
There was a problem hiding this comment.
I dont have expertise in the part of the codebase - but I think it might be usefull if we could declare constants for this false, null and [] magic values... I don't know how to interpret them.
There was a problem hiding this comment.
Might be a good idea, but goes a bit beyond this PR. I am not familiar at all with the SabreDAV code base and I am not willing to stir in it too much by adding constants. 😉
There was a problem hiding this comment.
I can do refactoring for "magic values" after we sort out this PR.
@DeepDiver1975 or @evert are you familiar with this and can give an opinion on changing from false to [] ?
phil-davis
force-pushed
the
patch/caldav-time-range
branch
from
66f410c
to
c3e20f7
Compare
|
@DeepDiver1975 any chance to look at this and #1364 please? |
|
Any chance this could get merged? I think it is ready and has been reviewed, the only open suggestion is some larger refactoring that should happen outside the scope of this PR. |
|
looks good to me 👍 -> merge |
This changes the default value of
time-rangein CalDAV prop filters fromfalse(not matching RFC 4791 https://tools.ietf.org/html/rfc4791#section-9.9) to an empty array.This closes #1299.