From 73438c7a32740a7874aa9463849f5b6bf8581ee0 Mon Sep 17 00:00:00 2001 From: Chen Wang Date: Mon, 2 Oct 2023 10:55:34 -0500 Subject: [PATCH 01/15] add endpoint and test --- backend/app/models/metadata.py | 20 ++-- backend/app/routers/metadata.py | 55 +++++++--- backend/app/tests/test_metadata.py | 49 +++++++-- frontend/src/actions/metadata.js | 169 ++++++++++++++++++++--------- 4 files changed, 208 insertions(+), 85 deletions(-) diff --git a/backend/app/models/metadata.py b/backend/app/models/metadata.py index faa34ca21..9e249432f 100644 --- a/backend/app/models/metadata.py +++ b/backend/app/models/metadata.py @@ -2,7 +2,7 @@ from datetime import datetime from typing import Optional, List, Union -from beanie import Document, PydanticObjectId +from beanie import Document from elasticsearch import Elasticsearch, NotFoundError from fastapi import HTTPException from pydantic import Field, validator, AnyUrl, BaseModel @@ -11,7 +11,6 @@ EventListenerIn, LegacyEventListenerIn, EventListenerOut, - ExtractorInfo, ) from app.models.mongomodel import MongoDBRef from app.models.users import UserOut @@ -114,7 +113,8 @@ class Settings: class MetadataDefinitionOut(MetadataDefinitionDB): - pass + class Config: + fields = {"id": "id"} def validate_definition(content: dict, metadata_def: MetadataDefinitionOut): @@ -248,10 +248,10 @@ class Config: async def validate_context( - content: dict, - definition: Optional[str] = None, - context_url: Optional[str] = None, - context: Optional[List[Union[dict, AnyUrl]]] = None, + content: dict, + definition: Optional[str] = None, + context_url: Optional[str] = None, + context: Optional[List[Union[dict, AnyUrl]]] = None, ): """Convenience function for making sure incoming metadata has valid definitions or resolvable context. @@ -269,9 +269,9 @@ async def validate_context( pass if definition is not None: if ( - md_def := await MetadataDefinitionDB.find_one( - MetadataDefinitionDB.name == definition - ) + md_def := await MetadataDefinitionDB.find_one( + MetadataDefinitionDB.name == definition + ) ) is not None: content = validate_definition(content, md_def) else: diff --git a/backend/app/routers/metadata.py b/backend/app/routers/metadata.py index 91ad816fc..0368a1057 100644 --- a/backend/app/routers/metadata.py +++ b/backend/app/routers/metadata.py @@ -26,8 +26,8 @@ @router.post("/definition", response_model=MetadataDefinitionOut) async def save_metadata_definition( - definition_in: MetadataDefinitionIn, - user=Depends(get_current_user), + definition_in: MetadataDefinitionIn, + user=Depends(get_current_user), ): existing = await MetadataDefinitionDB.find_one( MetadataDefinitionDB.name == definition_in.name @@ -45,10 +45,10 @@ async def save_metadata_definition( @router.get("/definition", response_model=List[MetadataDefinitionOut]) async def get_metadata_definition( - name: Optional[str] = None, - user=Depends(get_current_user), - skip: int = 0, - limit: int = 2, + name: Optional[str] = None, + user=Depends(get_current_user), + skip: int = 0, + limit: int = 2, ): if name is None: defs = await MetadataDefinitionDB.find().skip(skip).limit(limit).to_list() @@ -62,13 +62,40 @@ async def get_metadata_definition( return [mddef.dict() for mddef in defs] +@router.delete("/definition/{metadata_definition_id}", response_model=MetadataDefinitionOut) +async def delete_metadata_definition( + metadata_definition_id: str, + user=Depends(get_current_user), +): + """Delete metadata definition by specific ID.""" + mdd = await MetadataDefinitionDB.find_one(MetadataDefinitionDB.id == PyObjectId(metadata_definition_id)) + if mdd: + # Check if metadata using this definition exists + metadata_using_definition = await MetadataDB.find( + MetadataDB.definition == mdd.name + ).to_list() + + if len(metadata_using_definition) > 0: + raise HTTPException( + status_code=400, + detail=f"Metadata definition {metadata_definition_id} in use. " + f"You cannot delete it until all metadata records using it are deleted.", + ) + + # TODO: Refactor this with permissions checks etc. + await mdd.delete() + return mdd.dict() + else: + raise HTTPException(status_code=404, detail=f"Metadata definition {metadata_definition_id} not found") + + @router.patch("/{metadata_id}", response_model=MetadataOut) async def update_metadata( - metadata_in: MetadataPatch, - metadata_id: str, - es: Elasticsearch = Depends(dependencies.get_elasticsearchclient), - user=Depends(get_current_user), - allow: bool = Depends(MetadataAuthorization("editor")), + metadata_in: MetadataPatch, + metadata_id: str, + es: Elasticsearch = Depends(dependencies.get_elasticsearchclient), + user=Depends(get_current_user), + allow: bool = Depends(MetadataAuthorization("editor")), ): """Update metadata. Any fields provided in the contents JSON will be added or updated in the metadata. If context or agent should be changed, use PUT. @@ -86,9 +113,9 @@ async def update_metadata( @router.delete("/{metadata_id}") async def delete_metadata( - metadata_id: str, - user=Depends(get_current_user), - allow: bool = Depends(MetadataAuthorization("editor")), + metadata_id: str, + user=Depends(get_current_user), + allow: bool = Depends(MetadataAuthorization("editor")), ): """Delete metadata by specific ID.""" md = await MetadataDB.find_one(MetadataDB.id == PyObjectId(metadata_id)) diff --git a/backend/app/tests/test_metadata.py b/backend/app/tests/test_metadata.py index f68ce6826..fd83b85d3 100644 --- a/backend/app/tests/test_metadata.py +++ b/backend/app/tests/test_metadata.py @@ -74,7 +74,7 @@ def test_dataset_create_metadata_no_context(client: TestClient, headers: dict): assert response.status_code == 400 -def test_dataset_create_metadata_definition(client: TestClient, headers: dict): +def test_dataset_create_and_delete_metadata_definition_success(client: TestClient, headers: dict): # Post the definition itself response = client.post( f"{settings.API_V2_STR}/metadata/definition", @@ -82,8 +82,27 @@ def test_dataset_create_metadata_definition(client: TestClient, headers: dict): headers=headers, ) assert ( - response.status_code == 200 or response.status_code == 409 + response.status_code == 200 or response.status_code == 409 ) # 409 = definition already exists + metadata_definition_id = response.json().get("id") + + # Delete metadata definition + response = client.delete(f"{settings.API_V2_STR}/metadata/definition/{metadata_definition_id}", headers=headers) + assert response.status_code == 200 + assert response.json().get("id") == metadata_definition_id + + +def test_dataset_create_and_delete_metadata_definition_fail(client: TestClient, headers: dict): + # Post the definition itself + response = client.post( + f"{settings.API_V2_STR}/metadata/definition", + json=metadata_definition, + headers=headers, + ) + assert ( + response.status_code == 200 or response.status_code == 409 + ) # 409 = definition already exists + metadata_definition_id = response.json().get("id") # Create dataset and add metadata to it using new definition dataset_id = create_dataset(client, headers).get("id") @@ -105,6 +124,12 @@ def test_dataset_create_metadata_definition(client: TestClient, headers: dict): ) assert response.status_code == 409 + # Delete metadata definition + response = client.delete(f"{settings.API_V2_STR}/metadata/definition/{metadata_definition_id}", headers=headers) + assert response.status_code == 400 + assert response.json()["detail"] == f"Metadata definition {metadata_definition_id} in use. " \ + f"You cannot delete it until all metadata records using it are deleted." + @pytest.mark.asyncio async def test_dataset_patch_metadata_definition(client: TestClient, headers: dict): @@ -115,7 +140,7 @@ async def test_dataset_patch_metadata_definition(client: TestClient, headers: di headers=headers, ) assert ( - response.status_code == 200 or response.status_code == 409 + response.status_code == 200 or response.status_code == 409 ) # 409 = definition already exists # Create dataset and add metadata to it using new definition @@ -138,10 +163,10 @@ async def test_dataset_patch_metadata_definition(client: TestClient, headers: di metadata_query.append({"query": {"match": {"metadata.latitude": "24.4"}}}) result = search_index(es, settings.elasticsearch_index, metadata_query) assert ( - result.body["responses"][0]["hits"]["hits"][0]["_source"]["metadata"][0][ - "latitude" - ] - == 24.4 + result.body["responses"][0]["hits"]["hits"][0]["_source"]["metadata"][0][ + "latitude" + ] + == 24.4 ) @@ -169,10 +194,10 @@ async def test_dataset_create_metadata_context_url(client: TestClient, headers: ) result = search_index(es, settings.elasticsearch_index, metadata_query) assert ( - result.body["responses"][0]["hits"]["hits"][0]["_source"]["metadata"][0][ - "alternateName" - ] - == "different name" + result.body["responses"][0]["hits"]["hits"][0]["_source"]["metadata"][0][ + "alternateName" + ] + == "different name" ) @@ -218,7 +243,7 @@ def test_file_create_metadata_definition(client: TestClient, headers: dict): headers=headers, ) assert ( - response.status_code == 200 or response.status_code == 409 + response.status_code == 200 or response.status_code == 409 ) # 409 = definition already exists # Create dataset and add metadata to it using new definition diff --git a/frontend/src/actions/metadata.js b/frontend/src/actions/metadata.js index fa3b2445f..7dc2bf313 100644 --- a/frontend/src/actions/metadata.js +++ b/frontend/src/actions/metadata.js @@ -1,173 +1,244 @@ -import {handleErrors} from "./common"; -import {V2} from "../openapi"; +import { handleErrors } from "./common"; +import { V2 } from "../openapi"; export const RECEIVE_METADATA_DEFINITIONS = "RECEIVE_METADATA_DEFINITIONS"; -export function fetchMetadataDefinitions(name=null, skip=0, limit=10){ + +export function fetchMetadataDefinitions(name = null, skip = 0, limit = 10) { return (dispatch) => { // TODO need to think pagination - return V2.MetadataService.getMetadataDefinitionApiV2MetadataDefinitionGet(name, skip, limit) - .then(json => { + return V2.MetadataService.getMetadataDefinitionApiV2MetadataDefinitionGet( + name, + skip, + limit + ) + .then((json) => { dispatch({ type: RECEIVE_METADATA_DEFINITIONS, metadataDefinitionList: json, receivedAt: Date.now(), }); }) - .catch(reason => { - dispatch(handleErrors(reason, fetchMetadataDefinitions(name, skip, limit))); + .catch((reason) => { + dispatch( + handleErrors(reason, fetchMetadataDefinitions(name, skip, limit)) + ); }); }; } export const SAVE_METADATA_DEFINITIONS = "SAVE_METADATA_DEFINITIONS"; -export function postMetadataDefinitions(metadataDefinition){ + +export function postMetadataDefinitions(metadataDefinition) { return (dispatch) => { - return V2.MetadataService.saveMetadataDefinitionApiV2MetadataDefinitionPost(metadataDefinition) - .then(json => { + return V2.MetadataService.saveMetadataDefinitionApiV2MetadataDefinitionPost( + metadataDefinition + ) + .then((json) => { dispatch({ type: SAVE_METADATA_DEFINITIONS, metadataDefinitionList: json, receivedAt: Date.now(), }); }) - .catch(reason => { - dispatch(handleErrors(reason, postMetadataDefinitions(metadataDefinition))); + .catch((reason) => { + dispatch( + handleErrors(reason, postMetadataDefinitions(metadataDefinition)) + ); }); }; } +export const DELETE_METADATA_DEFINITIONS = "DELETE_METADATA_DEFINITIONS"; + +export function deleteMetadataDefinitions(metadataDefinition) { + return (dispatch) => { + return V2.MetadataService.( + metadataDefinition + ) + .then((json) => { + dispatch({ + type: DELETE_METADATA_DEFINITIONS, + metadataDefinitionList: json, + receivedAt: Date.now(), + }); + }) + .catch((reason) => { + dispatch( + handleErrors(reason, deleteMetadataDefinitions(metadataDefinition)) + ); + }); + }; +} export const RECEIVE_DATASET_METADATA = "RECEIVE_DATASET_METADATA"; -export function fetchDatasetMetadata(datasetId){ + +export function fetchDatasetMetadata(datasetId) { return (dispatch) => { - return V2.MetadataService.getDatasetMetadataApiV2DatasetsDatasetIdMetadataGet(datasetId) - .then(json => { + return V2.MetadataService.getDatasetMetadataApiV2DatasetsDatasetIdMetadataGet( + datasetId + ) + .then((json) => { dispatch({ type: RECEIVE_DATASET_METADATA, metadataList: json, receivedAt: Date.now(), }); }) - .catch(reason => { + .catch((reason) => { dispatch(handleErrors(reason, fetchDatasetMetadata(datasetId))); }); }; } export const RECEIVE_FILE_METADATA = "RECEIVE_FILE_METADATA"; -export function fetchFileMetadata(fileId, version){ + +export function fetchFileMetadata(fileId, version) { return (dispatch) => { - return V2.MetadataService.getFileMetadataApiV2FilesFileIdMetadataGet(fileId, version, false) - .then(json => { + return V2.MetadataService.getFileMetadataApiV2FilesFileIdMetadataGet( + fileId, + version, + false + ) + .then((json) => { dispatch({ type: RECEIVE_FILE_METADATA, metadataList: json, receivedAt: Date.now(), }); }) - .catch(reason => { + .catch((reason) => { dispatch(handleErrors(reason, fetchFileMetadata(fileId, version))); }); }; } export const POST_DATASET_METADATA = "POST_DATASET_METADATA"; -export function postDatasetMetadata(datasetId, metadata){ + +export function postDatasetMetadata(datasetId, metadata) { return (dispatch) => { - return V2.MetadataService.addDatasetMetadataApiV2DatasetsDatasetIdMetadataPost(datasetId, metadata) - .then(json =>{ + return V2.MetadataService.addDatasetMetadataApiV2DatasetsDatasetIdMetadataPost( + datasetId, + metadata + ) + .then((json) => { dispatch({ type: POST_DATASET_METADATA, metadata: json, receivedAt: Date.now(), }); }) - .catch(reason => { - dispatch(handleErrors(reason, postDatasetMetadata(datasetId, metadata))); + .catch((reason) => { + dispatch( + handleErrors(reason, postDatasetMetadata(datasetId, metadata)) + ); }); }; } export const POST_FILE_METADATA = "POST_FILE_METADATA"; -export function postFileMetadata(fileId, metadata){ + +export function postFileMetadata(fileId, metadata) { return (dispatch) => { - return V2.MetadataService.addFileMetadataApiV2FilesFileIdMetadataPost(fileId, metadata) - .then(json =>{ + return V2.MetadataService.addFileMetadataApiV2FilesFileIdMetadataPost( + fileId, + metadata + ) + .then((json) => { dispatch({ type: POST_FILE_METADATA, metadata: json, receivedAt: Date.now(), }); }) - .catch(reason => { + .catch((reason) => { dispatch(handleErrors(reason, postFileMetadata(fileId, metadata))); }); }; } export const DELETE_DATASET_METADATA = "DELETE_DATASET_METADATA"; -export function deleteDatasetMetadata(datasetId, metadata){ - return (dispatch) =>{ - return V2.MetadataService.deleteDatasetMetadataApiV2DatasetsDatasetIdMetadataDelete(datasetId, metadata). - then(json => { + +export function deleteDatasetMetadata(datasetId, metadata) { + return (dispatch) => { + return V2.MetadataService.deleteDatasetMetadataApiV2DatasetsDatasetIdMetadataDelete( + datasetId, + metadata + ) + .then((json) => { dispatch({ type: DELETE_DATASET_METADATA, metadata: json, receivedAt: Date.now(), }); }) - .catch(reason => { - dispatch(handleErrors(reason, deleteDatasetMetadata(datasetId, metadata))); + .catch((reason) => { + dispatch( + handleErrors(reason, deleteDatasetMetadata(datasetId, metadata)) + ); }); }; } export const DELETE_FILE_METADATA = "DELETE_FILE_METADATA"; -export function deleteFileMetadata(fileId, metadata){ - return (dispatch) =>{ - return V2.MetadataService.deleteFileMetadataApiV2FilesFileIdMetadataDelete(fileId, metadata). - then(json => { + +export function deleteFileMetadata(fileId, metadata) { + return (dispatch) => { + return V2.MetadataService.deleteFileMetadataApiV2FilesFileIdMetadataDelete( + fileId, + metadata + ) + .then((json) => { dispatch({ type: DELETE_FILE_METADATA, metadata: json, receivedAt: Date.now(), }); }) - .catch(reason => { + .catch((reason) => { dispatch(handleErrors(reason, deleteFileMetadata(fileId, metadata))); }); }; } export const UPDATE_DATASET_METADATA = "UPDATE_DATASET_METADATA"; -export function patchDatasetMetadata(datasetId, metadata){ + +export function patchDatasetMetadata(datasetId, metadata) { return (dispatch) => { - return V2.MetadataService.updateDatasetMetadataApiV2DatasetsDatasetIdMetadataPatch(datasetId, metadata) - .then(json => { + return V2.MetadataService.updateDatasetMetadataApiV2DatasetsDatasetIdMetadataPatch( + datasetId, + metadata + ) + .then((json) => { dispatch({ type: UPDATE_DATASET_METADATA, metadata: json, receivedAt: Date.now(), }); }) - .catch(reason => { - dispatch(handleErrors(reason, patchDatasetMetadata(datasetId, metadata))); + .catch((reason) => { + dispatch( + handleErrors(reason, patchDatasetMetadata(datasetId, metadata)) + ); }); }; } export const UPDATE_FILE_METADATA = "UPDATE_FILE_METADATA"; -export function patchFileMetadata(fileId, metadata){ + +export function patchFileMetadata(fileId, metadata) { return (dispatch) => { - return V2.MetadataService.updateFileMetadataApiV2FilesFileIdMetadataPatch(fileId, metadata) - .then(json => { + return V2.MetadataService.updateFileMetadataApiV2FilesFileIdMetadataPatch( + fileId, + metadata + ) + .then((json) => { dispatch({ type: UPDATE_FILE_METADATA, metadata: json, receivedAt: Date.now(), }); }) - .catch(reason => { + .catch((reason) => { dispatch(handleErrors(reason, patchFileMetadata(fileId, metadata))); }); }; From aec39f97ba6b11789203e3d85c30f90cc900876a Mon Sep 17 00:00:00 2001 From: Chen Wang Date: Mon, 2 Oct 2023 10:56:36 -0500 Subject: [PATCH 02/15] codegen and black --- backend/app/models/metadata.py | 14 +++--- backend/app/routers/metadata.py | 47 ++++++++++-------- backend/app/tests/test_metadata.py | 49 ++++++++++++------- .../v2/models/MetadataDefinitionOut.ts | 2 +- .../openapi/v2/services/MetadataService.ts | 19 +++++++ 5 files changed, 85 insertions(+), 46 deletions(-) diff --git a/backend/app/models/metadata.py b/backend/app/models/metadata.py index 9e249432f..f60e5d4b8 100644 --- a/backend/app/models/metadata.py +++ b/backend/app/models/metadata.py @@ -248,10 +248,10 @@ class Config: async def validate_context( - content: dict, - definition: Optional[str] = None, - context_url: Optional[str] = None, - context: Optional[List[Union[dict, AnyUrl]]] = None, + content: dict, + definition: Optional[str] = None, + context_url: Optional[str] = None, + context: Optional[List[Union[dict, AnyUrl]]] = None, ): """Convenience function for making sure incoming metadata has valid definitions or resolvable context. @@ -269,9 +269,9 @@ async def validate_context( pass if definition is not None: if ( - md_def := await MetadataDefinitionDB.find_one( - MetadataDefinitionDB.name == definition - ) + md_def := await MetadataDefinitionDB.find_one( + MetadataDefinitionDB.name == definition + ) ) is not None: content = validate_definition(content, md_def) else: diff --git a/backend/app/routers/metadata.py b/backend/app/routers/metadata.py index 0368a1057..3c23e8744 100644 --- a/backend/app/routers/metadata.py +++ b/backend/app/routers/metadata.py @@ -26,8 +26,8 @@ @router.post("/definition", response_model=MetadataDefinitionOut) async def save_metadata_definition( - definition_in: MetadataDefinitionIn, - user=Depends(get_current_user), + definition_in: MetadataDefinitionIn, + user=Depends(get_current_user), ): existing = await MetadataDefinitionDB.find_one( MetadataDefinitionDB.name == definition_in.name @@ -45,10 +45,10 @@ async def save_metadata_definition( @router.get("/definition", response_model=List[MetadataDefinitionOut]) async def get_metadata_definition( - name: Optional[str] = None, - user=Depends(get_current_user), - skip: int = 0, - limit: int = 2, + name: Optional[str] = None, + user=Depends(get_current_user), + skip: int = 0, + limit: int = 2, ): if name is None: defs = await MetadataDefinitionDB.find().skip(skip).limit(limit).to_list() @@ -62,13 +62,17 @@ async def get_metadata_definition( return [mddef.dict() for mddef in defs] -@router.delete("/definition/{metadata_definition_id}", response_model=MetadataDefinitionOut) +@router.delete( + "/definition/{metadata_definition_id}", response_model=MetadataDefinitionOut +) async def delete_metadata_definition( - metadata_definition_id: str, - user=Depends(get_current_user), + metadata_definition_id: str, + user=Depends(get_current_user), ): """Delete metadata definition by specific ID.""" - mdd = await MetadataDefinitionDB.find_one(MetadataDefinitionDB.id == PyObjectId(metadata_definition_id)) + mdd = await MetadataDefinitionDB.find_one( + MetadataDefinitionDB.id == PyObjectId(metadata_definition_id) + ) if mdd: # Check if metadata using this definition exists metadata_using_definition = await MetadataDB.find( @@ -79,23 +83,26 @@ async def delete_metadata_definition( raise HTTPException( status_code=400, detail=f"Metadata definition {metadata_definition_id} in use. " - f"You cannot delete it until all metadata records using it are deleted.", + f"You cannot delete it until all metadata records using it are deleted.", ) # TODO: Refactor this with permissions checks etc. await mdd.delete() return mdd.dict() else: - raise HTTPException(status_code=404, detail=f"Metadata definition {metadata_definition_id} not found") + raise HTTPException( + status_code=404, + detail=f"Metadata definition {metadata_definition_id} not found", + ) @router.patch("/{metadata_id}", response_model=MetadataOut) async def update_metadata( - metadata_in: MetadataPatch, - metadata_id: str, - es: Elasticsearch = Depends(dependencies.get_elasticsearchclient), - user=Depends(get_current_user), - allow: bool = Depends(MetadataAuthorization("editor")), + metadata_in: MetadataPatch, + metadata_id: str, + es: Elasticsearch = Depends(dependencies.get_elasticsearchclient), + user=Depends(get_current_user), + allow: bool = Depends(MetadataAuthorization("editor")), ): """Update metadata. Any fields provided in the contents JSON will be added or updated in the metadata. If context or agent should be changed, use PUT. @@ -113,9 +120,9 @@ async def update_metadata( @router.delete("/{metadata_id}") async def delete_metadata( - metadata_id: str, - user=Depends(get_current_user), - allow: bool = Depends(MetadataAuthorization("editor")), + metadata_id: str, + user=Depends(get_current_user), + allow: bool = Depends(MetadataAuthorization("editor")), ): """Delete metadata by specific ID.""" md = await MetadataDB.find_one(MetadataDB.id == PyObjectId(metadata_id)) diff --git a/backend/app/tests/test_metadata.py b/backend/app/tests/test_metadata.py index fd83b85d3..77dd6bde9 100644 --- a/backend/app/tests/test_metadata.py +++ b/backend/app/tests/test_metadata.py @@ -74,7 +74,9 @@ def test_dataset_create_metadata_no_context(client: TestClient, headers: dict): assert response.status_code == 400 -def test_dataset_create_and_delete_metadata_definition_success(client: TestClient, headers: dict): +def test_dataset_create_and_delete_metadata_definition_success( + client: TestClient, headers: dict +): # Post the definition itself response = client.post( f"{settings.API_V2_STR}/metadata/definition", @@ -82,17 +84,22 @@ def test_dataset_create_and_delete_metadata_definition_success(client: TestClien headers=headers, ) assert ( - response.status_code == 200 or response.status_code == 409 + response.status_code == 200 or response.status_code == 409 ) # 409 = definition already exists metadata_definition_id = response.json().get("id") # Delete metadata definition - response = client.delete(f"{settings.API_V2_STR}/metadata/definition/{metadata_definition_id}", headers=headers) + response = client.delete( + f"{settings.API_V2_STR}/metadata/definition/{metadata_definition_id}", + headers=headers, + ) assert response.status_code == 200 assert response.json().get("id") == metadata_definition_id -def test_dataset_create_and_delete_metadata_definition_fail(client: TestClient, headers: dict): +def test_dataset_create_and_delete_metadata_definition_fail( + client: TestClient, headers: dict +): # Post the definition itself response = client.post( f"{settings.API_V2_STR}/metadata/definition", @@ -100,7 +107,7 @@ def test_dataset_create_and_delete_metadata_definition_fail(client: TestClient, headers=headers, ) assert ( - response.status_code == 200 or response.status_code == 409 + response.status_code == 200 or response.status_code == 409 ) # 409 = definition already exists metadata_definition_id = response.json().get("id") @@ -125,10 +132,16 @@ def test_dataset_create_and_delete_metadata_definition_fail(client: TestClient, assert response.status_code == 409 # Delete metadata definition - response = client.delete(f"{settings.API_V2_STR}/metadata/definition/{metadata_definition_id}", headers=headers) + response = client.delete( + f"{settings.API_V2_STR}/metadata/definition/{metadata_definition_id}", + headers=headers, + ) assert response.status_code == 400 - assert response.json()["detail"] == f"Metadata definition {metadata_definition_id} in use. " \ - f"You cannot delete it until all metadata records using it are deleted." + assert ( + response.json()["detail"] + == f"Metadata definition {metadata_definition_id} in use. " + f"You cannot delete it until all metadata records using it are deleted." + ) @pytest.mark.asyncio @@ -140,7 +153,7 @@ async def test_dataset_patch_metadata_definition(client: TestClient, headers: di headers=headers, ) assert ( - response.status_code == 200 or response.status_code == 409 + response.status_code == 200 or response.status_code == 409 ) # 409 = definition already exists # Create dataset and add metadata to it using new definition @@ -163,10 +176,10 @@ async def test_dataset_patch_metadata_definition(client: TestClient, headers: di metadata_query.append({"query": {"match": {"metadata.latitude": "24.4"}}}) result = search_index(es, settings.elasticsearch_index, metadata_query) assert ( - result.body["responses"][0]["hits"]["hits"][0]["_source"]["metadata"][0][ - "latitude" - ] - == 24.4 + result.body["responses"][0]["hits"]["hits"][0]["_source"]["metadata"][0][ + "latitude" + ] + == 24.4 ) @@ -194,10 +207,10 @@ async def test_dataset_create_metadata_context_url(client: TestClient, headers: ) result = search_index(es, settings.elasticsearch_index, metadata_query) assert ( - result.body["responses"][0]["hits"]["hits"][0]["_source"]["metadata"][0][ - "alternateName" - ] - == "different name" + result.body["responses"][0]["hits"]["hits"][0]["_source"]["metadata"][0][ + "alternateName" + ] + == "different name" ) @@ -243,7 +256,7 @@ def test_file_create_metadata_definition(client: TestClient, headers: dict): headers=headers, ) assert ( - response.status_code == 200 or response.status_code == 409 + response.status_code == 200 or response.status_code == 409 ) # 409 = definition already exists # Create dataset and add metadata to it using new definition diff --git a/frontend/src/openapi/v2/models/MetadataDefinitionOut.ts b/frontend/src/openapi/v2/models/MetadataDefinitionOut.ts index 89757ca72..8b1791b0e 100644 --- a/frontend/src/openapi/v2/models/MetadataDefinitionOut.ts +++ b/frontend/src/openapi/v2/models/MetadataDefinitionOut.ts @@ -24,6 +24,6 @@ export type MetadataDefinitionOut = { context?: Array; context_url?: string; fields: Array; - _id?: string; + id?: string; creator: UserOut; } diff --git a/frontend/src/openapi/v2/services/MetadataService.ts b/frontend/src/openapi/v2/services/MetadataService.ts index f66c33559..0012839b7 100644 --- a/frontend/src/openapi/v2/services/MetadataService.ts +++ b/frontend/src/openapi/v2/services/MetadataService.ts @@ -61,6 +61,25 @@ export class MetadataService { }); } + /** + * Delete Metadata Definition + * Delete metadata definition by specific ID. + * @param metadataDefinitionId + * @returns MetadataDefinitionOut Successful Response + * @throws ApiError + */ + public static deleteMetadataDefinitionApiV2MetadataDefinitionMetadataDefinitionIdDelete( + metadataDefinitionId: string, + ): CancelablePromise { + return __request({ + method: 'DELETE', + path: `/api/v2/metadata/definition/${metadataDefinitionId}`, + errors: { + 422: `Validation Error`, + }, + }); + } + /** * Delete Metadata * Delete metadata by specific ID. From e8a92129ad641bdc9c9b21e9011a2fedeaa0b105 Mon Sep 17 00:00:00 2001 From: Chen Wang Date: Mon, 2 Oct 2023 11:00:06 -0500 Subject: [PATCH 03/15] add action --- frontend/src/actions/metadata.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/frontend/src/actions/metadata.js b/frontend/src/actions/metadata.js index 7dc2bf313..e35213dd0 100644 --- a/frontend/src/actions/metadata.js +++ b/frontend/src/actions/metadata.js @@ -48,23 +48,23 @@ export function postMetadataDefinitions(metadataDefinition) { }; } -export const DELETE_METADATA_DEFINITIONS = "DELETE_METADATA_DEFINITIONS"; +export const DELETE_METADATA_DEFINITION = "DELETE_METADATA_DEFINITION"; -export function deleteMetadataDefinitions(metadataDefinition) { +export function deleteMetadataDefinition(metadataDefinitionId) { return (dispatch) => { - return V2.MetadataService.( - metadataDefinition + return V2.MetadataService.deleteMetadataDefinitionApiV2MetadataDefinitionMetadataDefinitionIdDelete( + metadataDefinitionId ) .then((json) => { dispatch({ - type: DELETE_METADATA_DEFINITIONS, - metadataDefinitionList: json, + type: DELETE_METADATA_DEFINITION, + metadataDefinition: json, receivedAt: Date.now(), }); }) .catch((reason) => { dispatch( - handleErrors(reason, deleteMetadataDefinitions(metadataDefinition)) + handleErrors(reason, deleteMetadataDefinition(metadataDefinitionId)) ); }); }; From d40758b60710b88dfecce3cf60085f10b615ff3b Mon Sep 17 00:00:00 2001 From: Chen Wang Date: Mon, 2 Oct 2023 11:13:28 -0500 Subject: [PATCH 04/15] add search and get endpoint --- backend/app/routers/metadata.py | 46 +++++++++++++++++ .../openapi/v2/services/MetadataService.ts | 50 +++++++++++++++++++ 2 files changed, 96 insertions(+) diff --git a/backend/app/routers/metadata.py b/backend/app/routers/metadata.py index 3c23e8744..62fc338a1 100644 --- a/backend/app/routers/metadata.py +++ b/backend/app/routers/metadata.py @@ -1,5 +1,8 @@ from typing import Optional, List +from beanie import PydanticObjectId +from beanie.odm.operators.find.evaluation import RegEx +from beanie.odm.operators.find.logical import Or from elasticsearch import Elasticsearch from fastapi import ( APIRouter, @@ -96,6 +99,49 @@ async def delete_metadata_definition( ) +@router.get("/search/{search_term}", response_model=List[MetadataDefinitionOut]) +async def search_metadata_definition( + search_term: str, + skip: int = 0, + limit: int = 10, + user=Depends(get_current_user), +): + """Search all metadata definition in the db based on text. + + Arguments: + text -- any text matching name or description + skip -- number of initial records to skip (i.e. for pagination) + limit -- restrict number of records to be returned (i.e. for pagination) + """ + + mdds = await MetadataDefinitionDB.find( + Or( + RegEx(field=MetadataDefinitionDB.name, pattern=search_term), + RegEx(field=MetadataDefinitionDB.description, pattern=search_term), + RegEx(field=MetadataDefinitionDB.context, pattern=search_term), + ), + skip=skip, + limit=limit, + ).to_list() + + return [mdd.dict() for mdd in mdds] + + +@router.get("/{metadata_definition_id}", response_model=MetadataDefinitionOut) +async def get_metadata_definition( + metadata_definition_id: str, + user=Depends(get_current_user), +): + if ( + mdd := await MetadataDefinitionDB.get(PydanticObjectId(metadata_definition_id)) + ) is not None: + return mdd.dict() + raise HTTPException( + status_code=404, + detail=f"Metadata definition {metadata_definition_id} not found", + ) + + @router.patch("/{metadata_id}", response_model=MetadataOut) async def update_metadata( metadata_in: MetadataPatch, diff --git a/frontend/src/openapi/v2/services/MetadataService.ts b/frontend/src/openapi/v2/services/MetadataService.ts index 0012839b7..58077b673 100644 --- a/frontend/src/openapi/v2/services/MetadataService.ts +++ b/frontend/src/openapi/v2/services/MetadataService.ts @@ -80,6 +80,56 @@ export class MetadataService { }); } + /** + * Search Metadata Definition + * Search all metadata definition in the db based on text. + * + * Arguments: + * text -- any text matching name or description + * skip -- number of initial records to skip (i.e. for pagination) + * limit -- restrict number of records to be returned (i.e. for pagination) + * @param searchTerm + * @param skip + * @param limit + * @returns MetadataDefinitionOut Successful Response + * @throws ApiError + */ + public static searchMetadataDefinitionApiV2MetadataSearchSearchTermGet( + searchTerm: string, + skip?: number, + limit: number = 10, + ): CancelablePromise> { + return __request({ + method: 'GET', + path: `/api/v2/metadata/search/${searchTerm}`, + query: { + 'skip': skip, + 'limit': limit, + }, + errors: { + 422: `Validation Error`, + }, + }); + } + + /** + * Get Metadata Definition + * @param metadataDefinitionId + * @returns MetadataDefinitionOut Successful Response + * @throws ApiError + */ + public static getMetadataDefinitionApiV2MetadataMetadataDefinitionIdGet( + metadataDefinitionId: string, + ): CancelablePromise { + return __request({ + method: 'GET', + path: `/api/v2/metadata/${metadataDefinitionId}`, + errors: { + 422: `Validation Error`, + }, + }); + } + /** * Delete Metadata * Delete metadata by specific ID. From 501003f4508a4d7ee87c002a7350f1027d5c6813 Mon Sep 17 00:00:00 2001 From: Chen Wang Date: Mon, 2 Oct 2023 11:22:53 -0500 Subject: [PATCH 05/15] add search and get endpoint to action --- backend/app/routers/metadata.py | 80 ++++++++++--------- frontend/src/actions/metadata.js | 52 +++++++++++- .../openapi/v2/services/MetadataService.ts | 44 +++++----- 3 files changed, 114 insertions(+), 62 deletions(-) diff --git a/backend/app/routers/metadata.py b/backend/app/routers/metadata.py index 62fc338a1..e40ede057 100644 --- a/backend/app/routers/metadata.py +++ b/backend/app/routers/metadata.py @@ -29,8 +29,8 @@ @router.post("/definition", response_model=MetadataDefinitionOut) async def save_metadata_definition( - definition_in: MetadataDefinitionIn, - user=Depends(get_current_user), + definition_in: MetadataDefinitionIn, + user=Depends(get_current_user), ): existing = await MetadataDefinitionDB.find_one( MetadataDefinitionDB.name == definition_in.name @@ -47,11 +47,11 @@ async def save_metadata_definition( @router.get("/definition", response_model=List[MetadataDefinitionOut]) -async def get_metadata_definition( - name: Optional[str] = None, - user=Depends(get_current_user), - skip: int = 0, - limit: int = 2, +async def get_metadata_definition_list( + name: Optional[str] = None, + user=Depends(get_current_user), + skip: int = 0, + limit: int = 2, ): if name is None: defs = await MetadataDefinitionDB.find().skip(skip).limit(limit).to_list() @@ -65,12 +65,29 @@ async def get_metadata_definition( return [mddef.dict() for mddef in defs] +@router.get( + "/definition/{metadata_definition_id}", response_model=MetadataDefinitionOut +) +async def get_metadata_definition( + metadata_definition_id: str, + user=Depends(get_current_user), +): + if ( + mdd := await MetadataDefinitionDB.get(PydanticObjectId(metadata_definition_id)) + ) is not None: + return mdd.dict() + raise HTTPException( + status_code=404, + detail=f"Metadata definition {metadata_definition_id} not found", + ) + + @router.delete( "/definition/{metadata_definition_id}", response_model=MetadataDefinitionOut ) async def delete_metadata_definition( - metadata_definition_id: str, - user=Depends(get_current_user), + metadata_definition_id: str, + user=Depends(get_current_user), ): """Delete metadata definition by specific ID.""" mdd = await MetadataDefinitionDB.find_one( @@ -86,7 +103,7 @@ async def delete_metadata_definition( raise HTTPException( status_code=400, detail=f"Metadata definition {metadata_definition_id} in use. " - f"You cannot delete it until all metadata records using it are deleted.", + f"You cannot delete it until all metadata records using it are deleted.", ) # TODO: Refactor this with permissions checks etc. @@ -99,12 +116,14 @@ async def delete_metadata_definition( ) -@router.get("/search/{search_term}", response_model=List[MetadataDefinitionOut]) +@router.get( + "/definition/search/{search_term}", response_model=List[MetadataDefinitionOut] +) async def search_metadata_definition( - search_term: str, - skip: int = 0, - limit: int = 10, - user=Depends(get_current_user), + search_term: str, + skip: int = 0, + limit: int = 10, + user=Depends(get_current_user), ): """Search all metadata definition in the db based on text. @@ -127,28 +146,13 @@ async def search_metadata_definition( return [mdd.dict() for mdd in mdds] -@router.get("/{metadata_definition_id}", response_model=MetadataDefinitionOut) -async def get_metadata_definition( - metadata_definition_id: str, - user=Depends(get_current_user), -): - if ( - mdd := await MetadataDefinitionDB.get(PydanticObjectId(metadata_definition_id)) - ) is not None: - return mdd.dict() - raise HTTPException( - status_code=404, - detail=f"Metadata definition {metadata_definition_id} not found", - ) - - @router.patch("/{metadata_id}", response_model=MetadataOut) async def update_metadata( - metadata_in: MetadataPatch, - metadata_id: str, - es: Elasticsearch = Depends(dependencies.get_elasticsearchclient), - user=Depends(get_current_user), - allow: bool = Depends(MetadataAuthorization("editor")), + metadata_in: MetadataPatch, + metadata_id: str, + es: Elasticsearch = Depends(dependencies.get_elasticsearchclient), + user=Depends(get_current_user), + allow: bool = Depends(MetadataAuthorization("editor")), ): """Update metadata. Any fields provided in the contents JSON will be added or updated in the metadata. If context or agent should be changed, use PUT. @@ -166,9 +170,9 @@ async def update_metadata( @router.delete("/{metadata_id}") async def delete_metadata( - metadata_id: str, - user=Depends(get_current_user), - allow: bool = Depends(MetadataAuthorization("editor")), + metadata_id: str, + user=Depends(get_current_user), + allow: bool = Depends(MetadataAuthorization("editor")), ): """Delete metadata by specific ID.""" md = await MetadataDB.find_one(MetadataDB.id == PyObjectId(metadata_id)) diff --git a/frontend/src/actions/metadata.js b/frontend/src/actions/metadata.js index e35213dd0..eae296aff 100644 --- a/frontend/src/actions/metadata.js +++ b/frontend/src/actions/metadata.js @@ -5,8 +5,7 @@ export const RECEIVE_METADATA_DEFINITIONS = "RECEIVE_METADATA_DEFINITIONS"; export function fetchMetadataDefinitions(name = null, skip = 0, limit = 10) { return (dispatch) => { - // TODO need to think pagination - return V2.MetadataService.getMetadataDefinitionApiV2MetadataDefinitionGet( + return V2.MetadataService.getMetadataDefinitionListApiV2MetadataDefinitionGet( name, skip, limit @@ -26,6 +25,28 @@ export function fetchMetadataDefinitions(name = null, skip = 0, limit = 10) { }; } +export const RECEIVE_METADATA_DEFINITION = "RECEIVE_METADATA_DEFINITION"; + +export function fetchMetadataDefinition(metadataDefinitionId) { + return (dispatch) => { + return V2.MetadataService.getMetadataDefinitionApiV2MetadataDefinitionMetadataDefinitionIdGet( + metadataDefinitionId + ) + .then((json) => { + dispatch({ + type: RECEIVE_METADATA_DEFINITION, + metadataDefinition: json, + receivedAt: Date.now(), + }); + }) + .catch((reason) => { + dispatch( + handleErrors(reason, fetchMetadataDefinition(metadataDefinitionId)) + ); + }); + }; +} + export const SAVE_METADATA_DEFINITIONS = "SAVE_METADATA_DEFINITIONS"; export function postMetadataDefinitions(metadataDefinition) { @@ -70,6 +91,33 @@ export function deleteMetadataDefinition(metadataDefinitionId) { }; } +export const SEARCH_METADATA_DEFINITIONS = "SEARCH_METADATA_DEFINITIONS"; + +export function searchMetadataDefinitions(searchTerm, skip, limit) { + return (dispatch) => { + return V2.MetadataService.searchMetadataDefinitionApiV2MetadataSearchSearchTermGet( + searchTerm, + skip, + limit + ) + .then((json) => { + dispatch({ + type: SEARCH_METADATA_DEFINITIONS, + metadataDefinitionList: json, + receivedAt: Date.now(), + }); + }) + .catch((reason) => { + dispatch( + handleErrors( + reason, + searchMetadataDefinitions(searchTerm, skip, limit) + ) + ); + }); + }; +} + export const RECEIVE_DATASET_METADATA = "RECEIVE_DATASET_METADATA"; export function fetchDatasetMetadata(datasetId) { diff --git a/frontend/src/openapi/v2/services/MetadataService.ts b/frontend/src/openapi/v2/services/MetadataService.ts index 58077b673..caeda4b59 100644 --- a/frontend/src/openapi/v2/services/MetadataService.ts +++ b/frontend/src/openapi/v2/services/MetadataService.ts @@ -15,14 +15,14 @@ import { request as __request } from '../core/request'; export class MetadataService { /** - * Get Metadata Definition + * Get Metadata Definition List * @param name * @param skip * @param limit * @returns MetadataDefinitionOut Successful Response * @throws ApiError */ - public static getMetadataDefinitionApiV2MetadataDefinitionGet( + public static getMetadataDefinitionListApiV2MetadataDefinitionGet( name?: string, skip?: number, limit: number = 2, @@ -61,6 +61,24 @@ export class MetadataService { }); } + /** + * Get Metadata Definition + * @param metadataDefinitionId + * @returns MetadataDefinitionOut Successful Response + * @throws ApiError + */ + public static getMetadataDefinitionApiV2MetadataDefinitionMetadataDefinitionIdGet( + metadataDefinitionId: string, + ): CancelablePromise { + return __request({ + method: 'GET', + path: `/api/v2/metadata/definition/${metadataDefinitionId}`, + errors: { + 422: `Validation Error`, + }, + }); + } + /** * Delete Metadata Definition * Delete metadata definition by specific ID. @@ -94,14 +112,14 @@ export class MetadataService { * @returns MetadataDefinitionOut Successful Response * @throws ApiError */ - public static searchMetadataDefinitionApiV2MetadataSearchSearchTermGet( + public static searchMetadataDefinitionApiV2MetadataDefinitionSearchSearchTermGet( searchTerm: string, skip?: number, limit: number = 10, ): CancelablePromise> { return __request({ method: 'GET', - path: `/api/v2/metadata/search/${searchTerm}`, + path: `/api/v2/metadata/definition/search/${searchTerm}`, query: { 'skip': skip, 'limit': limit, @@ -112,24 +130,6 @@ export class MetadataService { }); } - /** - * Get Metadata Definition - * @param metadataDefinitionId - * @returns MetadataDefinitionOut Successful Response - * @throws ApiError - */ - public static getMetadataDefinitionApiV2MetadataMetadataDefinitionIdGet( - metadataDefinitionId: string, - ): CancelablePromise { - return __request({ - method: 'GET', - path: `/api/v2/metadata/${metadataDefinitionId}`, - errors: { - 422: `Validation Error`, - }, - }); - } - /** * Delete Metadata * Delete metadata by specific ID. From 8c4c3bbb34fc02da0f49a45fb4229460e1e15f6f Mon Sep 17 00:00:00 2001 From: Chen Wang Date: Mon, 2 Oct 2023 11:41:11 -0500 Subject: [PATCH 06/15] add to reducer --- frontend/src/reducers/metadata.ts | 86 ++++++++++++++++++++++--------- frontend/src/types/action.ts | 23 +++++++-- frontend/src/types/data.ts | 1 + 3 files changed, 82 insertions(+), 28 deletions(-) diff --git a/frontend/src/reducers/metadata.ts b/frontend/src/reducers/metadata.ts index 843de3a48..fccbc6c78 100644 --- a/frontend/src/reducers/metadata.ts +++ b/frontend/src/reducers/metadata.ts @@ -1,66 +1,102 @@ import { + DELETE_DATASET_METADATA, + DELETE_FILE_METADATA, + DELETE_METADATA_DEFINITION, POST_DATASET_METADATA, - SAVE_METADATA_DEFINITIONS, POST_FILE_METADATA, RECEIVE_DATASET_METADATA, - RECEIVE_METADATA_DEFINITIONS, RECEIVE_FILE_METADATA, + RECEIVE_METADATA_DEFINITION, + RECEIVE_METADATA_DEFINITIONS, + SAVE_METADATA_DEFINITIONS, + SEARCH_METADATA_DEFINITIONS, UPDATE_DATASET_METADATA, UPDATE_FILE_METADATA, - DELETE_DATASET_METADATA, DELETE_FILE_METADATA } from "../actions/metadata"; -import {DataAction} from "../types/action"; -import {MetadataState} from "../types/data"; +import { DataAction } from "../types/action"; +import { MetadataState } from "../types/data"; +import { MetadataDefinitionOut } from "../openapi/v2/"; const defaultState: MetadataState = { datasetMetadataList: [], fileMetadataList: [], - metadataDefinitionList: [] + metadataDefinitionList: [], + metadataDefinition: {}, }; const metadata = (state = defaultState, action: DataAction) => { switch (action.type) { case RECEIVE_METADATA_DEFINITIONS: - return Object.assign({}, state, {metadataDefinitionList: action.metadataDefinitionList}); + return Object.assign({}, state, { + metadataDefinitionList: action.metadataDefinitionList, + }); + case RECEIVE_METADATA_DEFINITION: + return Object.assign({}, state, { + metadataDefinition: action.metadataDefinition, + }); + case SEARCH_METADATA_DEFINITIONS: + return Object.assign({}, state, { + metadataDefinitionList: action.metadataDefinitionList, + }); + case DELETE_METADATA_DEFINITION: + return Object.assign({}, state, { + metadataDefinitionList: state.metadataDefinitionList.filter( + (metadataDefinition) => + metadataDefinition.id !== action.metadataDefinition.id + ), + }); + case SAVE_METADATA_DEFINITIONS: + return Object.assign({}, state, { + metadataDefinitionList: [ + ...state.metadataDefinitionList, + action.metadataDefinitionList, + ], + }); case RECEIVE_DATASET_METADATA: - return Object.assign({}, state, {datasetMetadataList: action.metadataList}); + return Object.assign({}, state, { + datasetMetadataList: action.metadataList, + }); case RECEIVE_FILE_METADATA: - return Object.assign({}, state, {fileMetadataList: action.metadataList}); + return Object.assign({}, state, { + fileMetadataList: action.metadataList, + }); case UPDATE_DATASET_METADATA: return Object.assign({}, state, { - datasetMetadataList: state.datasetMetadataList.map(dm => { - if (dm.id === action.metadata.id){ - return action.metadata + datasetMetadataList: state.datasetMetadataList.map((dm) => { + if (dm.id === action.metadata.id) { + return action.metadata; } - return dm - } ), + return dm; + }), }); case DELETE_DATASET_METADATA: return Object.assign({}, state, { - datasetMetadataList: state.datasetMetadataList.filter(metadata => metadata.id !== action.metadata.id), + datasetMetadataList: state.datasetMetadataList.filter( + (metadata) => metadata.id !== action.metadata.id + ), }); case DELETE_FILE_METADATA: return Object.assign({}, state, { - fileMetadataList: state.fileMetadataList.filter(metadata => metadata.id !== action.metadata.id), + fileMetadataList: state.fileMetadataList.filter( + (metadata) => metadata.id !== action.metadata.id + ), }); case UPDATE_FILE_METADATA: return Object.assign({}, state, { - fileMetadataList: state.fileMetadataList.map(fm => { - if (fm.id === action.metadata.id){ - return action.metadata + fileMetadataList: state.fileMetadataList.map((fm) => { + if (fm.id === action.metadata.id) { + return action.metadata; } - return fm - } ), + return fm; + }), }); case POST_DATASET_METADATA: return Object.assign({}, state, { - datasetMetadataList: [...state.datasetMetadataList, action.metadata] + datasetMetadataList: [...state.datasetMetadataList, action.metadata], }); - case SAVE_METADATA_DEFINITIONS: - return Object.assign({}, state, {metadataDefinitionList: [...state.metadataDefinitionList, action.metadataDefinitionList]}); case POST_FILE_METADATA: return Object.assign({}, state, { - fileMetadataList: [...state.fileMetadataList, action.metadata] + fileMetadataList: [...state.fileMetadataList, action.metadata], }); default: return state; diff --git a/frontend/src/types/action.ts b/frontend/src/types/action.ts index ea7798334..242d66a0b 100644 --- a/frontend/src/types/action.ts +++ b/frontend/src/types/action.ts @@ -1,14 +1,13 @@ import { - Dataset, ExtractedMetadata, FilePreview, Folder, MetadataJsonld, Profile, - FileState, } from "./data"; import { AuthorizationBase, + DatasetOut as Dataset, DatasetRoles, FileOut as FileSummary, FileVersion, @@ -92,7 +91,7 @@ interface RECEIVE_VERSIONS { interface CHANGE_SELECTED_VERSION { type: "CHANGE_SELECTED_VERSION"; - selected_version:number; + selected_version: number; } interface SET_USER { @@ -264,6 +263,21 @@ interface RECEIVE_METADATA_DEFINITIONS { metadataDefinitionList: MetadataDefinition[]; } +interface RECEIVE_METADATA_DEFINITION { + type: "RECEIVE_METADATA_DEFINITION"; + metadataDefinition: MetadataDefinition; +} + +interface SEARCH_METADATA_DEFINITIONS { + type: "SEARCH_METADATA_DEFINITIONS"; + metadataDefinitionList: MetadataDefinition[]; +} + +interface DELETE_METADATA_DEFINITION { + type: "DELETE_METADATA_DEFINITION"; + metadataDefinition: MetadataDefinition; +} + interface SAVE_METADATA_DEFINITIONS { type: "SAVE_METADATA_DEFINITIONS"; metadataDefinitionList: MetadataDefinition[]; @@ -470,6 +484,9 @@ export type DataAction = | POST_DATASET_METADATA | POST_FILE_METADATA | RECEIVE_METADATA_DEFINITIONS + | RECEIVE_METADATA_DEFINITION + | SEARCH_METADATA_DEFINITIONS + | DELETE_METADATA_DEFINITION | SAVE_METADATA_DEFINITIONS | RECEIVE_DATASET_METADATA | RECEIVE_FILE_METADATA diff --git a/frontend/src/types/data.ts b/frontend/src/types/data.ts index 3e7917001..200409243 100644 --- a/frontend/src/types/data.ts +++ b/frontend/src/types/data.ts @@ -173,6 +173,7 @@ export interface GroupState { export interface MetadataState { metadataDefinitionList: MetadataDefinitionOut[]; + metadataDefinition: MetadataDefinitionOut; datasetMetadataList: Metadata[]; fileMetadataList: Metadata[]; } From de2a0b876b3ec717d6327c6fbe877c3cebb358b4 Mon Sep 17 00:00:00 2001 From: Chen Wang Date: Mon, 2 Oct 2023 12:07:35 -0500 Subject: [PATCH 07/15] basic page for metadata definition ready --- frontend/src/actions/metadata.js | 4 +- frontend/src/components/Layout.tsx | 12 +- .../metadata/MetadataDefinitions.tsx | 272 ++++++++++++++++++ frontend/src/routes.tsx | 15 +- 4 files changed, 290 insertions(+), 13 deletions(-) create mode 100644 frontend/src/components/metadata/MetadataDefinitions.tsx diff --git a/frontend/src/actions/metadata.js b/frontend/src/actions/metadata.js index eae296aff..1a341a149 100644 --- a/frontend/src/actions/metadata.js +++ b/frontend/src/actions/metadata.js @@ -3,7 +3,7 @@ import { V2 } from "../openapi"; export const RECEIVE_METADATA_DEFINITIONS = "RECEIVE_METADATA_DEFINITIONS"; -export function fetchMetadataDefinitions(name = null, skip = 0, limit = 10) { +export function fetchMetadataDefinitions(name, skip, limit) { return (dispatch) => { return V2.MetadataService.getMetadataDefinitionListApiV2MetadataDefinitionGet( name, @@ -95,7 +95,7 @@ export const SEARCH_METADATA_DEFINITIONS = "SEARCH_METADATA_DEFINITIONS"; export function searchMetadataDefinitions(searchTerm, skip, limit) { return (dispatch) => { - return V2.MetadataService.searchMetadataDefinitionApiV2MetadataSearchSearchTermGet( + return V2.MetadataService.searchMetadataDefinitionApiV2MetadataDefinitionSearchSearchTermGet( searchTerm, skip, limit diff --git a/frontend/src/components/Layout.tsx b/frontend/src/components/Layout.tsx index 7e3847743..e13da0d6e 100644 --- a/frontend/src/components/Layout.tsx +++ b/frontend/src/components/Layout.tsx @@ -21,11 +21,12 @@ import { Link, Menu, MenuItem, MenuList } from "@mui/material"; import { Link as RouterLink, useLocation } from "react-router-dom"; import { useSelector } from "react-redux"; import { RootState } from "../types/data"; -import { AddBox, Create, Explore } from "@material-ui/icons"; +import { AddBox, Explore } from "@material-ui/icons"; import HistoryIcon from "@mui/icons-material/History"; import GroupIcon from "@mui/icons-material/Group"; import Gravatar from "react-gravatar"; import PersonIcon from "@mui/icons-material/Person"; +import InfoOutlinedIcon from "@mui/icons-material/InfoOutlined"; import { getCurrEmail } from "../utils/common"; import VpnKeyIcon from "@mui/icons-material/VpnKey"; import LogoutIcon from "@mui/icons-material/Logout"; @@ -302,13 +303,10 @@ export default function PersistentDrawerLeft(props) { - - + + - + diff --git a/frontend/src/components/metadata/MetadataDefinitions.tsx b/frontend/src/components/metadata/MetadataDefinitions.tsx new file mode 100644 index 000000000..7376a08f8 --- /dev/null +++ b/frontend/src/components/metadata/MetadataDefinitions.tsx @@ -0,0 +1,272 @@ +import React, { useEffect, useState } from "react"; +import { + Box, + Button, + ButtonGroup, + Dialog, + DialogContent, + DialogTitle, + Grid, + IconButton, + InputBase, +} from "@mui/material"; +import { RootState } from "../../types/data"; +import { useDispatch, useSelector } from "react-redux"; +import { + deleteMetadataDefinition as deleteMetadataDefinitionAction, + fetchMetadataDefinitions as fetchMetadataDefinitionsAction, + searchMetadataDefinitions as searchMetadataDefinitionsAction, +} from "../../actions/metadata"; +import { ArrowBack, ArrowForward, SearchOutlined } from "@material-ui/icons"; +import { Link } from "react-router-dom"; +import Paper from "@mui/material/Paper"; +import Table from "@mui/material/Table"; +import TableHead from "@mui/material/TableHead"; +import TableRow from "@mui/material/TableRow"; +import TableCell from "@mui/material/TableCell"; +import TableBody from "@mui/material/TableBody"; +import TableContainer from "@mui/material/TableContainer"; +import InfoIcon from "@mui/icons-material/Info"; +import { theme } from "../../theme"; +import Layout from "../Layout"; +import { MainBreadcrumbs } from "../navigation/BreadCrumb"; +import { ErrorModal } from "../errors/ErrorModal"; +import { CreateMetadataDefinition } from "./CreateMetadataDefinition"; + +export function MetadataDefinitions() { + // Redux connect equivalent + const dispatch = useDispatch(); + const listMetadataDefinitions = ( + name: string | undefined | null, + skip: number | undefined, + limit: number | undefined + ) => dispatch(fetchMetadataDefinitionsAction(name, skip, limit)); + const searchMetadataDefinitions = ( + searchTerm: string, + skip: number | undefined, + limit: number | undefined + ) => dispatch(searchMetadataDefinitionsAction(searchTerm, skip, limit)); + const deleteMetadataDefinition = (id: string) => + dispatch(deleteMetadataDefinitionAction(id)); + const metadataDefinitions = useSelector( + (state: RootState) => state.metadata.metadataDefinitionList + ); + + // TODO add option to determine limit number; default show 5 metadata definitions each time + const [currPageNum, setCurrPageNum] = useState(0); + const [limit] = useState(5); + const [skip, setSkip] = useState(0); + const [prevDisabled, setPrevDisabled] = useState(true); + const [nextDisabled, setNextDisabled] = useState(false); + const [searchTerm, setSearchTerm] = useState(""); + const [createMetadataDefinitionOpen, setCreateMetadataDefinitionOpen] = + useState(false); + + // for breadcrumb + const paths = [ + { + name: "Metadata Definitions", + url: "/metadata-definitions", + }, + ]; + + // component did mount + useEffect(() => { + listMetadataDefinitions(null, skip, limit); + }, []); + + useEffect(() => { + // disable flipping if reaches the last page + if (metadataDefinitions.length < limit) setNextDisabled(true); + else setNextDisabled(false); + }, [metadataDefinitions]); + + // search + useEffect(() => { + if (searchTerm !== "") searchMetadataDefinitions(searchTerm, skip, limit); + else listMetadataDefinitions(null, skip, limit); + }, [searchTerm]); + + useEffect(() => { + if (skip !== null && skip !== undefined) { + listMetadataDefinitions(null, skip, limit); + if (skip === 0) setPrevDisabled(true); + else setPrevDisabled(false); + } + }, [skip]); + + // Error msg dialog + const [errorOpen, setErrorOpen] = useState(false); + + const previous = () => { + if (currPageNum - 1 >= 0) { + setSkip((currPageNum - 1) * limit); + setCurrPageNum(currPageNum - 1); + } + }; + const next = () => { + if (metadataDefinitions.length === limit) { + setSkip((currPageNum + 1) * limit); + setCurrPageNum(currPageNum + 1); + } + }; + + return ( + + {/*Error Message dialogue*/} + + + {/*create new metadata definition*/} + { + setCreateMetadataDefinitionOpen(false); + }} + fullWidth={true} + maxWidth="md" + aria-labelledby="form-dialog" + > + Create New Metadata Definition + + + + + {/*breadcrumb*/} + + + + + + + + +
+ + + + { + setSearchTerm(e.target.value); + }} + onKeyDown={(e) => { + if (e.key === "Enter") { + e.preventDefault(); + searchMetadataDefinitions(searchTerm, skip, limit); + } + }} + value={searchTerm} + /> + { + searchMetadataDefinitions(searchTerm, skip, limit); + }} + > + + + + + + + + + + + Metadata Definition + + + Description + + + + + + {metadataDefinitions.map((mdd) => { + return ( + + + + + + {mdd.description} + + + 0 + + + ); + })} + +
+ + + + + + + + + + + ); +} diff --git a/frontend/src/routes.tsx b/frontend/src/routes.tsx index 81895c7a8..305b81fc9 100644 --- a/frontend/src/routes.tsx +++ b/frontend/src/routes.tsx @@ -7,8 +7,6 @@ import { useNavigate, useParams, } from "react-router-dom"; - -import { CreateMetadataDefinitionPage } from "./components/metadata/CreateMetadataDefinition"; import { Dataset as DatasetComponent } from "./components/datasets/Dataset"; import { File as FileComponent } from "./components/files/File"; import { CreateDataset } from "./components/datasets/CreateDataset"; @@ -32,6 +30,7 @@ import { Forbidden } from "./components/errors/Forbidden"; import { ApiKeys } from "./components/ApiKeys/ApiKey"; import { Profile } from "./components/users/Profile"; import config from "./app.config"; +import { MetadataDefinitions } from "./components/metadata/MetadataDefinitions"; // https://dev.to/iamandrewluca/private-route-in-react-router-v6-lg5 const PrivateRoute = (props): JSX.Element => { @@ -123,13 +122,21 @@ export const AppRoutes = (): JSX.Element => { } /> - + } /> + {/**/} + {/* */} + {/* */} + {/* }*/} + {/*/>*/} Date: Mon, 2 Oct 2023 12:23:21 -0500 Subject: [PATCH 08/15] add delete confirmation --- .../DeleteMetadataDefinitionModal.tsx | 42 ++++++++++++++++++ .../metadata/MetadataDefinitions.tsx | 44 +++++++++++++++++-- 2 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 frontend/src/components/metadata/DeleteMetadataDefinitionModal.tsx diff --git a/frontend/src/components/metadata/DeleteMetadataDefinitionModal.tsx b/frontend/src/components/metadata/DeleteMetadataDefinitionModal.tsx new file mode 100644 index 000000000..68eb64bb4 --- /dev/null +++ b/frontend/src/components/metadata/DeleteMetadataDefinitionModal.tsx @@ -0,0 +1,42 @@ +import React from "react"; + +import { ActionModal } from "../dialog/ActionModal"; +import { deleteMetadataDefinition as deleteMetadataDefinitionAction } from "../../actions/metadata"; +import { useNavigate } from "react-router-dom"; +import { useDispatch } from "react-redux"; + +type DeleteMetadataDefinitionModalProps = { + deleteMetadataDefinitionConfirmOpen: any; + setDeleteMetadataDefinitionConfirmOpen: any; + metdataDefinitionId: string | undefined; +}; + +export default function DeleteMetadataDefinitionModal( + props: DeleteMetadataDefinitionModalProps +) { + const { + deleteMetadataDefinitionConfirmOpen, + setDeleteMetadataDefinitionConfirmOpen, + metdataDefinitionId, + } = props; + const history = useNavigate(); + const dispatch = useDispatch(); + const deleteMetadataDefinition = (metadataDefinitionId: string | undefined) => + dispatch(deleteMetadataDefinitionAction(metadataDefinitionId)); + return ( + { + deleteMetadataDefinition(metdataDefinitionId); + setDeleteMetadataDefinitionConfirmOpen(false); + history("/metadata-definitions"); + }} + handleActionCancel={() => { + setDeleteMetadataDefinitionConfirmOpen(false); + }} + /> + ); +} diff --git a/frontend/src/components/metadata/MetadataDefinitions.tsx b/frontend/src/components/metadata/MetadataDefinitions.tsx index 7376a08f8..b96e51200 100644 --- a/frontend/src/components/metadata/MetadataDefinitions.tsx +++ b/frontend/src/components/metadata/MetadataDefinitions.tsx @@ -32,6 +32,8 @@ import Layout from "../Layout"; import { MainBreadcrumbs } from "../navigation/BreadCrumb"; import { ErrorModal } from "../errors/ErrorModal"; import { CreateMetadataDefinition } from "./CreateMetadataDefinition"; +import DeleteIcon from "@mui/icons-material/Delete"; +import DeleteMetadataDefinitionModal from "./DeleteMetadataDefinitionModal"; export function MetadataDefinitions() { // Redux connect equivalent @@ -61,6 +63,12 @@ export function MetadataDefinitions() { const [searchTerm, setSearchTerm] = useState(""); const [createMetadataDefinitionOpen, setCreateMetadataDefinitionOpen] = useState(false); + const [ + deleteMetadataDefinitionConfirmOpen, + setDeleteMetadataDefinitionConfirmOpen, + ] = useState(false); + const [selectedMetadataDefinition, setSelectedMetadataDefinition] = + useState(); // for breadcrumb const paths = [ @@ -116,6 +124,17 @@ export function MetadataDefinitions() { {/*Error Message dialogue*/} + {/*Delete metadata definition modal*/} + + {/*create new metadata definition*/} - + + {contextMap.map((item, idx) => { + return ( + + + { + updateContext(idx, "term", event.target.value); + }} + /> + + + option.url + )} + onInputChange={(event, value) => { + updateContext(idx, "iri", value); + }} + renderInput={(params) => ( + + )} + /> + + addNewContext(idx)} + > + + + {idx == 0 ? ( + <> + ) : ( + removeContext(idx)} + > + + + )} + + ); + })} + + {formInput.fields.map((input, idx) => { - return ( - {idx == 0 ? - { - setActiveStep(idx + 1) - }}> - Add metadata entry* - {idx == activeStep - 1 ? - addNewField(idx)}> - + return ( + + {idx == 0 ? ( + { + setActiveStep(idx + 1); + }} + > + + Add metadata entry* + {idx == activeStep - 1 ? ( + addNewField(idx)} + > + - : <>} + ) : ( + <> + )} - : - { - setActiveStep(idx + 1) - }}> - Add additional entry - {idx == activeStep - 1 ? + ) : ( + { + setActiveStep(idx + 1); + }} + > + + Add additional entry + {idx == activeStep - 1 ? ( <> - addNewField(idx)}> - + addNewField(idx)} + > + - removeField(idx)}> - + removeField(idx)} + > + - : - removeField(idx)}> - - } + ) : ( + removeField(idx)} + > + + + )} - } + )} - { - handleInputChange(idx, "list", event.target.value); - }} - />} label="Allow Many"/> - { - handleInputChange(idx, "required", event.target.value); - }} - />} label="Required"/> + { + handleInputChange( + idx, + "list", + event.target.value + ); + }} + /> + } + label="Allow Many" + /> + { + handleInputChange( + idx, + "required", + event.target.value + ); + }} + /> + } + label="Required" + /> @@ -431,7 +522,7 @@ export const CreateMetadataDefinition = (): JSX.Element => { id="field-name" label="Field Name" placeholder="Please enter field name" - InputLabelProps={{shrink: true}} + InputLabelProps={{ shrink: true }} value={input.name} onChange={(event) => { handleInputChange(idx, "name", event.target.value); @@ -449,12 +540,15 @@ export const CreateMetadataDefinition = (): JSX.Element => { onChange={(event) => { handleInputChange(idx, "widgetType", event.target.value); }} - InputLabelProps={{shrink: true}} + InputLabelProps={{ shrink: true }} helperText="Please select metadata widget type" - select> + select + > {Object.keys(widgetTypes).map((key) => { return ( - {widgetTypes[key]} + + {widgetTypes[key]} + ); })} @@ -469,41 +563,51 @@ export const CreateMetadataDefinition = (): JSX.Element => { onChange={(event) => { handleInputChange(idx, "type", event.target.value); }} - InputLabelProps={{shrink: true}} + InputLabelProps={{ shrink: true }} helperText="Please select metadata field data type" - select> + select + > {Object.keys(inputTypes).map((key) => { return ( - {inputTypes[key]} + + {inputTypes[key]} + ); })} {/* - * TODO: Expand to support different config data type actions - * https://github.com/clowder-framework/clowder2/issues/169 - */} - {(input.config.type == "enum") ? <> - { - handleInputChange(idx, "options", event.target.value); - }} - name="Field Data Options" - multiline - maxRows={6} - /> - : <>} - @@ -514,17 +618,19 @@ export const CreateMetadataDefinition = (): JSX.Element => { Submit - -
+ +
From 35de0ebd4c9e2b1e001e31fae06b6ae53fb55d83 Mon Sep 17 00:00:00 2001 From: Chen Wang Date: Mon, 2 Oct 2023 14:08:30 -0500 Subject: [PATCH 10/15] ordering in decending order --- backend/app/models/metadata.py | 15 ++--- backend/app/routers/groups.py | 61 +++++++++---------- backend/app/routers/metadata.py | 19 +++--- .../metadata/MetadataDefinitions.tsx | 18 +++--- 4 files changed, 58 insertions(+), 55 deletions(-) diff --git a/backend/app/models/metadata.py b/backend/app/models/metadata.py index f60e5d4b8..82c1a798d 100644 --- a/backend/app/models/metadata.py +++ b/backend/app/models/metadata.py @@ -89,6 +89,7 @@ class MetadataDefinitionBase(BaseModel): name: str description: Optional[str] + created: datetime = Field(default_factory=datetime.utcnow) context: Optional[ List[Union[dict, AnyUrl]] ] # https://json-ld.org/spec/latest/json-ld/#the-context @@ -248,10 +249,10 @@ class Config: async def validate_context( - content: dict, - definition: Optional[str] = None, - context_url: Optional[str] = None, - context: Optional[List[Union[dict, AnyUrl]]] = None, + content: dict, + definition: Optional[str] = None, + context_url: Optional[str] = None, + context: Optional[List[Union[dict, AnyUrl]]] = None, ): """Convenience function for making sure incoming metadata has valid definitions or resolvable context. @@ -269,9 +270,9 @@ async def validate_context( pass if definition is not None: if ( - md_def := await MetadataDefinitionDB.find_one( - MetadataDefinitionDB.name == definition - ) + md_def := await MetadataDefinitionDB.find_one( + MetadataDefinitionDB.name == definition + ) ) is not None: content = validate_definition(content, md_def) else: diff --git a/backend/app/routers/groups.py b/backend/app/routers/groups.py index 84a9813a6..5bd36a827 100644 --- a/backend/app/routers/groups.py +++ b/backend/app/routers/groups.py @@ -5,7 +5,6 @@ from beanie.operators import Or, Push, RegEx from bson.objectid import ObjectId from fastapi import HTTPException, Depends, APIRouter -from pymongo import DESCENDING from app.deps.authorization_deps import AuthorizationDB, GroupAuthorization from app.keycloak_auth import get_current_user, get_user @@ -18,8 +17,8 @@ @router.post("", response_model=GroupOut) async def save_group( - group_in: GroupIn, - user=Depends(get_current_user), + group_in: GroupIn, + user=Depends(get_current_user), ): group_db = GroupDB(**group_in.dict(), creator=user.email) user_member = Member(user=user, editor=True) @@ -31,14 +30,14 @@ async def save_group( @router.get("", response_model=List[GroupOut]) async def get_groups( - user_id=Depends(get_user), - skip: int = 0, - limit: int = 10, + user_id=Depends(get_user), + skip: int = 0, + limit: int = 10, ): """Get a list of all Groups in the db the user is a member/owner of. Arguments: - skip -- number of initial records to skip (i.e. for pagination) + skip -- number of initial recoto_list()rds to skip (i.e. for pagination) limit -- restrict number of records to be returned (i.e. for pagination) @@ -57,10 +56,10 @@ async def get_groups( @router.get("/search/{search_term}", response_model=List[GroupOut]) async def search_group( - search_term: str, - user_id=Depends(get_user), - skip: int = 0, - limit: int = 10, + search_term: str, + user_id=Depends(get_user), + skip: int = 0, + limit: int = 10, ): """Search all groups in the db based on text. @@ -86,8 +85,8 @@ async def search_group( @router.get("/{group_id}", response_model=GroupOut) async def get_group( - group_id: str, - allow: bool = Depends(GroupAuthorization("viewer")), + group_id: str, + allow: bool = Depends(GroupAuthorization("viewer")), ): if (group := await GroupDB.get(PydanticObjectId(group_id))) is not None: return group.dict() @@ -96,10 +95,10 @@ async def get_group( @router.put("/{group_id}", response_model=GroupOut) async def edit_group( - group_id: str, - group_info: GroupBase, - user_id=Depends(get_user), - allow: bool = Depends(GroupAuthorization("editor")), + group_id: str, + group_info: GroupBase, + user_id=Depends(get_user), + allow: bool = Depends(GroupAuthorization("editor")), ): if (group := await GroupDB.get(PydanticObjectId(group_id))) is not None: group_dict = dict(group_info) if group_info is not None else {} @@ -122,7 +121,7 @@ async def edit_group( if original_user not in groups_users: # remove them from auth async for auth in AuthorizationDB.find( - {"group_ids": ObjectId(group_id)} + {"group_ids": ObjectId(group_id)} ): auth.user_ids.remove(original_user.user.email) await auth.replace() @@ -166,8 +165,8 @@ async def edit_group( @router.delete("/{group_id}", response_model=GroupOut) async def delete_group( - group_id: str, - allow: bool = Depends(GroupAuthorization("owner")), + group_id: str, + allow: bool = Depends(GroupAuthorization("owner")), ): if (group := await GroupDB.get(PydanticObjectId(group_id))) is not None: await group.delete() @@ -178,10 +177,10 @@ async def delete_group( @router.post("/{group_id}/add/{username}", response_model=GroupOut) async def add_member( - group_id: str, - username: str, - role: Optional[str] = None, - allow: bool = Depends(GroupAuthorization("editor")), + group_id: str, + username: str, + role: Optional[str] = None, + allow: bool = Depends(GroupAuthorization("editor")), ): """Add a new user to a group.""" if (user := await UserDB.find_one(UserDB.email == username)) is not None: @@ -215,9 +214,9 @@ async def add_member( @router.post("/{group_id}/remove/{username}", response_model=GroupOut) async def remove_member( - group_id: str, - username: str, - allow: bool = Depends(GroupAuthorization("editor")), + group_id: str, + username: str, + allow: bool = Depends(GroupAuthorization("editor")), ): """Remove a user from a group.""" @@ -247,10 +246,10 @@ async def remove_member( @router.put("/{group_id}/update/{username}", response_model=GroupOut) async def update_member( - group_id: str, - username: str, - role: str, - allow: bool = Depends(GroupAuthorization("editor")), + group_id: str, + username: str, + role: str, + allow: bool = Depends(GroupAuthorization("editor")), ): """Update user role.""" if (user := await UserDB.find_one({"email": username})) is not None: diff --git a/backend/app/routers/metadata.py b/backend/app/routers/metadata.py index e40ede057..4d225158b 100644 --- a/backend/app/routers/metadata.py +++ b/backend/app/routers/metadata.py @@ -54,14 +54,18 @@ async def get_metadata_definition_list( limit: int = 2, ): if name is None: - defs = await MetadataDefinitionDB.find().skip(skip).limit(limit).to_list() + defs = await MetadataDefinitionDB.find( + sort=(-MetadataDefinitionDB.created), + skip=skip, + limit=limit + ).to_list() else: - defs = ( - await MetadataDefinitionDB.find(MetadataDefinitionDB.name == name) - .skip(skip) - .limit(limit) - .to_list() - ) + defs = await MetadataDefinitionDB.find( + MetadataDefinitionDB.name == name, + sort=(-MetadataDefinitionDB.created), + skip=skip, + limit=limit, + ).to_list() return [mddef.dict() for mddef in defs] @@ -139,6 +143,7 @@ async def search_metadata_definition( RegEx(field=MetadataDefinitionDB.description, pattern=search_term), RegEx(field=MetadataDefinitionDB.context, pattern=search_term), ), + sort=(-MetadataDefinitionDB.created), skip=skip, limit=limit, ).to_list() diff --git a/frontend/src/components/metadata/MetadataDefinitions.tsx b/frontend/src/components/metadata/MetadataDefinitions.tsx index b96e51200..a0367c632 100644 --- a/frontend/src/components/metadata/MetadataDefinitions.tsx +++ b/frontend/src/components/metadata/MetadataDefinitions.tsx @@ -13,12 +13,10 @@ import { import { RootState } from "../../types/data"; import { useDispatch, useSelector } from "react-redux"; import { - deleteMetadataDefinition as deleteMetadataDefinitionAction, fetchMetadataDefinitions as fetchMetadataDefinitionsAction, searchMetadataDefinitions as searchMetadataDefinitionsAction, } from "../../actions/metadata"; import { ArrowBack, ArrowForward, SearchOutlined } from "@material-ui/icons"; -import { Link } from "react-router-dom"; import Paper from "@mui/material/Paper"; import Table from "@mui/material/Table"; import TableHead from "@mui/material/TableHead"; @@ -48,8 +46,6 @@ export function MetadataDefinitions() { skip: number | undefined, limit: number | undefined ) => dispatch(searchMetadataDefinitionsAction(searchTerm, skip, limit)); - const deleteMetadataDefinition = (id: string) => - dispatch(deleteMetadataDefinitionAction(id)); const metadataDefinitions = useSelector( (state: RootState) => state.metadata.metadataDefinitionList ); @@ -244,12 +240,14 @@ export function MetadataDefinitions() { sx={{ "&:last-child td, &:last-child th": { border: 0 } }} > - + {/*TODO write individual metadata definition page*/} + {/**/} + {/* {mdd.name}*/} + {/**/} + {mdd.name} Date: Mon, 2 Oct 2023 14:21:43 -0500 Subject: [PATCH 11/15] update message --- backend/app/routers/metadata.py | 2 +- frontend/src/reducers/metadata.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/app/routers/metadata.py b/backend/app/routers/metadata.py index 4d225158b..34c6e079c 100644 --- a/backend/app/routers/metadata.py +++ b/backend/app/routers/metadata.py @@ -106,7 +106,7 @@ async def delete_metadata_definition( if len(metadata_using_definition) > 0: raise HTTPException( status_code=400, - detail=f"Metadata definition {metadata_definition_id} in use. " + detail=f"Metadata definition: {mdd.name} ({metadata_definition_id}) in use. " f"You cannot delete it until all metadata records using it are deleted.", ) diff --git a/frontend/src/reducers/metadata.ts b/frontend/src/reducers/metadata.ts index fccbc6c78..8a753e87b 100644 --- a/frontend/src/reducers/metadata.ts +++ b/frontend/src/reducers/metadata.ts @@ -48,8 +48,8 @@ const metadata = (state = defaultState, action: DataAction) => { case SAVE_METADATA_DEFINITIONS: return Object.assign({}, state, { metadataDefinitionList: [ - ...state.metadataDefinitionList, action.metadataDefinitionList, + ...state.metadataDefinitionList, ], }); case RECEIVE_DATASET_METADATA: From 7cda0ab6482906e395b53a30a3edb3a2d9ac7889 Mon Sep 17 00:00:00 2001 From: Chen Wang Date: Mon, 2 Oct 2023 14:22:37 -0500 Subject: [PATCH 12/15] black --- backend/app/models/metadata.py | 14 ++++---- backend/app/routers/groups.py | 58 ++++++++++++++++----------------- backend/app/routers/metadata.py | 52 ++++++++++++++--------------- 3 files changed, 61 insertions(+), 63 deletions(-) diff --git a/backend/app/models/metadata.py b/backend/app/models/metadata.py index 82c1a798d..a3a5f01ca 100644 --- a/backend/app/models/metadata.py +++ b/backend/app/models/metadata.py @@ -249,10 +249,10 @@ class Config: async def validate_context( - content: dict, - definition: Optional[str] = None, - context_url: Optional[str] = None, - context: Optional[List[Union[dict, AnyUrl]]] = None, + content: dict, + definition: Optional[str] = None, + context_url: Optional[str] = None, + context: Optional[List[Union[dict, AnyUrl]]] = None, ): """Convenience function for making sure incoming metadata has valid definitions or resolvable context. @@ -270,9 +270,9 @@ async def validate_context( pass if definition is not None: if ( - md_def := await MetadataDefinitionDB.find_one( - MetadataDefinitionDB.name == definition - ) + md_def := await MetadataDefinitionDB.find_one( + MetadataDefinitionDB.name == definition + ) ) is not None: content = validate_definition(content, md_def) else: diff --git a/backend/app/routers/groups.py b/backend/app/routers/groups.py index 5bd36a827..5c6115ccc 100644 --- a/backend/app/routers/groups.py +++ b/backend/app/routers/groups.py @@ -17,8 +17,8 @@ @router.post("", response_model=GroupOut) async def save_group( - group_in: GroupIn, - user=Depends(get_current_user), + group_in: GroupIn, + user=Depends(get_current_user), ): group_db = GroupDB(**group_in.dict(), creator=user.email) user_member = Member(user=user, editor=True) @@ -30,9 +30,9 @@ async def save_group( @router.get("", response_model=List[GroupOut]) async def get_groups( - user_id=Depends(get_user), - skip: int = 0, - limit: int = 10, + user_id=Depends(get_user), + skip: int = 0, + limit: int = 10, ): """Get a list of all Groups in the db the user is a member/owner of. @@ -56,10 +56,10 @@ async def get_groups( @router.get("/search/{search_term}", response_model=List[GroupOut]) async def search_group( - search_term: str, - user_id=Depends(get_user), - skip: int = 0, - limit: int = 10, + search_term: str, + user_id=Depends(get_user), + skip: int = 0, + limit: int = 10, ): """Search all groups in the db based on text. @@ -85,8 +85,8 @@ async def search_group( @router.get("/{group_id}", response_model=GroupOut) async def get_group( - group_id: str, - allow: bool = Depends(GroupAuthorization("viewer")), + group_id: str, + allow: bool = Depends(GroupAuthorization("viewer")), ): if (group := await GroupDB.get(PydanticObjectId(group_id))) is not None: return group.dict() @@ -95,10 +95,10 @@ async def get_group( @router.put("/{group_id}", response_model=GroupOut) async def edit_group( - group_id: str, - group_info: GroupBase, - user_id=Depends(get_user), - allow: bool = Depends(GroupAuthorization("editor")), + group_id: str, + group_info: GroupBase, + user_id=Depends(get_user), + allow: bool = Depends(GroupAuthorization("editor")), ): if (group := await GroupDB.get(PydanticObjectId(group_id))) is not None: group_dict = dict(group_info) if group_info is not None else {} @@ -121,7 +121,7 @@ async def edit_group( if original_user not in groups_users: # remove them from auth async for auth in AuthorizationDB.find( - {"group_ids": ObjectId(group_id)} + {"group_ids": ObjectId(group_id)} ): auth.user_ids.remove(original_user.user.email) await auth.replace() @@ -165,8 +165,8 @@ async def edit_group( @router.delete("/{group_id}", response_model=GroupOut) async def delete_group( - group_id: str, - allow: bool = Depends(GroupAuthorization("owner")), + group_id: str, + allow: bool = Depends(GroupAuthorization("owner")), ): if (group := await GroupDB.get(PydanticObjectId(group_id))) is not None: await group.delete() @@ -177,10 +177,10 @@ async def delete_group( @router.post("/{group_id}/add/{username}", response_model=GroupOut) async def add_member( - group_id: str, - username: str, - role: Optional[str] = None, - allow: bool = Depends(GroupAuthorization("editor")), + group_id: str, + username: str, + role: Optional[str] = None, + allow: bool = Depends(GroupAuthorization("editor")), ): """Add a new user to a group.""" if (user := await UserDB.find_one(UserDB.email == username)) is not None: @@ -214,9 +214,9 @@ async def add_member( @router.post("/{group_id}/remove/{username}", response_model=GroupOut) async def remove_member( - group_id: str, - username: str, - allow: bool = Depends(GroupAuthorization("editor")), + group_id: str, + username: str, + allow: bool = Depends(GroupAuthorization("editor")), ): """Remove a user from a group.""" @@ -246,10 +246,10 @@ async def remove_member( @router.put("/{group_id}/update/{username}", response_model=GroupOut) async def update_member( - group_id: str, - username: str, - role: str, - allow: bool = Depends(GroupAuthorization("editor")), + group_id: str, + username: str, + role: str, + allow: bool = Depends(GroupAuthorization("editor")), ): """Update user role.""" if (user := await UserDB.find_one({"email": username})) is not None: diff --git a/backend/app/routers/metadata.py b/backend/app/routers/metadata.py index 34c6e079c..21f829e83 100644 --- a/backend/app/routers/metadata.py +++ b/backend/app/routers/metadata.py @@ -29,8 +29,8 @@ @router.post("/definition", response_model=MetadataDefinitionOut) async def save_metadata_definition( - definition_in: MetadataDefinitionIn, - user=Depends(get_current_user), + definition_in: MetadataDefinitionIn, + user=Depends(get_current_user), ): existing = await MetadataDefinitionDB.find_one( MetadataDefinitionDB.name == definition_in.name @@ -48,16 +48,14 @@ async def save_metadata_definition( @router.get("/definition", response_model=List[MetadataDefinitionOut]) async def get_metadata_definition_list( - name: Optional[str] = None, - user=Depends(get_current_user), - skip: int = 0, - limit: int = 2, + name: Optional[str] = None, + user=Depends(get_current_user), + skip: int = 0, + limit: int = 2, ): if name is None: defs = await MetadataDefinitionDB.find( - sort=(-MetadataDefinitionDB.created), - skip=skip, - limit=limit + sort=(-MetadataDefinitionDB.created), skip=skip, limit=limit ).to_list() else: defs = await MetadataDefinitionDB.find( @@ -73,11 +71,11 @@ async def get_metadata_definition_list( "/definition/{metadata_definition_id}", response_model=MetadataDefinitionOut ) async def get_metadata_definition( - metadata_definition_id: str, - user=Depends(get_current_user), + metadata_definition_id: str, + user=Depends(get_current_user), ): if ( - mdd := await MetadataDefinitionDB.get(PydanticObjectId(metadata_definition_id)) + mdd := await MetadataDefinitionDB.get(PydanticObjectId(metadata_definition_id)) ) is not None: return mdd.dict() raise HTTPException( @@ -90,8 +88,8 @@ async def get_metadata_definition( "/definition/{metadata_definition_id}", response_model=MetadataDefinitionOut ) async def delete_metadata_definition( - metadata_definition_id: str, - user=Depends(get_current_user), + metadata_definition_id: str, + user=Depends(get_current_user), ): """Delete metadata definition by specific ID.""" mdd = await MetadataDefinitionDB.find_one( @@ -107,7 +105,7 @@ async def delete_metadata_definition( raise HTTPException( status_code=400, detail=f"Metadata definition: {mdd.name} ({metadata_definition_id}) in use. " - f"You cannot delete it until all metadata records using it are deleted.", + f"You cannot delete it until all metadata records using it are deleted.", ) # TODO: Refactor this with permissions checks etc. @@ -124,10 +122,10 @@ async def delete_metadata_definition( "/definition/search/{search_term}", response_model=List[MetadataDefinitionOut] ) async def search_metadata_definition( - search_term: str, - skip: int = 0, - limit: int = 10, - user=Depends(get_current_user), + search_term: str, + skip: int = 0, + limit: int = 10, + user=Depends(get_current_user), ): """Search all metadata definition in the db based on text. @@ -153,11 +151,11 @@ async def search_metadata_definition( @router.patch("/{metadata_id}", response_model=MetadataOut) async def update_metadata( - metadata_in: MetadataPatch, - metadata_id: str, - es: Elasticsearch = Depends(dependencies.get_elasticsearchclient), - user=Depends(get_current_user), - allow: bool = Depends(MetadataAuthorization("editor")), + metadata_in: MetadataPatch, + metadata_id: str, + es: Elasticsearch = Depends(dependencies.get_elasticsearchclient), + user=Depends(get_current_user), + allow: bool = Depends(MetadataAuthorization("editor")), ): """Update metadata. Any fields provided in the contents JSON will be added or updated in the metadata. If context or agent should be changed, use PUT. @@ -175,9 +173,9 @@ async def update_metadata( @router.delete("/{metadata_id}") async def delete_metadata( - metadata_id: str, - user=Depends(get_current_user), - allow: bool = Depends(MetadataAuthorization("editor")), + metadata_id: str, + user=Depends(get_current_user), + allow: bool = Depends(MetadataAuthorization("editor")), ): """Delete metadata by specific ID.""" md = await MetadataDB.find_one(MetadataDB.id == PyObjectId(metadata_id)) From 5cfadc0c93116400d1692883bb58838d4fdef733 Mon Sep 17 00:00:00 2001 From: Chen Wang Date: Mon, 2 Oct 2023 14:30:58 -0500 Subject: [PATCH 13/15] fix the pytest --- backend/app/tests/test_metadata.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/backend/app/tests/test_metadata.py b/backend/app/tests/test_metadata.py index 77dd6bde9..aace9fe39 100644 --- a/backend/app/tests/test_metadata.py +++ b/backend/app/tests/test_metadata.py @@ -80,7 +80,7 @@ def test_dataset_create_and_delete_metadata_definition_success( # Post the definition itself response = client.post( f"{settings.API_V2_STR}/metadata/definition", - json=metadata_definition, + json=metadata_definition2, headers=headers, ) assert ( @@ -137,11 +137,6 @@ def test_dataset_create_and_delete_metadata_definition_fail( headers=headers, ) assert response.status_code == 400 - assert ( - response.json()["detail"] - == f"Metadata definition {metadata_definition_id} in use. " - f"You cannot delete it until all metadata records using it are deleted." - ) @pytest.mark.asyncio From 5b6388a77e54536bd9a7b7ff747541582f62b803 Mon Sep 17 00:00:00 2001 From: Chen Wang Date: Mon, 9 Oct 2023 09:52:52 -0500 Subject: [PATCH 14/15] codegen --- frontend/src/openapi/v2/models/MetadataDefinitionIn.ts | 1 + frontend/src/openapi/v2/models/MetadataDefinitionOut.ts | 1 + frontend/src/openapi/v2/services/GroupsService.ts | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/frontend/src/openapi/v2/models/MetadataDefinitionIn.ts b/frontend/src/openapi/v2/models/MetadataDefinitionIn.ts index fc47109a4..9c75bf424 100644 --- a/frontend/src/openapi/v2/models/MetadataDefinitionIn.ts +++ b/frontend/src/openapi/v2/models/MetadataDefinitionIn.ts @@ -42,6 +42,7 @@ import type { MetadataField } from './MetadataField'; export type MetadataDefinitionIn = { name: string; description?: string; + created?: string; context?: Array; context_url?: string; fields: Array; diff --git a/frontend/src/openapi/v2/models/MetadataDefinitionOut.ts b/frontend/src/openapi/v2/models/MetadataDefinitionOut.ts index 8b1791b0e..7d93dc678 100644 --- a/frontend/src/openapi/v2/models/MetadataDefinitionOut.ts +++ b/frontend/src/openapi/v2/models/MetadataDefinitionOut.ts @@ -21,6 +21,7 @@ import type { UserOut } from './UserOut'; export type MetadataDefinitionOut = { name: string; description?: string; + created?: string; context?: Array; context_url?: string; fields: Array; diff --git a/frontend/src/openapi/v2/services/GroupsService.ts b/frontend/src/openapi/v2/services/GroupsService.ts index 587fc642d..b1efef730 100644 --- a/frontend/src/openapi/v2/services/GroupsService.ts +++ b/frontend/src/openapi/v2/services/GroupsService.ts @@ -14,7 +14,7 @@ export class GroupsService { * Get a list of all Groups in the db the user is a member/owner of. * * Arguments: - * skip -- number of initial records to skip (i.e. for pagination) + * skip -- number of initial recoto_list()rds to skip (i.e. for pagination) * limit -- restrict number of records to be returned (i.e. for pagination) * @param skip * @param limit From 3d165f8a5769c4d71cf928b709499f778ec2f68c Mon Sep 17 00:00:00 2001 From: Chen Wang Date: Thu, 12 Oct 2023 10:40:35 -0500 Subject: [PATCH 15/15] fix codegen --- frontend/src/openapi/v2/models/MetadataDefinitionIn.ts | 1 + frontend/src/openapi/v2/models/MetadataDefinitionOut.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/frontend/src/openapi/v2/models/MetadataDefinitionIn.ts b/frontend/src/openapi/v2/models/MetadataDefinitionIn.ts index 55803356a..cde469e4a 100644 --- a/frontend/src/openapi/v2/models/MetadataDefinitionIn.ts +++ b/frontend/src/openapi/v2/models/MetadataDefinitionIn.ts @@ -42,6 +42,7 @@ import type { MetadataField } from './MetadataField'; export type MetadataDefinitionIn = { name: string; description?: string; + created?: string; '@context'?: Array; context_url?: string; fields: Array; diff --git a/frontend/src/openapi/v2/models/MetadataDefinitionOut.ts b/frontend/src/openapi/v2/models/MetadataDefinitionOut.ts index 9c5d624a8..a9f8b2688 100644 --- a/frontend/src/openapi/v2/models/MetadataDefinitionOut.ts +++ b/frontend/src/openapi/v2/models/MetadataDefinitionOut.ts @@ -21,6 +21,7 @@ import type { UserOut } from './UserOut'; export type MetadataDefinitionOut = { name: string; description?: string; + created?: string; '@context'?: Array; context_url?: string; fields: Array;