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

[stable11] Fix legacy DAV endpoint #2685

Merged
merged 2 commits into from
Dec 16, 2016
Merged

[stable11] Fix legacy DAV endpoint #2685

merged 2 commits into from
Dec 16, 2016

Conversation

LukasReschke
Copy link
Member

@LukasReschke LukasReschke commented Dec 15, 2016

This is #2684 for stable11, as the test fail this clearly proofs that something is broken 😢

See https://drone.nextcloud.com/nextcloud/server/3075/32 and https://drone.nextcloud.com/nextcloud/server/3075/35

cc @rullzer

@mention-bot
Copy link

@LukasReschke, thanks for your PR! By analyzing the history of the files in this pull request, we identified @Faldon, @tcitworld and @MorrisJobke to be potential reviewers.

@rullzer rullzer added this to the Nextcloud 11.0.1 milestone Dec 15, 2016
@rullzer rullzer self-assigned this Dec 15, 2016
@rullzer rullzer force-pushed the stable11-2648 branch 2 times, most recently from 45cb71d to 811f167 Compare December 15, 2016 10:16
Since the tests to quite hugely rely on sync tokens being present I also included those in the legacy backend.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@LukasReschke
Copy link
Member Author

LukasReschke commented Dec 15, 2016

@rullzer Can you revert your changes to the test file? In my opinion we should 1:1 use the same test file on this branch as in stable10 and if something fails then … well, then we should adjust the endpoint. Not the test file. This seems wrong to me.

* CaldavBackend is now endpoint aware (use old style principals on old
endpoint and new onces on new).

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer
Copy link
Member

rullzer commented Dec 15, 2016

@pbek could you try this patch?

@rullzer
Copy link
Member

rullzer commented Dec 15, 2016

@LukasReschke also you please try again. Hopefully this works...

@pbek
Copy link
Member

pbek commented Dec 16, 2016

@rullzer the last test brought my NC version up to 12.0.0.10, after checking out stable11-2648 I get an error message Downgrading is not supported and is likely to cause unpredictable issues (from 12.0.0.10 to 11.0.0.10), is there a way to get around that?

@rullzer
Copy link
Member

rullzer commented Dec 16, 2016

@pbek uhm. If it is a live instance better not. If this works for @LukasReschke I'll create a master PR you should then be able to test.

@pbek
Copy link
Member

pbek commented Dec 16, 2016

@rullzer, no, I just created that instance for testing the CalDAV PRs.

@rullzer
Copy link
Member

rullzer commented Dec 16, 2016

ah uhm. Well either install again. Or edit the version in config/config.php

@pbek
Copy link
Member

pbek commented Dec 16, 2016

I went for the latter.
Now I'm happy to confirm that the PUT request went through (with an empty return body) and the ICS was modified on the server. QOwnNotes also works again with that branch!
Great job, @rullzer!

@mariokorte
Copy link

@rullzer I too can confirm your fix works! Thanks a million!

@rullzer
Copy link
Member

rullzer commented Dec 16, 2016

Phew. Ok let me wait for a review of @LukasReschke and then this can hopefully soon go in :D

@rullzer rullzer changed the title [PROOF_THAT_IT_IS_BROKEN] Add test execution against legacy DAV backend [stable11] Fix legacy DAV backend Dec 16, 2016
@rullzer rullzer changed the title [stable11] Fix legacy DAV backend [stable11] Fix legacy DAV endpoint Dec 16, 2016
@rullzer rullzer mentioned this pull request Dec 16, 2016
@rullzer rullzer added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Dec 16, 2016
@rullzer
Copy link
Member

rullzer commented Dec 16, 2016

Failing tests are unrelated (jsunit + timeout)

@LukasReschke
Copy link
Member Author

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants