Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Move CRUDS permission consts to OC(P) namespace #345

Closed
tanghus opened this Issue · 11 comments

4 participants

Thomas Tanghus Michael Gapczynski Thomas Müller Frank Karlitschek
Thomas Tanghus
Collaborator

The sharing API defines constants for permissions on shared objects:

const PERMISSION_CREATE = 4;
const PERMISSION_READ = 1;
const PERMISSION_UPDATE = 2;
const PERMISSION_DELETE = 8;
const PERMISSION_SHARE = 16;

As these constants can be relevant in contexts that doesn't have to do with sharing I suggest moving them to for example base.php in class OC and also add a PERMISSION_ALL which would be all permissions.

A use case for this is an app where you need to list a mix of shared and owned objects (files/contacts/events etc.). Each object has it's own permissions. For shared objects they are determined from what OCP\Share::getItemsSharedWith() returns, but for owned objects there is no pre-defined constant.
It can be produced by either just assigning $permissions = 31 (ugly) or $permissions = \OCP\Share::PERMISSION_CREATE | \OCP\Share::PERMISSION_READ | OCP\Share::PERMISSION_UPDATE | \OCP\Share::PERMISSION_DELETE | OCP\Share::PERMISSION_SHARE; (ugly, non-DRY and error prone). Being able to assign $permissions = OC(P)::PERMISSIONS_ALL would improve readability and reduce the risk of omitting permissions or making copy/paste errors.

See also issue #151 and https://mail.kde.org/pipermail/owncloud/2012-November/006388.html

Michael Gapczynski

Are we making a whole new class for this or is there somewhere we can include these constants? Do we need any helper functions?

Thomas Müller

:+1:
Let's put it into the public namespace OCP.
Maybe a new file required by base.php

Thomas Tanghus
Collaborator

Would this work?:

lib/public/constants.php (or something)

namespace OCP;

const PERMISSION_CREATE = 4;
const PERMISSION_READ = 1;
const PERMISSION_UPDATE = 2;
const PERMISSION_DELETE = 8;
const PERMISSION_SHARE = 16;
const PERMISSION_ALL = PERMISSION_CREATE | PERMISSION_READ | PERMISSION_UPDATE 
    | PERMISSION_DELETE | PERMISSION_SHARE;

Edit: corrected for my apparent blindness :-P

Thomas Tanghus
Collaborator

Do we need any helper functions?

Not any I can think of from the top of my head.

Thomas Müller

@tanghus
Please submit this as a pull request and we push it in.

Frank Karlitschek
Owner

wouldn't it make more sense to create a lib/public/sharing.php where we can also export sharing related classes and function for other apps?

Thomas Tanghus
Collaborator

share.php is already in lib/public or do I misunderstand you?

Just pushed btw ;) #356

Frank Karlitschek
Owner

ahh ok. I missed that. :-) Now we only have to make sure that it still works if someone disables the sharing app. All apps should be optional to have to code as modular as possible.

Thomas Tanghus
Collaborator

You've got a point there. I hadn't tested any apps with the sharing api disabled, and I do get some errors. Not because of this PR though.

Thomas Müller

@tanghus we can close that one - right?

Thomas Tanghus
Collaborator

I thought I had done it already :-)

Thomas Tanghus tanghus closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.