Add reference descriptions and make them and names updateable [AS-512]#221
Add reference descriptions and make them and names updateable [AS-512]#221zarsky-broad merged 8 commits intodevfrom
Conversation
davidangb
left a comment
davidangb
left a comment
There was a problem hiding this comment.
first-pass comments inline!
| new StringJoiner( | ||
| ", ", | ||
| "UPDATE workspace_data_reference SET ", | ||
| " WHERE reference_id = :id AND workspace_id = :workspace_id"); |
There was a problem hiding this comment.
using a StringJoiner here gets the job done and is a clever choice ... but it feels less readable imho than a StringBuilder. I would prefer readability ... but I also won't die on this hill.
There was a problem hiding this comment.
I liked the StringJoiner because it's extensible for the future, if we ever have other updatable properties.
There was a problem hiding this comment.
(plus, using StringBuilder would mean doing a whole nested case thing to add the comma...)
src/main/java/bio/terra/workspace/service/datareference/DataReferenceService.java
Show resolved
Hide resolved
src/test/java/bio/terra/workspace/service/datareference/DataReferenceServiceTest.java
Outdated
Show resolved
Hide resolved
| '404': | ||
| $ref: '#/components/responses/NotFound' | ||
| '500': | ||
| $ref: '#/components/responses/ServerError' |
There was a problem hiding this comment.
This satisfies the current Jira ticket, but from an API design perspective it's awkward that we can create and get entire data references, but we can only edit the name/description of those references - we can't edit the innards, such as pointing an existing reference at a different snapshot id. I'm not sure the best way to handle this - possibly just explaining it in the API description.
ddietterich
left a comment
ddietterich
left a comment
There was a problem hiding this comment.
Comment, suggestions, and nits
src/main/java/bio/terra/workspace/app/controller/WorkspaceApiController.java
Outdated
Show resolved
Hide resolved
src/main/java/bio/terra/workspace/app/controller/WorkspaceApiController.java
Show resolved
Hide resolved
davidangb
left a comment
davidangb
left a comment
There was a problem hiding this comment.
thanks for the changes!
3 participants
Once this goes through, I'll publish an updated client and update Rawls.
One note:
descriptionis a protected key for flights, so I've standardized onreferenceDescriptionso the name can be the same everywhere.