Use the new shareManager to get a share based on a token #589
Conversation
|
By analyzing the blame information on this pull request, we identified @oparoz to be a potential reviewer |
|
again this is my first backend PR..so please take a code....i am open for suggestion 😄 |
environment/environment.php
Outdated
| $this->fromRootToFolder = $this->buildFromRootToFolder($this->sharedNodeId); | ||
|
|
||
| $this->folderName = $linkItem->getTarget(); | ||
| $this->userId = $origShareOwnerId; |
There was a problem hiding this comment.
I think this may be wrong. There is a difference between the share owner and the file owner
There was a problem hiding this comment.
Never mind.. there was duplication in the code left over from refactoring
|
Thanks for this. I'll take a better look tomorrow and provide some feedback, but that looks pretty good :). One thing though, try to configure your IDE so that you keep the formatting. |
|
Also, Try to fix the tests as it gives a good indication of if things still work as expected. |
middleware/envcheckmiddleware.php
Outdated
| /** @var IControllerMethodReflector */ | ||
| protected $reflector; | ||
| /** @var IManager */ | ||
| protected $shareManager; |
There was a problem hiding this comment.
Your identation seems off... tabs vs spaces?
There was a problem hiding this comment.
yes....i am correcting my ide settings. Also i am trying to update unit tests...
|
Added a todo list to the OP |
tahaalibra
force-pushed
the
share_20
branch
4 times, most recently
from
305ed51 to
470915f
Compare
| self::invokePrivate($this->middleware, 'checkSession', [$share]); | ||
| } | ||
|
|
||
| public function testCheckPasswordAfterValidPasswordEntry() { |
There was a problem hiding this comment.
unit test results in Error...can you help
There was a problem hiding this comment.
As discussed, I think this was just due to a typo in EnvCheckMiddleware
|
late night updates.....check if its working ok.... |
|
@tahaalibra - Will you have time to finish this at some point? |
|
@oparoz as i remember the last issue was that IShare was missing setId, it has been fixed in owncloud/core#23526, i am not sure whats remaining, as i remember the tests where running ok on my laptop.... i am currently away from my laptop, if its ok could you test this is PR |
|
There was the missing setId, but also a typo which made tests fail. |
|
Replaced by #668 |
5 participants
Fixes: #579
Licence: AGPL
Description
OCA\Gallery\Environment::setTokenBasedEnv and OCA\Files_Sharing\Controllers\ShareController::showShare are updated to use the share 2.0
this helps as now there is no need to check with checkLinkItemExists, checkLinkItemIsValid and checkItemType.
Important changes
Features
Does not alter any Feature
Caveats
Tested on
Todo
ShareManger->checkPassword($share, $password)Reviewers
@oparoz @rullzer