Folders: Update folder using app platform APIs #110449
Conversation
| labels: { | ||
| 'grafana.app/deprecatedInternalID': id ?? '123', | ||
| }, |
| 'grafana.app/updatedBy': 'user:2', | ||
| 'grafana.app/managedBy': 'user', | ||
| 'grafana.app/updatedTimestamp': '2024-01-01T00:00:00Z', | ||
| 'grafana.app/folder': folder.kind === 'folder' ? folder.parentUID : undefined, |
There was a problem hiding this comment.
the only annotation you will need/want to send is:
grafana.app/folder
the rest are managed internally
| uid: folder.uid, | ||
| creationTimestamp: '2023-01-01T00:00:00Z', |
There was a problem hiding this comment.
| uid: folder.uid, | |
| creationTimestamp: '2023-01-01T00:00:00Z', |
do not send uid or creationTimestamp -- they are handled server side
|
@ryantxu Thanks for taking a look! The code you commented on is actually for a mock API that is used in tests - so it's good that your concerns are all about seemingly doing things that only the API should worry about 🙌 It's just making mock handlers that we want to behave in the same way that the API should do. The implementation may not be perfect, and there is hardcoded data, but it's all for tests/mocking out so it isn't performing any faulty app logic (I hope!) |
|
|
||
| // save an existing folder (e.g. rename) | ||
| saveFolder: builder.mutation<FolderDTO, FolderDTO>({ | ||
| saveFolder: builder.mutation<FolderDTO, Pick<FolderDTO, 'uid' | 'title' | 'version' | 'parentUid'>>({ |
There was a problem hiding this comment.
The existing API didn't use any more than this, so we were requiring more properties to be passed in/boxing ourselves in to always having to use the full folder object
tomratcliffe
requested review from
Clarity-89,
aocenas,
eledobleefe and
samsch
and removed request for
a team
| } | ||
|
|
||
| export function useUpdateFolder() { | ||
| const [updateFolder, result] = useReplaceFolderMutation(); |
There was a problem hiding this comment.
Would probably just add a comment that replace and update are the same thing, otherwise this is a bit confusing.
There was a problem hiding this comment.
Based on conversations in #app-platform channel, I'll consolidate these to both use replace (perhaps in a follow up PR to tidy the hooks and tests up)
| @@ -0,0 +1,65 @@ | |||
| /** Expected constant response from `/api/search/sorting` */ | |||
| export const SORT_OPTIONS = { | |||
There was a problem hiding this comment.
This is an enterprise response, I think OSS only has alphabetical sort, would that be enough for testing?
There was a problem hiding this comment.
Thanks, I didn't know about that distinction. Will slim it down to just the alphabetical options
| /** Renders test component with a button that will create a new folder */ | ||
| export const setupCreateFolder = () => render(<TestCreationComponent />); | ||
|
|
||
| /** Renders test component with a button that will allows updating a folder */ |
There was a problem hiding this comment.
| /** Renders test component with a button that will allows updating a folder */ | |
| /** Renders test component with a button that will allow updating a folder */ |
|
|
||
| it('selecting an item hides the filters and shows the actions instead', async () => { | ||
| render(<BrowseDashboardsPage queryParams={{}} />); | ||
| const { user } = render(<BrowseDashboardsPage queryParams={{}} />); |
4 participants
What is this feature?
Migrates updateFolder logic to use app platform APIs when feature toggle is enabled
Does some additional simplification of tests and handlers to remove mocks
Why do we need this feature?
So we can consume app platform APIs for folder actions
Which issue(s) does this PR fix?:
Fixes #109538