Custom page route causing 404

[ror3d]

Describe the bug

When creating a plugin and defining a route in a route.d/...yaml, if the route is too deep, it fails to be found and leads to 404 page.

To Reproduce

(I am putting the code for this with the :uuid parameter because this is how I have tested this to happen, because it is how I initially encountered the problem, but I believe without it a similar problem would occur)

1. Create two new test components in /usr/share/openmediavault/workbench/component.d: omv-services-test-link-page.yaml

   version: "1.0"
   type: component
   data:
     name: omv-services-test-link-page
     type: textPage
     config:
       title: "Test"
       buttons:
         - text: "Go To Test Page"
           url: "/services/test/foo/bar/baz/203d18f5-130b-430c-beb4-ca2efa8f406e"

   and the one that will 404 (using some form for some content) omv-services-test-text-page.yaml:

   version: "1.0"
   type: component
   data:
     name: omv-services-test-text-page
     type: textPage
     config:
       title: Test

2. Create routes to the test components in /usr/share/openmediavault/workbench/route.d: services.test.yaml:

   version: "1.0"
   type: route
   data:
     url: "/services/test"
     title: _("Test")
     component: omv-services-test-link-page

   services.test.text.yaml:

   version: "1.0"
   type: route
   data:
     url: "/services/test/foo/bar/baz/:uuid"
     title: _("Test")
     component: omv-services-test-text-page

3. Compile the new components with omv-mkworkbench all
4. Navigate to <server>/#/services/test
5. Click the button on the page to Go To Test Page
6. Notice the 404 page appears

Expected behavior

The text page should load normally

openmediavault Server:

• OS version: Linux omv-dev 6.1.0-23-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.99-1 (2024-07-15) x86_64 GNU/Linux
• openmediavault version: 7.4.5

Client:

• Device: desktop
• OS: windows
• Browser: Vivaldi
• Version 6.2

Additional context

I have traced the routing that happens in the webapp (this was quite painful) and determined that the issue is in the RouteConfigService.

It seems that if a base page exists, such as /services/test in this case, and the next page created under that branch too deep, by more than 2 more levels, then all but the last of those extra levels is dropped in the route structure that is created in RouteConfigService, so, in this case, /services/test/foo/bar/baz/:uuid would become /services/test/baz/:uuid.

I have been able to resolve this issue by properly following the recursive structure in the getParentNode function from the inject method of the RouteConfigService. That is, at the end of the if (!node.children) branch, add

if (segments.length) {
  return getParentNode(node.children, segments);
}

or by the patch

--- a/deb/openmediavault/workbench/src/app/core/services/route-config.service.ts
+++ b/deb/openmediavault/workbench/src/app/core/services/route-config.service.ts
@@ -190,6 +190,9 @@ export class RouteConfigService {
                 component: NavigationPageComponent
               });
             }
+            if (segments.length) {
+              return getParentNode(node.children, segments);
+            }
           }
           return node.children;
         };

[votdev]

I think the idea was to force plugin developers to create ALL routes they are using. This might looks like

version: "1.0"
type: route
data:
  url: "/services/test/foo"
  title: _("Foo")
  component: omv-blank-page

version: "1.0"
type: route
data:
  url: "/services/test/foo/bar"
  title: _("Bar")
  component: omv-blank-page

version: "1.0"
type: route
data:
  url: "/services/test/foo/bar/baz"
  title: _("Baz")
  component: omv-blank-page

[votdev]

I've added a draft PR to think about if this fix makes sense or if it contradicts the original idea how routes have to be configured.

[votdev]

The problem with the fix is that this automatic creation of unconfigured routes can lead to problems if two plugins configure the same routes. If a file is created for each route and sub-route, then it is ensured that only one plugin can configure this route because the Debian APT package manager throws an error at the latest when the other plugin is installed because a route config file is already under the control of another Debian package. Of course, this assumes that the file names for the route configs correspond to the common convention.

[ror3d]

I guess that would also make sense, although as long as plugins stay in a unique directory/subpath it shouldn't be a problem maybe? If it was a big concern, maybe the python script that compiles the routes into a single file could do some checking beforehand. It would be good if the use of the omv-blank-page and the need to create all the routes was documented.

I found this because I was adapting the snapraid plugin to use the snapraid-btrfs script that uses snapshots, and I changed a bit how the routes in that one work:

I have a /storage/snapraid-btrfs/arrays page that lists existing arrays, and you can access the disks in an array like /storage/snapraid-btrfs/array/disks/:array_uuid, which made sense to me that the "array" would be singular instead of plural. I guess for now I will create the /storage/snapraid-btrfs/array route to "lock" that path but it feels like the wrong way to fix it.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.

[github-actions[bot]]

This issue has been automatically closed because there has been no activity for 90 days. We are sorry that we haven't been able to prioritize it yet. Please feel free to reopen this issue or create a new one. Thank you!