fix(backend): derive INVITE_LINK_URL and BBB_LOGOUT_URL from FRONTEND_URL #1820
Conversation
Bettelstab
changed the title
backend/src/config/config.ts
Outdated
| FRONTEND_INVITE_LINK_URL: FRONTEND_URL + 'join-table/', | ||
| FRONTEND_BBB_LOGOUT_URL: FRONTEND_URL + 'table-closed/', |
There was a problem hiding this comment.
Yes, great idea, however I would completely remove these configuration variables. The frontend already knows its hostname. So, wherever FRONTEND_INVITE_LINK_URL is being used, that's the location where we would build a url. Only exception is if we have to reuse that link somewhere. If it's used only once, build it there.
There was a problem hiding this comment.
Yes, but this is for the Backend :)
Bettelstab
dismissed
roschaefer’s stale review
Implemented the URL construction The other comment would be true for frontend config only.
There was a problem hiding this comment.
This is what I mean. The config variable is used only once. Please create the variable in TableResolver.ts. In the TableResolver.spec.ts you can set a hard-coded value.
There is a backend-deployment.yaml, where you can remove the environment variable, and the backend-deployment.yaml links to a configmap which consumes all data from values.yaml, where you can remove the corresponding frontend_invite_link_url as well.
It's an easy change in the deployment configuration, do you feel OK to take care of that?
Remove FRONTEND_BBB_LOGOUT_URL and FRONTEND_INVITE_LINK_URL from config and calculate them where there are used
Bettelstab
dismissed
roschaefer’s stale review
Changes implemented as requested
| @@ -18,7 +18,7 @@ const getMeetingsMock = jest.mocked(getMeetings) | |||
|
|
|||
| let testServer: ApolloServer<Context> | |||
|
|
|||
| CONFIG.FRONTEND_INVITE_LINK_URL = '/' | |||
| CONFIG.FRONTEND_URL = 'https://my.frontend.url' | |||
There was a problem hiding this comment.
Well, this one is global state being mutated in a test suite. I.e. it will affect other test suites that get executed afterwards.
Anyways, I think this is for another PR. There was an assignment to CONFIG.FRONTEND_INVITE_LINK_URL already.
🍰 Pullrequest
In our kubernetes Deployment, the environment variable FRONTEND_BBB_LOGOUT_URL was missing. Therefore the fix for #1261 did not work.
Instead of adding the environment variable, I propose to simplify the config and derive its value, as well as the one for invite links, from the FRONTEND_URL.