OCS API should return fancy OC url #14998
Conversation
| @@ -303,8 +303,7 @@ public static function createShare($params) { | |||
| break; | |||
| } | |||
| } | |||
| $url = \OCP\Util::linkToPublic('files&t='.$token); | |||
| $data['url'] = $url; // '&' gets encoded to $amp; | |||
| $data['url'] = \OCP\Util::linkToPublic('files') . '/' . $token; | |||
There was a problem hiding this comment.
Can you try to use the URLGenerator using the route? 😄
There was a problem hiding this comment.
My quick and dirty fix not good enough 😜 ? But you are right. I'll have a look later.
|
👍 |
|
Code looks good 👍 |
| @@ -303,8 +303,7 @@ public static function createShare($params) { | |||
| break; | |||
| } | |||
| } | |||
| $url = \OCP\Util::linkToPublic('files&t='.$token); | |||
| $data['url'] = $url; // '&' gets encoded to $amp; | |||
| $data['url'] = \OC::$server->getURLGenerator()->linkToRouteAbsolute('files_sharing.sharecontroller.showShare', ['token' => $token]); | |||
There was a problem hiding this comment.
this adds the hard dependency that the route name never changes - I'd really welcome a unit test which double checks that the proper url is being generated
There was a problem hiding this comment.
If an invalid route name is provided stuff breaks here. So a lot of unit tests already fail.
Do we really want a unit test to check for this the correct url? The whole point of the URLGenerator is so we can change the generated url as long as the route name stays the same.
The only thing that might make it break is if somebody swaps the names of two functions.
There was a problem hiding this comment.
What could be done is check for a somewhat valid url? So that it at least an absolute url and contains the token.
There was a problem hiding this comment.
I'd assert $data['url'] to match http://localhost/index.php/s/12345678
There was a problem hiding this comment.
which would fail on ~50% of the dev installations, instead you can use the makeAbsolute from the url generator to make the expected string adapt to the install
* Also use the URLGenerator
|
The inspection completed: 1 updated code elements |
|
Finally Jenkins is Happy with the unit test (see #15085 ) Review time :) |
|
👍 |
1 similar comment
|
👍 |
OCS API should return fancy OC url
Right now the OCS API still returns the old url for link shares. It should use the fancy new shorter url.
Pretty trivial fix - just a tiny thing that annoys me way more than it should