Fix VTODO validation for PERCENT-COMPLETE

[jarikmarwede]

This PR should fix the linked issue I created about the PERCENT-COMPLETE property of the VTODO being misspelled as just PERCENT. I also added a test to verify that the validation of the property now works as expected. If you think the test is unnecessary, I can also just remove it again.

Fixes #663

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.78%. Comparing base (1d0d0bd) to head (4f21237).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##             master     #664   +/-   ##
=========================================
  Coverage     98.78%   98.78%           
  Complexity     1875     1875           
=========================================
  Files            71       71           
  Lines          5273     5273           
=========================================
  Hits           5209     5209           
  Misses           64       64           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[phil-davis]

@staabm this PR changes "PERCENT" to "PERCENT-COMPLETE" (which is the correct name in the RFC spec)

If we merge this, then "PERCENT" will no longer be "officially" valid. But I suppose, if anybody has actually used it, "PERCENT" will just be an extra property that lives in some VTODO entries. It shouldn't break anything.

Do we want to also keep validating "PERCENT"?

[phil-davis]

Also, maybe a separate issue to address, I don't see any code that checks that the value of PERCENT-COMPLETE is an integer from 1 to 100.

[phil-davis]

Code is fine, thanks. I would like to get the opinion of @staabm also about the old "PERCENT"

[staabm]

I can't say much about it. this PR seems to align our impl with the RFC which feels like a good thing.

I don't know whether the previous impl is wrong though (or intentionally made like that for some unknown reason).

[phil-davis]

I don't know whether the previous impl is wrong though (or intentionally made like that for some unknown reason).

Actually, I don't think it really matters. For anyone currently using "PERCENT", it will still work. So they won't notice a difference.

For the clients that do the correct thing, and use "PERCENT-COMPLETE", they also will be fine, as long as they only specify it once, as per the spec.

[staabm]

Actually, I don't think it really matters. For anyone currently using "PERCENT", it will still work. So they won't notice a difference.

is this something we should test?

[phil-davis]

is this something we should test?'

yes, I have a test case and it passes. The VTodo validation code does not complain about unrecognized properties.
It will be easiest for me to merge this PR (which is from the master branch of @jarikmarwede fork) and then add more tests, rather than try to manage it in that other master branch.

[staabm]

thank you both!