Add Application migration and instances endpoint #1813
Conversation
knolleary
force-pushed
the
application-migration
branch
from
c5237c0
to
af91919
Compare
| createdAt: raw.createdAt, | ||
| updatedAt: raw.updatedAt, | ||
| links: raw.links | ||
| if (application) { |
There was a problem hiding this comment.
Should this responsible for guarding against application being null? That feels like it belongs in whatever view is calling this view.
There was a problem hiding this comment.
This was self-consistent with the Team view that does the same thing. I'm okay with it as-is.
forge/db/views/Project.js
Outdated
| @@ -33,7 +33,14 @@ module.exports = { | |||
|
|
|||
| const settingsHostnameRow = proj.ProjectSettings?.find((projectSettingsRow) => projectSettingsRow.key === KEY_HOSTNAME) | |||
| result.hostname = settingsHostnameRow?.value || '' | |||
|
|
|||
| if (proj.Application) { | |||
| result.application = { | |||
There was a problem hiding this comment.
Should this also use app.db.views.Application.application?
There was a problem hiding this comment.
app.db.views.Application.application is intended to be the full view of the application.
Here we only want a summary - as we do in the later lines that add a team summary without deferring to a specific view.
There was a problem hiding this comment.
This feels like something that should be centralised e.g. app.db.views.Application.application(application, summary: true) rather than spreading throughout the app
There was a problem hiding this comment.
I have pushed a change to introduce applicationSummary in line with the teamSummary view we already had. In doing so, I've hit the reason for why they weren't currently used. In this code path, proj.Application has already been converted to a vanilla object - it is no longer a database model object. The summary view was intended for use against the database model object.
I've updated the view to check if its a vanilla object or not and to do the required conversion.
It isn't perhaps the cleanest approach, but it works for now and shouldn't hold up progress on the broader Application work.
Co-authored-by: Pez Cuckow <email@pezcuckow.com>
|
I have merged the suggested changes. A couple of the other comments regarding views I'm happy to leave as-is because the current code is relatively self-consistent with other views - and I don't want to get into a big refactoring of the views. |
This is a continuation of the work to introduce the Application concept into the runtime.
It targets the
feat-1735-application-modelbranch which introduces the Application model and basic CRUD API.This PR adds to that branch:
/api/v1/applications/:applicationId/instancesendpoint that lists the instances within an application