Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(nextcloud)!: deserialize webdav date fields #1850

Merged
merged 3 commits into from
Apr 3, 2024

Conversation

Leptopoda
Copy link
Member

No description provided.

@Leptopoda Leptopoda requested a review from provokateurin April 2, 2024 20:04
Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool stuff!

packages/nextcloud/generate_props.dart Outdated Show resolved Hide resolved
Copy link

codecov bot commented Apr 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 44.63%. Comparing base (67a122a) to head (0b2deb1).
Report is 2 commits behind head on main.

❗ Current head 0b2deb1 differs from pull request most recent head d10b22a. Consider uploading reports for the commit d10b22a to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1850      +/-   ##
==========================================
- Coverage   44.67%   44.63%   -0.05%     
==========================================
  Files         114      114              
  Lines        3805     3802       -3     
==========================================
- Hits         1700     1697       -3     
  Misses       2105     2105              
Flag Coverage Δ
neon_dashboard 92.56% <ø> (ø)
neon_framework 37.36% <ø> (ø)
neon_talk 100.00% <ø> (ø)

see 1 file with indirect coverage changes

@Leptopoda Leptopoda requested a review from provokateurin April 2, 2024 20:52
@Leptopoda Leptopoda enabled auto-merge April 2, 2024 20:59
Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait you also changed the int/bool fields?

@Leptopoda
Copy link
Member Author

yep. Why?
I only changed it where the docs said it's either 1 or 0

@provokateurin
Copy link
Member

But will it serialize correctly? Because the server might not accept a true instead of a 1 (I didn't test that though).

@Leptopoda
Copy link
Member Author

You're right.
I also only checked the code for deserialization.

@provokateurin
Copy link
Member

According to https://www.w3.org/TR/xmlschema-2/#boolean it should work because all those representations are possible. If this turns out to be true then the server docs should be changed to clearly state that the values are not integers but actually booleans.

Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oc:favorite should also be bool.
At least the favorites prop handles all representations correctly: https://github.com/nextcloud/server/blob/c0d21b44437d6ff0ef4ffa08096f18c1e4870fcc/apps/dav/lib/Connector/Sabre/TagsPlugin.php#L285

@provokateurin
Copy link
Member

Ok I had a look at all the boolean fields except for oc:favorite they are all dead props meaning the client can't update them. oc:favorite has the correct handling, so everything looks good!

@provokateurin
Copy link
Member

I already had the changes for oc:favorite for local testing, so I just pushed them. Please squash and rebase and then we are good to go.

@Leptopoda Leptopoda force-pushed the feat/nextcloud/deserialize_webdav_time_fields branch from 585161c to d10b22a Compare April 3, 2024 07:37
@Leptopoda Leptopoda requested a review from provokateurin April 3, 2024 07:37
@Leptopoda
Copy link
Member Author

Thanks for taking a look

@Leptopoda Leptopoda merged commit d0e9647 into main Apr 3, 2024
8 checks passed
@Leptopoda Leptopoda deleted the feat/nextcloud/deserialize_webdav_time_fields branch April 3, 2024 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants