Expose navigation entries as API endpoint by juliusknorr · Pull Request #8095 · nextcloud/server

juliusknorr · 2018-01-29T12:01:26Z

This PR adds new endpoints to get the navigation entries for the app and settings menu, as discussed in #7893. This will make it possible for clients to fetch the list of available "web apps" but also use this in apps management to update the app menu when installing/uninstalling an app. I'll do that in a separate PR once this is merged.

@camilasan

Example request: apps

$ curl http://admin:admin@localhost:8140/index.php/core/navigation/apps 
[
  {
    "id": "files",
    "order": "0",
    "href": "/index.php/apps/files/",
    "icon": "/apps/files/img/app.svg",
    "type": "link",
    "name": "Dateien",
    "active": false,
    "classes": ""
  },
  {
    "id": "activity",
    "order": "1",
    "href": "/index.php/apps/activity/",
    "icon": "/apps/activity/img/activity.svg",
    "type": "link",
    "name": "Aktivität",
    "active": false,
    "classes": ""
  },
  {
    "id": "gallery",
    "order": 2,
    "href": "/index.php/apps/gallery/",
    "icon": "/apps/gallery/img/app.svg",
    "name": "Galerie",
    "active": false,
    "classes": "",
    "type": "link"
  },
  {
    "id": "deck",
    "order": 10,
    "href": "/index.php/apps/deck/",
    "icon": "/apps-extra/deck/img/deck.svg",
    "name": "Deck",
    "active": false,
    "classes": "",
    "type": "link"
  }
]

rullzer · 2018-01-29T12:49:27Z

Any reason not to expose this in the capabilities?

juliusknorr · 2018-01-29T12:59:37Z

Quoting @nickvergessen from #7893, and I think it makes sense to have a separate endpoint which can also be used for the apps management as well.

Not abuse the capabilities even more

nickvergessen · 2018-01-29T13:22:40Z

+use OCP\IRequest;
+use OCP\IURLGenerator;
+
+class NavigationController extends Controller {


Use OCSController please

nickvergessen · 2018-01-29T13:23:10Z

+		if ($absolute) {
+			$this->rewriteToAbsoluteUrls($navigation);
+		}
+		return new JSONResponse($navigation);


return new DataResponse($navigation);

nickvergessen · 2018-01-29T13:23:17Z

+		if ($absolute) {
+			$this->rewriteToAbsoluteUrls($navigation);
+		}
+		return new JSONResponse($navigation);


return new DataResponse($navigation);

nickvergessen · 2018-01-29T13:23:57Z

+	 *
+	 * @param array $navigation
+	 */
+	private function rewriteToAbsoluteUrls(array &$navigation) {


I'd prefer return over by-reference

nickvergessen · 2018-01-29T13:24:19Z

 		['name' => 'Preview#getPreviewByFileId', 'url' => '/core/preview', 'verb' => 'GET'],
 		['name' => 'Preview#getPreview', 'url' => '/core/preview.png', 'verb' => 'GET'],
+		['name' => 'Navigation#getAppsNavigation', 'url' => '/core/navigation/apps', 'verb' => 'GET'],
+		['name' => 'Navigation#getSettingsNavigation', 'url' => '/core/navigation/settings', 'verb' => 'GET'],


should be OCS routes, everything else is not to be used by clients

codecov · 2018-02-01T11:19:08Z

Codecov Report

Merging #8095 into master will increase coverage by 0.08%.
The diff coverage is 72.34%.

@@             Coverage Diff              @@
##             master    #8095      +/-   ##
============================================
+ Coverage     51.69%   51.78%   +0.08%     
- Complexity    25386    25395       +9     
============================================
  Files          1599     1600       +1     
  Lines         95109    95130      +21     
  Branches       1376     1376              
============================================
+ Hits          49163    49259      +96     
+ Misses        45946    45871      -75

Impacted Files	Coverage Δ	Complexity Δ
core/routes.php	`0% <ø> (ø)`	`0 <0> (ø)`	⬇️
lib/private/TemplateLayout.php	`0% <0%> (ø)`	`48 <0> (ø)`	⬇️
lib/private/legacy/app.php	`57.86% <0%> (+1.95%)`	`199 <0> (-8)`	⬇️
settings/ajax/navigationdetect.php	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
core/Controller/NavigationController.php	`100% <100%> (ø)`	`9 <9> (?)`
lib/private/NavigationManager.php	`77.55% <61.9%> (-1.84%)`	`51 <12> (+8)`
lib/private/Files/ObjectStore/Swift.php	`49.61% <0%> (+49.61%)`	`45% <0%> (ø)`	⬇️

juliusknorr · 2018-02-01T11:25:39Z

@nickvergessen Migrated it to be an OCSController. Any opinions about the endpoint naming? Should we add it to the /cloud OCS root? Or just have a separate core one (as it is currently)?

Implemented

rullzer · 2018-02-01T11:37:48Z

I'm fine with the url like that :)

nickvergessen · 2018-02-01T12:04:07Z

+	 * @param bool $absolute
+	 * @return DataResponse
+	 */
+	public function getAppsNavigation(bool $absolute = false) {


You can add : DataResponse too

Ah thanks, I somehow missed that.

rullzer · 2018-02-08T20:13:12Z

+	 */
+	private function rewriteToAbsoluteUrls(array $navigation): array {
+		foreach ($navigation as &$entry) {
+			if (0 !== strpos($entry['href'], $this->urlGenerator->getBaseUrl())) {


Shouldn't we do the same check for the icon? So that they can also be used?

rullzer · 2018-02-08T20:13:29Z

I rebased upon master and updated the autoloader.

rullzer

Cool stuff!

rullzer · 2018-02-08T20:14:15Z

 	public static function getNavigation() {
-		$entries = OC::$server->getNavigationManager()->getAll();
-		return self::proceedNavigation($entries);
+		return OC::$server->getNavigationManager()->getAll();


deprecate them so IDE's give warnings ;)

Fixed and also replaced the usage of those methods in the server.

rullzer · 2018-02-08T20:14:25Z

@@ -631,8 +605,7 @@ public static function getNavigation() {
 	 * entries are sorted by the key 'order' ascending.
 	 */
 	public static function getSettingsNavigation() {


Signed-off-by: Julius Härtl <[email protected]>

They might be useful when requesting the navigation from the clients Signed-off-by: Julius Härtl <[email protected]>

Signed-off-by: Julius Härtl <[email protected]>

Signed-off-by: Roeland Jago Douma <[email protected]>

Signed-off-by: Julius Härtl <[email protected]>

juliusknorr · 2018-02-12T16:31:29Z

Shouldn't we do the same check for the icon? So that they can also be used?

Sorry, I missed that comment. Not the icon url will also be absolute if requested. Ready for review now.

Signed-off-by: Julius Härtl <[email protected]>

camilasan · 2018-02-14T19:57:21Z

Could we merge it @nickvergessen ?

nickvergessen · 2018-02-15T09:29:31Z

+	 * @return array
+	 * @since 14.0.0
+	 */
+	public function getAll(string $type = 'link'): array;


does not match docs

nickvergessen · 2018-02-15T09:30:20Z

+	 * @param $list
+	 * @return mixed
+	 */
+	private function proceedNavigation($list) {


add type hint and return type, also in docs

Signed-off-by: Julius Härtl <[email protected]>

camilasan · 2018-02-19T15:02:01Z

Just in case someone else also needs it - this is the correct way of doing the request:

$ curl http://user:password@localhost/nextcloud/ocs/v2.php/core/navigation/apps

After the changes in the code, the first example given in the comments it is not valid anymore.

rullzer · 2018-02-19T15:04:03Z

@camilasan please use v2.php ;) That gives better http status codes.

juliusknorr added enhancement 3. to review Waiting for reviews labels Jan 29, 2018

juliusknorr added this to the Nextcloud 14 milestone Jan 29, 2018

juliusknorr requested review from nickvergessen and rullzer January 29, 2018 12:01

juliusknorr mentioned this pull request Jan 29, 2018

Move endpoints to from apps management apps to AppSettingsController #7931

Closed

nickvergessen previously requested changes Jan 29, 2018

View reviewed changes

juliusknorr added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jan 29, 2018

juliusknorr force-pushed the webapps-capabilities branch from fefea23 to b665eea Compare February 1, 2018 11:19

juliusknorr added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Feb 1, 2018

nickvergessen reviewed Feb 1, 2018

View reviewed changes

juliusknorr force-pushed the webapps-capabilities branch 2 times, most recently from a64c123 to f91e852 Compare February 1, 2018 15:34

rullzer force-pushed the webapps-capabilities branch from f91e852 to c7a965d Compare February 8, 2018 20:08

rullzer reviewed Feb 8, 2018

View reviewed changes

rullzer approved these changes Feb 8, 2018

View reviewed changes

rullzer reviewed Feb 8, 2018

View reviewed changes

juliusknorr added 3 commits February 12, 2018 17:22

Move navigation processing to the NavigationManager

864474b

Signed-off-by: Julius Härtl <[email protected]>

Make sure we test the proper order in the NavigationManagerTest

f9dbaf6

Signed-off-by: Julius Härtl <[email protected]>

Add navigation endpoint

96480af

Signed-off-by: Julius Härtl <[email protected]>

juliusknorr and others added 6 commits February 12, 2018 17:22

Add getAll to public INavigationManager

b6da8c5

Signed-off-by: Julius Härtl <[email protected]>

Add tests for NavigationController

6211d18

Signed-off-by: Julius Härtl <[email protected]>

Allow requesting absolute URLs

8ecac56

They might be useful when requesting the navigation from the clients Signed-off-by: Julius Härtl <[email protected]>

Move to OCS endpoint

922cf44

Signed-off-by: Julius Härtl <[email protected]>

Update autoloader

3952985

Signed-off-by: Roeland Jago Douma <[email protected]>

Deprecate static methods and use Navigation manager

ada5aed

Signed-off-by: Julius Härtl <[email protected]>

juliusknorr force-pushed the webapps-capabilities branch from 6a8f3de to a395d40 Compare February 12, 2018 16:30

rullzer approved these changes Feb 12, 2018

View reviewed changes

Also rewrite icon url

5a23b35

Signed-off-by: Julius Härtl <[email protected]>

juliusknorr force-pushed the webapps-capabilities branch from a395d40 to 5a23b35 Compare February 12, 2018 20:20

nickvergessen requested changes Feb 15, 2018

View reviewed changes

Update phpDoc comments and add more type hinting

e694b30

Signed-off-by: Julius Härtl <[email protected]>

nickvergessen approved these changes Feb 15, 2018

View reviewed changes

MorrisJobke approved these changes Feb 15, 2018

View reviewed changes

MorrisJobke merged commit 14bc9b1 into master Feb 15, 2018

MorrisJobke deleted the webapps-capabilities branch February 15, 2018 15:55

MorrisJobke mentioned this pull request Feb 15, 2018

Add endpoint to list apps enabled in the server and its urls #7893

Closed

Conversation

juliusknorr commented Jan 29, 2018

Uh oh!

rullzer commented Jan 29, 2018

Uh oh!

juliusknorr commented Jan 29, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Feb 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

juliusknorr commented Feb 1, 2018

Uh oh!

rullzer commented Feb 1, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rullzer commented Feb 8, 2018

Uh oh!

rullzer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juliusknorr commented Feb 12, 2018

Uh oh!

camilasan commented Feb 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

camilasan commented Feb 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rullzer commented Feb 19, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov Bot commented Feb 1, 2018 •

edited

Loading

camilasan commented Feb 19, 2018 •

edited

Loading