↔️ Define and implement strategy for persisting order of apps in top bar. (Database?) #40254

Activity

1. sorbaugh

   added

   enhancement

   ❄️ 2023-Winter

   on Sep 4, 2023
2. sorbaugh

   added this to the Nextcloud 28 milestone on Sep 4, 2023
3. AndyScherzinger

   changed the title [-]Define and implement strategy for persisting. (Database?)[/-] [+]↔️ Define and implement strategy for persisting. (Database?)[/+] on Sep 5, 2023
4. sorbaugh

   changed the title [-]↔️ Define and implement strategy for persisting. (Database?)[/-] [+]↔️ Define and implement strategy for persisting order of apps in top bar. (Database?)[/+] on Sep 5, 2023
5. sorbaugh

   assigned

   sorbaugh

   and

   come-nc

   and unassigned

   sorbaugh

   on Sep 7, 2023
6. come-nc commented on Sep 19, 2023

   

   come-nc

   on Sep 19, 2023

   Contributor

   More actions

   Pointers:

   Order is stored in info.xml for each app, in navigations/navigation/order.
   The method AppManager::getAppInfo is used to get information about each application. I think it would be the right place to alter the value of order for each navigation entry.
   https://github.com/nextcloud/server/blob/master/lib/private/App/AppManager.php#L710
7. come-nc commented on Sep 19, 2023

   

   come-nc

   on Sep 19, 2023

   Contributor

   More actions

   POC:

   diff --git a/lib/private/NavigationManager.php b/lib/private/NavigationManager.php
   index ac6f16190dc..31f6b74691a 100644
   --- a/lib/private/NavigationManager.php
   +++ b/lib/private/NavigationManager.php
   @@ -290,6 +290,7 @@ class NavigationManager implements INavigationManager {
                           $apps = $this->appManager->getInstalledApps();
                   }
    
   +               $user = $this->userSession->getUser();
                   foreach ($apps as $app) {
                           if (!$this->userSession->isLoggedIn() && !$this->appManager->isEnabledForUser($app, $this->userSession->getUser())) {
                                   continue;
   @@ -315,7 +316,8 @@ class NavigationManager implements INavigationManager {
                                   }
                                   $l = $this->l10nFac->get($app);
                                   $id = $nav['id'] ?? $app . ($key === 0 ? '' : $key);
   -                               $order = isset($nav['order']) ? $nav['order'] : 100;
   +                               $order = (int)$this->config->getUserValue($user?->getUID(), 'core', 'apporder_'.$app.'_'.$key, '0');
   +                               $order = $order ?: $nav['order'] ?? 100;
                                   $type = $nav['type'];
                                   $route = !empty($nav['route']) ? $this->urlGenerator->linkToRoute($nav['route']) : '';
                                   $icon = isset($nav['icon']) ? $nav['icon'] : 'app.svg';

   Note:
   This is in navigationManager, more logical.
   A configuration key is stored for each application navigation link entry, we may want to store an array for all of them instead.
   For each navigation link we store an integer, I think this will behave nice when installing a new application it will go at its usual place, and the user can go in the settings to move it.
8. come-nc commented on Sep 19, 2023

   

   come-nc

   on Sep 19, 2023

   Contributor

   More actions

   Note: defaultapp is already picked in core/defaultapp for each user, but I think there is no UI for it
9. come-nc commented on Sep 25, 2023

   

   come-nc

   on Sep 25, 2023

   Contributor

   More actions

   On the format stored in the user configuration:
   We need to store an integer for each app/key combination. We may use -1 as a special value later for hide.

   Basically 3 possibilities:

   1. We store a config key for each app/key combination, like in the patch above, under 'apporder_'.$app.'_'.$key
      • Pros: easy to set through occ with occ user:setting admin core apporder_files_0 5 for instance
      • Cons: Lot of clutter in user config keys, harder to cleanup, leftovers on app uninstall
   2. We store an array under apporder
      • Pros: Cleaner in the config table
      • Cons: json encoding/decoding needed. Harder to set through occ.
   3. We store an array for each app under apporder_app
      • I think the worst solution because it has all the cons and non of the pros basically

   I think all in all second option is the best one. If needed we can add occ command for this but it does not seem that important.
10. sorbaugh commented on Sep 25, 2023

    

    sorbaugh

    on Sep 25, 2023

    ContributorAuthor

    More actions

    @come-nc I agree that nr.2 seems best for the current scope 👍
11. susnux commented on Oct 9, 2023

    

    susnux

    on Oct 9, 2023

    Contributor

    More actions

    #40617
12. susnux

    closed this as completedon Oct 9, 2023