[OGUI-1054] Status routes convention #2743
Conversation
| @@ -38,6 +38,8 @@ function buildPublicConfig(config) { | |||
| REFRESH_MIN_INTERVAL: config?.consul?.refreshRate?.min || 10, | |||
| REFRESH_MAX_INTERVAL: config?.consul?.refreshRate?.max || 120, | |||
| CONSUL_SERVICE: config.consul ? true : false, | |||
| ABOUT_COMPONENTS: ['qcg', 'qc', 'ccdb'], | |||
There was a problem hiding this comment.
Given that we have an enum for this, would it not be better to use that instead of hardcoding the values here?
There was a problem hiding this comment.
You’re right! I have a bad memory, and I had forgotten that I had already created an enum for the integrated services
QualityControl/lib/api.js
Outdated
| http.get( | ||
| '/status/:component', | ||
| statusComponentMiddleware, | ||
| statusController.getFrameworkInfo.bind(statusController), |
There was a problem hiding this comment.
I believe the method name here has two issues:
- name is not representative, you are retrieving the status of a component which is defined in the URL
- for controllers, let's try to keep the convention of also ending in
handleras name
| export const statusComponentMiddleware = (req, res, next) => { | ||
| try { | ||
| if (!req.params.component) { | ||
| throw new InvalidInputError('Component parameter is missing'); |
There was a problem hiding this comment.
If there is a missing component parameter, the URL will not be matched on this API route.
Thus, this if will never be triggered and it is unused code
There was a problem hiding this comment.
done
| this.item = RemoteData.failure(result.message); | ||
| try { | ||
| const results = {}; | ||
| await Promise.all(components.map(async (component) => { |
There was a problem hiding this comment.
Do I understand correctly that these are just migration changes and the changes for front-end will come in a future PR?
Because in these changes, the status cards will still be loaded all at one time compared to the requirement of the ticket to load independently
…ure/OGUI-1054/status-routes-convention
…ure/OGUI-1054/status-routes-convention
This PR: