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

Fix propfind type of subnodes with depth >= 1 #1344

Merged
merged 4 commits into from
Nov 16, 2021

Conversation

n-peugnet
Copy link
Contributor

@n-peugnet n-peugnet commented May 14, 2021

With properties manually added in a plugin using the propFind event like in this example: #482 and with depth >= 1, the subnodes' propfind's type was incorrectly deduced.
This resulted in custom properties only being added to the first node and not on the subnodes.

@codecov
Copy link

codecov bot commented May 14, 2021

Codecov Report

Merging #1344 (09bf553) into master (d762b19) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1344   +/-   ##
=========================================
  Coverage     97.31%   97.31%           
  Complexity     2806     2806           
=========================================
  Files           174      174           
  Lines          7975     7975           
=========================================
  Hits           7761     7761           
  Misses          214      214           
Impacted Files Coverage Δ
lib/DAV/Server.php 96.58% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d762b19...09bf553. Read the comment docs.

@n-peugnet
Copy link
Contributor Author

n-peugnet commented May 20, 2021

I added a test case and we can see that it fails on master:

There was 1 failure:

1) Sabre\DAV\ServerPropsTest::testPropFindEmptyBodyDepth1Custom
Response should contain 5 elements
Failed asserting that 1 matches expected 5.

/home/nicolas/Source/php/dav/tests/Sabre/DAV/ServerPropsTest.php:93

@kesselb
Copy link
Contributor

kesselb commented Nov 2, 2021

Hey @n-peugnet 👋

please rebase your branch to have a recent CI run.

@n-peugnet n-peugnet force-pushed the patch-1 branch 2 times, most recently from 21dc3a6 to 78fb937 Compare November 3, 2021 17:49
@n-peugnet
Copy link
Contributor Author

Done @kesselb

@n-peugnet
Copy link
Contributor Author

@phil-davis, could you maybe approve this PR to run the GitHub workflow ?

I have run the tests locally and everything is working fine.

When custom properties are manually added in a plugin with the `propFind` event like in this example: sabre-io#482 and depth >= 1, the subnodes' propfind's type was incorrectly deducted.
This resulted in custom properties only added to the first node and not on the subnodes.
Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

While I was looking, I refactored some identical preg_replace "magic" in the test code so that it is in a separate function.

@phil-davis phil-davis merged commit 7a638b6 into sabre-io:master Nov 16, 2021
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.

None yet

3 participants