-
Notifications
You must be signed in to change notification settings - Fork 896
refactor: Return template version name in the workspace build API #5178
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
Conversation
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.
FE ✅
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.
Looks good so far. Need to update the fakeQuerier
struct to have the new database method GetTemplateVersionByID
to fulfill the database.Store
interface. That will make lint and tests start passing.
@@ -51,6 +51,7 @@ func (api *API) workspaceBuild(rw http.ResponseWriter, r *http.Request) { | |||
data.metadata, | |||
data.agents, | |||
data.apps, | |||
data.templateVersions[0], |
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.
This will panic if the length is not at least 1. That may be fine if we have the expectation that this is always true, but wanted to point it out.
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.
Yes, we always expect to have at least 1. Thanks for pointing it out.
The UI will use the template version name in some places so we need to have it returned from the API. I tried doing a JOIN but it looks like it does not update the
workspaceBuild
model properly. We are running some queries to get extra "workspace build" data so I made it there but I'm not sure if it is the best approach.