Add openshiftMasterURL to list of builds #70
Conversation
psturc
changed the title
pkg/web/buildsHandler.go
Outdated
| return c.JSON(http.StatusOK, builds) | ||
| response := &response{ | ||
| BuildList: *builds, | ||
| OpenshiftMasterURL: mbh.openshiftMasterURL, |
There was a problem hiding this comment.
rather than return the url on its own, I think maybe use it to construct the url of each build, and then add the url to each build as a separate field (like buildURL)? There are at least 2 benefits:
- Cleaner API. Otherwise without some documentation, people wouldn't be able to understand why the master url is returned here.
- Hide the logic to build the full url of the builds. This only needs to be done on the server and the clients don't need to care.
WDYT?
There was a problem hiding this comment.
I think it sounds reasonable. This will definitely reduce amount of changes needed in UI code to reflect this, which I'm looking at right now. Thanks
pkg/web/buildsHandler.go
Outdated
|
|
||
| // Extend existing struct buildV1.BuildList so each build contains URL | ||
| // pointing to build in OpenShift Console | ||
| func extendBuildList(bl buildV1.BuildList, mbh MobileBuildsHandler) *extendedBuildList { |
There was a problem hiding this comment.
don't think we need to pass the MobileBuildsHandler object here. It's better to just pass the url and the namespace value as args explicitly here.
pkg/web/buildsHandler.go
Outdated
| BuildURL: buildURL, | ||
| } | ||
| } | ||
| return &extendedBuildList{ |
There was a problem hiding this comment.
what do you think of moving the extra field (and new types) to the BuildCRUDL class? The end result is the same, but I think they belong to the our core business, which is defined in the mobile package. The web handlers here should only be responsible for things like input validation, data transformation and error handling etc.
Fixes - addressing comments
Motivation
https://issues.jboss.org/browse/AEROGEAR-7979
How to verify
💁♂️ Feel free to use my project on community cluster (ping me for user/pass if you don't have it)
Build #x) are pointing to the correct page in OpenShift Console (also check those in build history)