[Large] Create logic for editing metadata #372
Conversation
aswallace
changed the title
aswallace
requested review from
SeanLeRoy,
kmitcham,
pgarrison and
BrianWhitneyAI
packages/core/services/FileService/DatabaseFileService/index.ts
Outdated
Show resolved
Hide resolved
| try { | ||
| await fileService.editFile( | ||
| fileId, | ||
| annotations, |
There was a problem hiding this comment.
I see... I think the request to get MMS "editable data" was to for insertion right here where this annotations parameter is. However, that is a very slow GET fetch so I don't think we should necessarily switch to that. Instead, can we just pass here the annotation we want to replace the values for? Lets check up on the way the PATCH method works we might be golden with just switching to that.
There was a problem hiding this comment.
Alright found the logic in MMS, this is the logical path:
https://github.com/AllenCellSoftware/metadata-management-service/blob/master/src/main/java/org/alleninstitute/aics/metadatamanagementservice/filemetadata/FileMetadataController.java#L92
https://github.com/AllenCellSoftware/metadata-management-service/blob/master/src/main/java/org/alleninstitute/aics/metadatamanagementservice/filemetadata/FileIngestionService.java#L101C13-L101C40
https://github.com/AllenCellSoftware/metadata-management-service/blob/master/src/main/java/org/alleninstitute/aics/metadatamanagementservice/filemetadata/FileIngestionService.java#L171-L205
So based on the last chunk of code there we actually want that boolean block of replaceOldValues to happen which does not happen with the PATCH and does happen with the PUT. Furthermore, it only replaces annotations that are included in the request with new values. So all to get this working that we need to do is for the annotations parameter in this BFF code assign annotations equal to just the annotation the user modified in the modal with the values they want to replace with from the modal. It seems like that may already be happening so everything might just be all good.
There was a problem hiding this comment.
Yes, currently we are only passing the annotations that we want to edit
|
Is it legal to delete an annotation? Or set the value to null? |
Technically yes, passing an empty array as the values effectively deletes annotations when you use the MMS |
packages/core/components/StatusMessage/StatusMessage.module.css
Outdated
Show resolved
Hide resolved
| const url = `${mmsBaseUrl}/${HttpFileService.BASE_EDIT_FILES_URL}/${fileId}`; | ||
| const annotations = Object.entries(annotationNameToValuesMap).map(([name, values]) => { | ||
| const annotationId = annotationNameToAnnotationMap?.[name].id; | ||
| if (!annotationId) { |
There was a problem hiding this comment.
it might be better to aggregate all missing annotations and throw a single error
packages/core/services/FileService/HttpFileService/test/HttpFileService.test.ts
Show resolved
Hide resolved
Adds editing logic for existing annotations for FMS and database (csv/duckdb) files. This reopens/continues Sean's work on the edit logic from #260 with minor updates to reflect codebase & MMS changes. The commit history will be a mess because of rebases, but the file changes are accurate.
This is a big PR, but the most important changes are the FileService additions in packages/core/services/FileService/DatabaseFileService/index.ts and packages/core/services/FileService/HttpFileService/index.ts. The logic for sending batches of edit requests via Redux is in packages/core/state/interaction/logics.ts. We also added a
putto theHttpFileService. Most other changes are to interfaces, abstract classes or tests.Testing
Notes:
Editing FMS files on web WILL NOT WORK unless you add a valid
"X-User-Id"to the headers in HttpServiceBase. I didn't commit this because I'm currently using my own user ID locally until we complete #341The UI occasionally takes a while to display correct data for edited FMS files, even though the edits go through to MMS immediately. I think this is an FMS/FES syncing issue rather than a BFF thing?