matebenyovszky commented on PR #39171: URL: https://github.com/apache/superset/pull/39171#issuecomment-4378900702
## 🐛 Bug: Frontend storage URLs don't match backend API routes Hi @michael-s-molina, great work on this PR! We're eager to use the Storage API for our extension work at [Integrity Authority Hungary](https://github.com/integrityauthority). While reviewing, I found a **breaking bug** introduced in commit `f08bcd4` ("Updates the API"): ### Problem The backend API routes were restructured to use the `/{publisher}/{name}/storage/{tier}/{key}` pattern: `python # Backend (api.py line 113) @expose("/<publisher>/<name>/storage/ephemeral/<key>", methods=("GET",)) # Results in: /api/v1/extensions/{publisher}/{name}/storage/ephemeral/{key} ` But the frontend `ephemeralState.ts` and `persistentState.ts` still use the **old** URL pattern: ` ypescript // Frontend (ephemeralState.ts line 30) const basePath = '/api/v1/extensions/storage/ephemeral'; const url = `//`; // Results in: /api/v1/extensions/storage/ephemeral/{extensionId}/{key} ` **All frontend storage API calls will 404** because the URL patterns don't match. ### Fix Split `extensionId` (format: `{publisher}.{name}`) on the first dot and build URLs using `/{publisher}/{name}/storage/...`: ` ypescript const dotIndex = extensionId.indexOf('.'); const publisher = encodeURIComponent(extensionId.slice(0, dotIndex)); const name = encodeURIComponent(extensionId.slice(dotIndex + 1)); const encodedKey = encodeURIComponent(key); const url = `/api/v1/extensions///storage/ephemeral/`; ` ### Additional: Missing unit tests I also wrote unit tests for all three storage modules (`localState`, `ephemeralState`, `persistentState`) which should help with the `codecov/project/core-packages-ts` check failure. ### Patch I've prepared a complete patch with the fix + tests (5 files, +495/-6 lines). Happy to provide it as a PR against your branch or as a downloadable patch file — let me know what works best! **Summary of changes:** - `ephemeralState.ts` — fix URL pattern to match backend `/{publisher}/{name}/storage/ephemeral/{key}` - `persistentState.ts` — fix URL pattern to match backend `/{publisher}/{name}/storage/persistent/{key}` - `localState.test.ts` — tests for createBrowserStorage (user-scoped, shared, isolation) - `ephemeralState.test.ts` — tests for createEphemeralState (CRUD, shared, URL encoding) - `persistentState.test.ts` — tests for createPersistentState (CRUD, shared, URL encoding, key length) Ping me if you'd like me to open a PR against your branch! 🙏 -- 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]
