fix(admin-settings): redirect when accessing /admin-settings#1337
fix(admin-settings): redirect when accessing /admin-settings#1337JammingBen merged 3 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes admin settings route handling by converting the /admin-settings redirect route to a component route with proper ability checking in the beforeEnter hook, allowing for proper user permission validation since the user is loaded at that point.
- Changes redirect route to component route with beforeEnter hook for admin settings
- Adds GlobalProperties type definition for better type safety across app properties
- Refactors multiple apps to use the new GlobalProperties type instead of generic ComponentCustomProperties
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/web-runtime/src/container/application/classic.ts | Updates type casting to use GlobalProperties for better type safety |
| packages/web-pkg/src/apps/types.ts | Adds comprehensive GlobalProperties interface and updates related type definitions |
| packages/web-app-search/src/index.ts | Refactors to use useGettext composable instead of dummy function |
| packages/web-app-files/tests/unit/index.spec.ts | Updates test mocks to use proper GlobalProperties typing |
| packages/web-app-files/src/index.ts | Updates navItems function signature and removes dummy gettext function |
| packages/web-app-admin-settings/tests/unit/index.spec.ts | Updates tests to reflect route change from redirect to beforeEnter hook |
| packages/web-app-admin-settings/src/index.ts | Changes root route from redirect to component with beforeEnter hook for proper ability checking |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| { | ||
| path: '/', | ||
| redirect: () => { | ||
| component: General, | ||
| beforeEnter: () => { |
There was a problem hiding this comment.
The route defines a component (General) but uses beforeEnter to redirect to other routes. This will cause the General component to be rendered briefly before the redirect occurs. Consider using a redirect route or a dedicated redirect component instead.
There was a problem hiding this comment.
I don't think so, the hook is called before entering the route and therefore mounting the component.
The route `/admin-settings` was a redirect route before, however, the user is not yet loaded when the `redirect` hook gets called. This means we can't do ability checks there. Changes the route to a normal component route so we can do the ability check in the `beforeEnter` hook.
JammingBen
force-pushed
the
fix/admin-settings-redirect
branch
from
8a42361 to
8032b6d
Compare
The route
/admin-settingswas a redirect route before, however, the user is not yet loaded when theredirecthook gets called. This means we can't do ability checks there. Changes the route to a normal component route so we can do the ability check in thebeforeEnterhook.Also adds the correct type for the global app properties in the course of this and refactors a few apps to use it.
fixes #1330