-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[Sharing 2.0] update share #21887
[Sharing 2.0] update share #21887
Conversation
By analyzing the blame information on this pull request, we identified @DeepDiver1975 to be a potential reviewer |
if ($publicUpload === 'true') { | ||
$share->setPermissions(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE); | ||
} else if ($publicUpload === 'false') { | ||
$share->setPermissions(\OCP\Constants::PERMISSION_READ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, so "publicUpload" has priority over "permissions", got it.
Code looks good 👍 |
smashbox tests had a timeout... |
4104960
to
44c073b
Compare
->set('item_source', $qb->createNamedParameter($share->getPath()->getId())) | ||
->set('file_source', $qb->createNamedParameter($share->getPath()->getId())) | ||
->set('token', $qb->createNamedParameter($share->getToken())) | ||
->set('expiration', $qb->createNamedParameter($share->getExpirationDate(), 'datetime')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IQueryBuilder::PARAM_DATE instead of 'datetime'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah was not there when this code was writen... but will fix :)
besides my notes 👍 |
Rebased and comments addressed. lets see if CI still thinks everything is fine. |
if ($fileTarget) $qb->setValue('file_target', $qb->expr()->literal($fileTarget)); | ||
if ($permissions) $qb->setValue('permissions', $qb->expr()->literal($permissions)); | ||
if ($token) $qb->setValue('token', $qb->expr()->literal($token)); | ||
if ($expiration) $qb->setValue('expiration', $qb->createNamedParameter($expiration, 'datetime')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IQueryBuilder
29b8985
to
d11682d
Compare
@nickvergessen could you smash this? Just to be sure. |
Smashbox tests passed for me locally. |
Awesome thanks... then this is good to go! |
[Sharing 2.0] update share
More for #19331
Updating shares is now handled in the sharemanager.
Basically we run the same bunch of tests for updateShare as we do for createShare. And some more.
Please have a look: @DeepDiver1975 @schiesbn @nickvergessen @PVince81