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.

Sign up for GitHub

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

Jump to bottom

Rewire dav files endpoint to home (better approach) #1125

Merged
merged 2 commits into from
Aug 26, 2020
Merged

Rewire dav files endpoint to home (better approach) #1125

merged 2 commits into from
Aug 26, 2020

Conversation

PVince81
Copy link
Contributor

@PVince81 PVince81 commented Aug 26, 2020 •
edited
Loading

If the user specified in the dav files URL matches the current one,
rewire it to use the webDavHandler which is wired to the home storage.

This fixes path mapping issues.

Obsoletes #1120

  • EOS
    • test regular file operations on dav files
    • test upload on dav files
    • test file ops on a received share
    • test upload on a received share
  • BUG: litmus issue with OPTIONS on new dav
  • discard or update code from Do not swallow 'not found' errors in Stat #1124 ? => discarded and rebased

@PVince81 PVince81 requested a review from labkode as a code owner August 26, 2020 13:24
This was referenced Aug 26, 2020
Closed
Closed
@PVince81
Copy link
Contributor Author

hmmm, so did my PR magically fix many issues ? @phil-davis

image

@PVince81
Copy link
Contributor Author

I've pushed a fix for the OPTIONS request, that should at least fix the Litmus tests

@PVince81
Copy link
Contributor Author

there was a bug to the fallback, fixed now

expecting more tests to pass now

@PVince81 PVince81 marked this pull request as draft August 26, 2020 15:10
@PVince81
Copy link
Contributor Author

local and litmus tests passed.

there are still a few trashbin tests that apparently were supposed to fail?
image

but I don't really see how this PR would help fixing those as it doesn't affect the trashbin endpoint

@PVince81 PVince81 mentioned this pull request Aug 26, 2020
Merged
Vincent Petry added 2 commits August 26, 2020 17:42
Rewire dav files endpoint to home
ad24657 
If the user specified in the dav files URL matches the current one,
rewire it to use the webDavHandler which is wired to the home storage.

This fixes path mapping issues.
Enable passing trashbin tests after dav rewiring
c49663b
@PVince81
Copy link
Contributor Author

Rebased onto master which has #1124 merged now.

I've also adjusted the expected failures. It is possible that the rewiring has fixed some issues very indirectly.

@PVince81 PVince81 marked this pull request as ready for review August 26, 2020 15:42
@PVince81 PVince81 requested a review from butonic August 26, 2020 15:43
@butonic
Copy link
Contributor

butonic commented Aug 26, 2020

Cool!

phil-davis
phil-davis approved these changes Aug 26, 2020
View reviewed changes
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.

All the trashbin scenarios that have started passing are for the "new" dav-path. So this PR definitely does fix stuff.

tests/acceptance/expected-failures-on-OC-storage.txt
Comment on lines -503 to -509
apiTrashbin/trashbinFilesFolders.feature:273
apiTrashbin/trashbinFilesFolders.feature:274
apiTrashbin/trashbinFilesFolders.feature:275
apiTrashbin/trashbinFilesFolders.feature:276
apiTrashbin/trashbinFilesFolders.feature:277
apiTrashbin/trashbinFilesFolders.feature:278
apiTrashbin/trashbinFilesFolders.feature:289
Copy link
Contributor

Choose a reason for hiding this comment

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

These are Scenario Outline Examples for the "new" dav-path. The Examples for the "old" dav-path were already passing. So the "rewiring" in this PR has fixed some issue with the "new" dav-path. That is "a good thing" (tm)

tests/acceptance/expected-failures-on-OC-storage.txt
Comment on lines -537 to -538
apiTrashbin/trashbinRestore.feature:295
apiTrashbin/trashbinRestore.feature:310
Copy link
Contributor

Choose a reason for hiding this comment

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

These are both Examples for "new" dav-path. The example for "old" dav-path at 294 and 309 were already passing.

@phil-davis
Copy link
Contributor

@labkode @butonic or ? IMO this is "a good thing". Who can merge?

@ishank011 ishank011 merged commit c0f54e1 into cs3org:master Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ishank011 ishank011 ishank011 approved these changes

@phil-davis phil-davis phil-davis approved these changes

@butonic butonic butonic approved these changes

@labkode labkode Awaiting requested review from labkode

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@PVince81 @butonic @phil-davis @ishank011