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

Switch the web UI to the new endpoint for logged in users #25494

Merged
merged 1 commit into from
Jul 19, 2017

Conversation

PVince81
Copy link
Contributor

@DeepDiver1975

To be tested fully.

Seems we also need more integration tests, found a bug already on root (will raise)

@PVince81 PVince81 added this to the 9.2 milestone Jul 15, 2016
@mention-bot
Copy link

@PVince81, thanks for your PR! By analyzing the annotation information on this pull request, we identified @LukasReschke and @DeepDiver1975 to be potential reviewers

@PVince81
Copy link
Contributor Author

PVince81 commented Sep 8, 2016

@PVince81
Copy link
Contributor Author

PVince81 commented Sep 12, 2016

@PVince81
Copy link
Contributor Author

PVince81 commented Sep 22, 2016

  • BUG: missing share info, seems the plugin is missing there too

@PVince81
Copy link
Contributor Author

and also comments plugin for "comments-unread":

<?xml version="1.0"?>
<d:multistatus xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:cal="urn:ietf:params:xml:ns:caldav" xmlns:cs="http://calendarserver.org/ns/" xmlns:card="urn:ietf:params:xml:ns:carddav" xmlns:oc="http://owncloud.org/ns">
 <d:response>
  <d:href>/owncloud/remote.php/dav/files/admin/test/</d:href>
  <d:propstat>
   <d:prop>
    <d:getlastmodified>Thu, 22 Sep 2016 10:59:03 GMT</d:getlastmodified>
    <d:getetag>&quot;57e3b977111c7&quot;</d:getetag>
    <d:resourcetype>
     <d:collection/>
    </d:resourcetype>
    <oc:fileid>9</oc:fileid>
    <oc:permissions>RDNVCK</oc:permissions>
    <oc:size>0</oc:size>
    <oc:tags/>
    <oc:favorite>1</oc:favorite>
    <oc:owner-display-name>admin</oc:owner-display-name>
   </d:prop>
   <d:status>HTTP/1.1 200 OK</d:status>
  </d:propstat>
  <d:propstat>
   <d:prop>
    <d:getcontenttype/>
    <d:getcontentlength/>
    <oc:comments-unread/>
    <oc:share-types/>
   </d:prop>
   <d:status>HTTP/1.1 404 Not Found</d:status>
  </d:propstat>
 </d:response>
 <d:response>
  <d:href>/owncloud/remote.php/dav/files/admin/test/abc/</d:href>
  <d:propstat>
   <d:prop>
    <d:getlastmodified>Thu, 22 Sep 2016 10:59:03 GMT</d:getlastmodified>
    <d:getetag>&quot;57e3b9770c56a&quot;</d:getetag>
    <d:resourcetype>
     <d:collection/>
    </d:resourcetype>
    <oc:fileid>10</oc:fileid>
    <oc:permissions>RDNVCK</oc:permissions>
    <oc:size>0</oc:size>
    <oc:tags/>
    <oc:favorite></oc:favorite>
    <oc:owner-display-name>admin</oc:owner-display-name>
   </d:prop>
   <d:status>HTTP/1.1 200 OK</d:status>
  </d:propstat>
  <d:propstat>
   <d:prop>
    <d:getcontenttype/>
    <d:getcontentlength/>
    <oc:comments-unread/>
    <oc:share-types/>
   </d:prop>
   <d:status>HTTP/1.1 404 Not Found</d:status>
  </d:propstat>
 </d:response>
</d:multistatus>

@PVince81
Copy link
Contributor Author

PVince81 commented Sep 22, 2016

  • BUG: missing "comments-unread" property

@PVince81
Copy link
Contributor Author

PVince81 commented Oct 5, 2016

Looks like we're done here, it works !

@PVince81
Copy link
Contributor Author

Fixes #22753

@PVince81
Copy link
Contributor Author

PVince81 commented Dec 6, 2016

Any objections to this ?

The only part I see is that QA would need to retest all the files app features (comments, system tags, file operations, etc).

@DeepDiver1975 @owncloud/qa @jvillafanez

@jvillafanez
Copy link
Member

Not tested but changes look good 👍

@PVince81 PVince81 modified the milestones: 10.0.1, 10.0 Apr 7, 2017
@PVince81 PVince81 modified the milestones: 10.0.2, 10.0.1 May 19, 2017
@PVince81 PVince81 modified the milestones: 10.0.2, 10.0.3 May 26, 2017
@PVince81
Copy link
Contributor Author

PVince81 commented Jul 4, 2017

and we keep on finding issues with the new DAV endpoint... like the recent quota one: #28159

@DeepDiver1975 DeepDiver1975 self-requested a review July 19, 2017 11:37
@PVince81 PVince81 merged commit 17840e7 into master Jul 19, 2017
@PVince81 PVince81 deleted the dav-usenewendpont branch July 19, 2017 11:38
@phil-davis
Copy link
Contributor

@PVince81 @DeepDiver1975 this change causes 2 fails in the UI tests.
The last Travis-CI run before this was merged yesterday was from before the UI tests existed, so it passed many moons ago. That is why the CI for this PR shows good, but when it has been rebased/merged to master the CI fails from the latest master.
See issue #28441

@PVince81
Copy link
Contributor Author

PVince81 commented Aug 1, 2017

@DeepDiver1975 should we backport this to stable10 ?

@PVince81
Copy link
Contributor Author

Regression: #28730.

Maybe we could backport for 10.0.4. If we do, we also need to backport the fix of the above ticket.

@phil-davis
Copy link
Contributor

Note: if/when this is backported to 10.0.? then either issue #28441 needs to be fixed and the fix backported, or PR #28442 needs to be backported to skip the tests that will fail.

@PVince81
Copy link
Contributor Author

The above fix was wrong.

This is the correct fix for encoding mismatch: #28749

So if we backport "switch to new DAV endpoint in web UI", we also need to backport #28749

@PVince81
Copy link
Contributor Author

backport here: #28769

@PVince81 PVince81 mentioned this pull request Aug 31, 2017
13 tasks
@PVince81
Copy link
Contributor Author

Another missing bit: #28874

@PVince81
Copy link
Contributor Author

PVince81 commented Sep 5, 2017

Switching back here #28914 because of #28779 which is just the tip of the iceberg. There are more perf issues hidden in there, to be looked into in this WIP PR #28853

@lock
Copy link

lock bot commented Aug 2, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants