gabotorresruiz commented on code in PR #43190:
URL: https://github.com/apache/superset/pull/43190#discussion_r3806865445


##########
superset-frontend/src/pages/DatasetList/DatasetList.subdirectory.test.tsx:
##########
@@ -115,6 +123,27 @@ test('explore link is single-prefixed under a subdirectory 
deployment', async ()
   expect(exploreLink.getAttribute('href')).not.toContain('/superset/superset');
 });
 
+test('legacy dashboard default URL uses the router basename once', async () => 
{
+  const dataset = {
+    ...mockDatasets[0],
+    explore_url: '/superset/dashboard/123/?standalone=1#section',

Review Comment:
   Just a small NIT, not a blocker: this test passes even without the fix. I 
verified it on this branch with `index.tsx` reverted to master: `stripAppRoot` 
already removes `/superset` here because the mocked app root is also 
`/superset`, so `normalizeLegacyDashboardUrl` never sees the legacy prefix and 
the assertion stays green. It still pins the single-prefix invariant, which is 
valuable, but it does not cover the new regex under a subdirectory deployment.
   
   If you change the input to the double-prefixed form a subdirectory user 
would actually have saved by pasting the full browser path, the test exercises 
the fix and fails without it (I ran both directions):
   
   ```ts
   explore_url: '/superset/superset/dashboard/123/?standalone=1#section',
   ```
   
   Same expected href, `stripAppRoot` eats the app root and the new 
normalization eats the legacy prefix.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to