-
-
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 2 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(); | ||
|
@@ -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' }); | ||
}); | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,9 @@ import { useCallback, MutableRefObject } from 'react'; | |
// @ts-ignore | ||
import { parse } from 'query-string'; | ||
import { useLocation } from 'react-router-dom'; | ||
import { Location } from 'history'; | ||
|
||
import { useAuthenticated } from '../../auth'; | ||
import { useCreate } from '../../dataProvider'; | ||
import { | ||
useNotify, | ||
|
@@ -49,20 +51,23 @@ export const useCreateController = < | |
props: CreateControllerProps = {} | ||
): CreateControllerResult<RecordType> => { | ||
const { | ||
record = {}, | ||
successMessage, | ||
disableAuthentication, | ||
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. You should update the Auth doc to mention this prop. Same for other controllers. |
||
onSuccess, | ||
onFailure, | ||
record, | ||
successMessage, | ||
transform, | ||
} = props; | ||
|
||
useAuthenticated({ enabled: !disableAuthentication }); | ||
const resource = useResourceContext(props); | ||
const { hasEdit, hasShow } = useResourceDefinition(props); | ||
const location = useLocation(); | ||
const translate = useTranslate(); | ||
const notify = useNotify(); | ||
const redirect = useRedirect(); | ||
const recordToUse = getRecord(location, record); | ||
const recordToUse = | ||
record ?? getRecordFromLocation(location) ?? emptyRecord; | ||
const version = useVersion(); | ||
|
||
if (process.env.NODE_ENV !== 'production' && successMessage) { | ||
|
@@ -190,12 +195,13 @@ export const useCreateController = < | |
export interface CreateControllerProps< | ||
RecordType extends Omit<Record, 'id'> = Record | ||
> { | ||
disableAuthentication?: boolean; | ||
record?: Partial<RecordType>; | ||
resource?: string; | ||
onSuccess?: OnSuccess; | ||
onFailure?: OnFailure; | ||
transform?: TransformData; | ||
successMessage?: string; | ||
transform?: TransformData; | ||
} | ||
|
||
export interface CreateControllerResult< | ||
|
@@ -230,7 +236,12 @@ export interface CreateControllerResult< | |
version: number; | ||
} | ||
|
||
export const getRecord = ({ state, search }, record: any = {}) => { | ||
const emptyRecord = {}; | ||
/** | ||
* Get the initial record from the location, whether it comes from the location | ||
* state or is serialized in the url search part. | ||
*/ | ||
export const getRecordFromLocation = ({ state, search }: Location) => { | ||
if (state && state.record) { | ||
return state.record; | ||
} | ||
|
@@ -252,7 +263,7 @@ export const getRecord = ({ state, search }, record: any = {}) => { | |
); | ||
} | ||
} | ||
return record; | ||
return null; | ||
}; | ||
|
||
const getDefaultRedirectRoute = (hasShow, hasEdit) => { | ||
|
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