feature: added context-path support #209
Conversation
|
|
There was a problem hiding this comment.
@xamien thank you for your contribution 🎉
It looks good 👍 I just added some minor suggestions.
Please have a look.
And please have a look at the CLA bot. Maybe, the email of the commit is different from GitHub. See:
Michal Kotek seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
| import org.springframework.context.annotation.Bean; | ||
| import org.springframework.core.env.Environment; |
There was a problem hiding this comment.
The new imports are not used and can be removed.
src/main/resources/application.yaml
Outdated
| servlet: | ||
| context-path: /zeebe-monitor/ |
There was a problem hiding this comment.
Please don't commit this change. Changing the default base path would break the existing deployments and confuses the users 😅
| @@ -30,7 +30,7 @@ | |||
| import java.util.List; | |||
|
|
|||
| @RestController | |||
| @RequestMapping("/api/jobs") | |||
| @RequestMapping("${server.servlet.context-path}api/jobs") | |||
There was a problem hiding this comment.
This is handled by Spring itself. We don't need to add the variable as a prefix here and on the other resources.
For deployments and jobs, the UI doesn't work with the change anymore.
| @Override | ||
| public void configureMessageBroker(MessageBrokerRegistry config) { | ||
| config.enableSimpleBroker("/"); | ||
| config.enableSimpleBroker(base_path); |
There was a problem hiding this comment.
It seems that this change is not required. If I remove it then it still works. Please verify that it is also true for you.
src/main/resources/public/js/app.js
Outdated
| @@ -53,7 +53,7 @@ function reload() { | |||
| } | |||
|
|
|||
| function sendMessage(msg) { | |||
| stompClient.send("/notifications", {}, | |||
| stompClient.send(base_path + "notifications", {}, | |||
There was a problem hiding this comment.
Following DRY ("dont-repeat-yourself"), we could extract the building of the path into a method.
| @@ -99,6 +105,7 @@ public String workflowList(Map<String, Object> model, Pageable pageable) { | |||
|
|
|||
| model.put("workflows", workflows); | |||
| model.put("count", count); | |||
| model.put("context-path", base_path); | |||
There was a problem hiding this comment.
Following DRY, we can extract this part into a method.
|
@saig0 thank you for review. I hope that I worked all comments. |
I follow up on pull request for route prefix #171 using
server.servlet.context-path@shawnsarwar