-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove permissions prop injection from main views #6921
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,5 @@ | ||
import { useEffect } from 'react'; | ||
import useCheckAuth from './useCheckAuth'; | ||
|
||
const emptyParams = {}; | ||
import { useCheckAuth } from './useCheckAuth'; | ||
|
||
/** | ||
* Restrict access to authenticated users. | ||
|
@@ -28,9 +26,21 @@ const emptyParams = {}; | |
* </Admin> | ||
* ); | ||
*/ | ||
export default (params: any = emptyParams) => { | ||
export const useAuthenticated = <ParamsType = any>( | ||
options: UseAuthenticatedOptions<ParamsType> = {} | ||
) => { | ||
const { enabled = true, params = emptyParams } = options; | ||
const checkAuth = useCheckAuth(); | ||
useEffect(() => { | ||
checkAuth(params).catch(() => {}); | ||
}, [checkAuth, params]); | ||
if (enabled) { | ||
checkAuth(params).catch(() => {}); | ||
} | ||
}, [checkAuth, enabled, params]); | ||
}; | ||
|
||
export type UseAuthenticatedOptions<ParamsType> = { | ||
enabled?: boolean; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be great to have documentation of the new |
||
params?: ParamsType; | ||
}; | ||
|
||
const emptyParams = {}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,7 +40,7 @@ import useNotify from '../sideEffect/useNotify'; | |
* return authenticated ? <Bar /> : <BarNotAuthenticated />; | ||
* } // tip: use useAuthState() hook instead | ||
*/ | ||
const useCheckAuth = (): CheckAuth => { | ||
export const useCheckAuth = (): CheckAuth => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing export for type |
||
const authProvider = useAuthProvider(); | ||
const notify = useNotify(); | ||
const logout = useLogout(); | ||
|
@@ -90,7 +90,7 @@ const checkAuthWithoutAuthProvider = () => Promise.resolve(); | |
* | ||
* @return {Promise} Resolved to the authProvider response if the user passes the check, or rejected with an error otherwise | ||
*/ | ||
type CheckAuth = ( | ||
export type CheckAuth = ( | ||
params?: any, | ||
logoutOnFailure?: boolean, | ||
redirectTo?: string, | ||
|
@@ -104,5 +104,3 @@ const getErrorMessage = (error, defaultMessage) => | |
: typeof error === 'undefined' || !error.message | ||
? defaultMessage | ||
: error.message; | ||
|
||
export default useCheckAuth; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,6 @@ | ||
import { | ||
CreateControllerResult, | ||
useCreateController, | ||
} from './useCreateController'; | ||
|
||
export * from './CreateBase'; | ||
export * from './CreateContext'; | ||
export * from './CreateContextProvider'; | ||
export * from './CreateController'; | ||
|
||
export * from './useCreateContext'; | ||
export * from '../show/useShowContext'; | ||
|
||
// We don't want to export CreateProps and EditProps as they should | ||
// not be used outside ra-core, since it would conflict with ra-ui-materialui types, | ||
// hence the named imports/exports | ||
export type { CreateControllerResult as CreateControllerProps }; | ||
export { useCreateController }; | ||
export * from './useCreateController'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,60 +1,49 @@ | ||
import React from 'react'; | ||
import expect from 'expect'; | ||
import { act } from '@testing-library/react'; | ||
import { Location } from 'history'; | ||
|
||
import { getRecord } from './useCreateController'; | ||
import { getRecordFromLocation } from './useCreateController'; | ||
import { CreateController } from './CreateController'; | ||
import { renderWithRedux } from 'ra-test'; | ||
import { DataProviderContext } from '../../dataProvider'; | ||
import { DataProvider } from '../../types'; | ||
|
||
describe('useCreateController', () => { | ||
describe('getRecord', () => { | ||
const location = { | ||
describe('getRecordFromLocation', () => { | ||
const location: Location = { | ||
key: 'a_key', | ||
pathname: '/foo', | ||
search: undefined, | ||
state: undefined, | ||
hash: undefined, | ||
}; | ||
|
||
it('should return an empty record by default', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why did you remove this one ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because that's not the responsability of this function anymore |
||
expect(getRecord(location, undefined)).toEqual({}); | ||
}); | ||
|
||
it('should return location state record when set', () => { | ||
expect( | ||
getRecord( | ||
{ | ||
...location, | ||
state: { record: { foo: 'bar' } }, | ||
}, | ||
undefined | ||
) | ||
getRecordFromLocation({ | ||
...location, | ||
state: { record: { foo: 'bar' } }, | ||
}) | ||
).toEqual({ foo: 'bar' }); | ||
}); | ||
|
||
it('should return location search when set', () => { | ||
expect( | ||
getRecord( | ||
{ | ||
...location, | ||
search: '?source={"foo":"baz","array":["1","2"]}', | ||
}, | ||
undefined | ||
) | ||
getRecordFromLocation({ | ||
...location, | ||
search: '?source={"foo":"baz","array":["1","2"]}', | ||
}) | ||
).toEqual({ foo: 'baz', array: ['1', '2'] }); | ||
}); | ||
|
||
it('should return location state record when both state and search are set', () => { | ||
expect( | ||
getRecord( | ||
{ | ||
...location, | ||
state: { record: { foo: 'bar' } }, | ||
search: '?foo=baz', | ||
}, | ||
undefined | ||
) | ||
getRecordFromLocation({ | ||
...location, | ||
state: { record: { foo: 'bar' } }, | ||
search: '?foo=baz', | ||
}) | ||
).toEqual({ foo: 'bar' }); | ||
}); | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a breaking change for users who used to pass params, and should be mentioned in the upgrade guide