-
Notifications
You must be signed in to change notification settings - Fork 0
allow morphology get by id to retrieve by legacy id #114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
with ensure_result(error_message="ReconstructionMorphology not found"): | ||
if is_legacy: | ||
decoded_id = urllib.parse.unquote(id_) | ||
query = constrain_to_accessible_entities( | ||
sa.select(ReconstructionMorphology), user_context.project_id | ||
).filter(decoded_id == sa.any_(ReconstructionMorphology.legacy_id)) | ||
else: | ||
uuid_id = uuid.UUID(id_) | ||
query = constrain_to_accessible_entities( | ||
sa.select(ReconstructionMorphology), user_context.project_id | ||
).filter(ReconstructionMorphology.id == uuid_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer another endpoint instead of the conditional logic here, as a matter of fact @GianlucaFicarelli just moved the logic to /service folder in this #111, so maybe we can do the change in /service/morphology. That would also obviate the need for the validate_id dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah good to know,
i did not know that there is such a change, actually I like moving to repos/services
i will wait for the 111-PR to be merged and update this one accordingly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
query (get("") / morphology_query) wouldn't work with the legacy_id perhaps instead of adding another endpoint?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah that would work too, as a matter of fact, technically we don't need get /{id} either and we could add id to the filter, he he.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer this kind of things to be done client side.
Either similarity is broken until we move it to entitycore, or the client performs a query using a predicate like legacy_id==input_url
GET
morphology by id to retrieve by legacy idReconstructionMorphologyRead
(this is needed in the UI for every entity)NOTE: the
GET /{id_}
may not conform to your API design, please lemme know if it should be in another endpointa use case to use the
legacy id
:in the generalization section (this part still use nexus) of the the details page, we list the morpho-similarities of a morphology, the user can click on the morphology card to open the details page, and since we use now the entitycore ID we have to get the mapped id-legacyid from entitycode