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

Add OCS sharing info to capabilities - take 2 #13964

Merged
merged 7 commits into from
Mar 30, 2015
Merged

Add OCS sharing info to capabilities - take 2 #13964

merged 7 commits into from
Mar 30, 2015

Conversation

rullzer
Copy link
Contributor

@rullzer rullzer commented Feb 7, 2015

Since the previous request was already merged (which was a bit to soon). Here is another take. It is still a patch for #13673.

@rullzer rullzer added this to the 8.1-next milestone Feb 7, 2015
public function getCaps() {
$res = array();

$public = false;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@LukasReschke in reply to your comments on the previous PR.

I did it like this to have the json return format make a bit more sense. So it is false if public sharing is disabled. And an array if not. The same is done for the expire_date.

Another, maybe more elegant solution is to have an enabled element in the array. So that the format is always the same.

@rullzer
Copy link
Contributor Author

rullzer commented Feb 26, 2015

cc @PVince81 @LukasReschke @DeepDiver1975

Could you have another look at this? Would be nice to be able to allow checking for this in the client(s).

@@ -0,0 +1,87 @@
<?php
/**
* @author Not Committed Yet <not.committed.yet>

Choose a reason for hiding this comment

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

Accidental leftover?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah you are right.
Fixed.

@PVince81
Copy link
Contributor

@DeepDiver1975 should this be moved to the OCS Sharing API as discussed on IRC ? (instead of the capabilities)

@DeepDiver1975
Copy link
Member

@DeepDiver1975 should this be moved to the OCS Sharing API as discussed on IRC ? (instead of the capabilities)

Well - I had a chat with @dragotin and he actually likes the idea to have one call to get all capabilities instead of calling multiple apis ....

@DeepDiver1975
Copy link
Member

@rullzer this pr by itself looks good - we finally have to decide on how to move on ...

@PVince81
Copy link
Contributor

PVince81 commented Mar 9, 2015

@DeepDiver1975 small reminder: one issue remains is that if we use the app framework for getcapabilities in the future it won't be possible to combine responses from multiple apps that respond to the same call. (which I believe is one of the reason this discussion started)

Display the capabilities regarding file sharing in the capabilities API.
This will allow the clients to provide users a better experince.
This change allows for more generic parsing for the capabilities.
* @param string[] $data Capabilities
* @return string[]
*/
function getFilesSharingPart(array $data) {
Copy link
Contributor

Choose a reason for hiding this comment

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

public, protected or private

@scrutinizer-notifier
Copy link

A new inspection was created.

@ghost
Copy link

ghost commented Mar 11, 2015

Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/10363/
Test PASSed.

@rullzer
Copy link
Contributor Author

rullzer commented Mar 11, 2015

A possible solution for the capabilities issue described by @PVince81 is in #14805. However I would prefer not to wait with this PR. It would be great to have this in 8.1.

@rullzer
Copy link
Contributor Author

rullzer commented Mar 18, 2015

@DeepDiver1975 @PVince81 how to we want to proceed here? As I would really like to have a feature like this in 8.1 so the client can provide a better UX.

@PVince81
Copy link
Contributor

I like the idea of the capabilities manager

@rullzer rullzer mentioned this pull request Mar 21, 2015
@rullzer
Copy link
Contributor Author

rullzer commented Mar 24, 2015

PR for Capabilities manager is in #15093 . Once that is in place adding the capabilities here should be a bit nicer.

@dragotin
Copy link
Contributor

@DeepDiver1975 yes, please give this a priority, as it blocks functionality in the client. Would be great :-)

@DeepDiver1975
Copy link
Member

The capabilities manager is for sure a nicer approach - but we simply cannot use it yet on the ocs/v1.php endpoint.

👍 for moving on using the current ocs approach

@rullzer
Copy link
Contributor Author

rullzer commented Mar 30, 2015

@DeepDiver1975 well the code is rather easy to understand. So it would be not to hard to move this capabilities class also to the capabilities manager for 8.2.

So one more +1 and I can make the users of the client a bit happier ;)

@DeepDiver1975
Copy link
Member

@MorrisJobke @nickvergessen please review - thx

@MorrisJobke
Copy link
Contributor

Tested 👍

MorrisJobke added a commit that referenced this pull request Mar 30, 2015
Add OCS sharing info to capabilities - take 2
@MorrisJobke MorrisJobke merged commit cfe241a into owncloud:master Mar 30, 2015
@MorrisJobke
Copy link
Contributor

Before:

<?xml version="1.0"?>
<ocs>
 <meta>
  <status>ok</status>
  <statuscode>100</statuscode>
  <message/>
 </meta>
 <data>
  <capabilities>
   <files>
    <versioning>1</versioning>
    <undelete>1</undelete>
    <bigfilechunking>1</bigfilechunking>
   </files>
   <core>
    <pollinterval>60</pollinterval>
   </core>
  </capabilities>
  <version>
   <major>8</major>
   <minor>1</minor>
   <micro>0</micro>
   <string>8.1 pre alpha</string>
   <edition></edition>
  </version>
 </data>
</ocs>

After:

<?xml version="1.0"?>
<ocs>
 <meta>
  <status>ok</status>
  <statuscode>100</statuscode>
  <message/>
 </meta>
 <data>
  <capabilities>
   <files>
    <versioning>1</versioning>
    <undelete>1</undelete>
    <bigfilechunking>1</bigfilechunking>
   </files>
   <files_sharing>
    <public>
     <enabled>1</enabled>
     <password>
      <enforced>1</enforced>
     </password>
     <expire_date>
      <enabled>1</enabled>
      <days>7</days>
      <enforced>1</enforced>
     </expire_date>
     <send_mail>1</send_mail>
    </public>
    <user>
     <send_mail>1</send_mail>
    </user>
    <resharing>1</resharing>
   </files_sharing>
   <core>
    <pollinterval>60</pollinterval>
   </core>
  </capabilities>
  <version>
   <major>8</major>
   <minor>1</minor>
   <micro>0</micro>
   <string>8.1 pre alpha</string>
   <edition></edition>
  </version>
 </data>
</ocs>

Command:

curl http://admin:pwd@localhost/master/ocs/v1.php/cloud/capabilities

@dragotin
Copy link
Contributor

Looks very sufficient 👍

@DeepDiver1975
Copy link
Member

@carlaschroder I guess we have to update the api docs on this - THX

@rullzer rullzer deleted the capabilities branch May 22, 2015 11:51
@lock lock bot locked as resolved and limited conversation to collaborators Aug 12, 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.

7 participants