Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(other): copy all constants in backend when branding #6853

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
5 changes: 5 additions & 0 deletions backend/src/branding/groupsBranding.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// this file is duplicated in backend/src/branding/groupsBranding' and 'webapp/branding/groupsBranding'
// configurable values see: 'backend/src/constants/groupsDefault', 'webapp/constants/groupsDefault'
// configurable values see: 'backend/src/constants/groups', 'webapp/constants/groups'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// configurable values see: 'backend/src/constants/groups', 'webapp/constants/groups'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can remove this comments. 👍🏼


export const DUMMY = '' // to avoid import error ts(2306): File X is not a module.
13 changes: 10 additions & 3 deletions backend/src/constants/groups.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
// this file is duplicated in `backend/src/constants/group` and `webapp/constants/group.js`
export const DESCRIPTION_WITHOUT_HTML_LENGTH_MIN = 3 // with removed HTML tags
export const DESCRIPTION_EXCERPT_HTML_LENGTH = 250 // with removed HTML tags
// configurable values see: 'backend/src/constants/groupsDefault', 'webapp/constants/groupsDefault'
// configurable values see: 'backend/src/constants/groups', 'webapp/constants/groups'
Comment on lines +1 to +2
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// configurable values see: 'backend/src/constants/groupsDefault', 'webapp/constants/groupsDefault'
// configurable values see: 'backend/src/constants/groups', 'webapp/constants/groups'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can remove this comments. 👍🏼


import * as groupsDefault from './groupsDefault'
import * as groupsBranding from '../branding/groupsBranding'

export const { DESCRIPTION_WITHOUT_HTML_LENGTH_MIN, DESCRIPTION_EXCERPT_HTML_LENGTH } = {
...groupsDefault,
...groupsBranding,
}
Comment on lines +7 to +10
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why export specific variables? Is there a typescript advantage doing this?

Suggested change
export const { DESCRIPTION_WITHOUT_HTML_LENGTH_MIN, DESCRIPTION_EXCERPT_HTML_LENGTH } = {
...groupsDefault,
...groupsBranding,
}
export default {
...groupsDefault,
...groupsBranding,
}

Copy link
Member

@Tirokk Tirokk Jan 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did it this way because this export is imported as:

import { DESCRIPTION_WITHOUT_HTML_LENGTH_MIN } from '../../constants/groups'

So your suggestion creates an error on this import:

src/middleware/excerptMiddleware.ts:2:10 - error TS2614: Module '"../constants/groups"' has no exported member 'DESCRIPTION_EXCERPT_HTML_LENGTH'. Did you mean to use 'import DESCRIPTION_EXCERPT_HTML_LENGTH from "../constants/groups"' instead?

It's not imported as whole object where a property is selected.

The way I do it has the following good sides:

  • if I go on refactoring I have not to rewrite all the imports
  • it's clear which configs can be passed through and all others are filtered out

That's why I tend to leave my code as it is. Is that all right? @ulfgebhardt

6 changes: 6 additions & 0 deletions backend/src/constants/groupsDefault.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// this file is duplicated in 'backend/src/constants/groupsDefault' and 'webapp/constants/groupsDefault'
// configurable values see: 'backend/src/constants/groupsDefault', 'webapp/constants/groupsDefault'
// configurable values see: 'backend/src/constants/groups', 'webapp/constants/groups'
Comment on lines +2 to +3
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// configurable values see: 'backend/src/constants/groupsDefault', 'webapp/constants/groupsDefault'
// configurable values see: 'backend/src/constants/groups', 'webapp/constants/groups'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can remove this comments. 👍🏼


export const DESCRIPTION_WITHOUT_HTML_LENGTH_MIN = 3 // with removed HTML tags
export const DESCRIPTION_EXCERPT_HTML_LENGTH = 250 // with removed HTML tags
2 changes: 2 additions & 0 deletions backend/src/schema/resolvers/groups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import Resolver, {
import { mergeImage } from './images/images'
import { createOrUpdateLocations } from './users/location'

// Wolle console.log('DESCRIPTION_WITHOUT_HTML_LENGTH_MIN: ', DESCRIPTION_WITHOUT_HTML_LENGTH_MIN)

Comment on lines +14 to +15
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Wolle console.log('DESCRIPTION_WITHOUT_HTML_LENGTH_MIN: ', DESCRIPTION_WITHOUT_HTML_LENGTH_MIN)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I's a work in progress comment. I'll remove it later.

export default {
Query: {
Group: async (_object, params, context, _resolveInfo) => {
Expand Down
18 changes: 15 additions & 3 deletions deployment/src/docker/backend.Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,22 @@ FROM $APP_IMAGE_CODE as code

ARG CONFIGURATION=example

# alt Wolle # copy public constants and email templates into the Docker image to brand it
# COPY configurations/${CONFIGURATION}/branding/constants/emails.ts src/config/
# COPY configurations/${CONFIGURATION}/branding/constants/logos.ts src/config/
# COPY configurations/${CONFIGURATION}/branding/constants/metadata.ts src/config/
# COPY configurations/${CONFIGURATION}/branding/email/ src/middleware/helpers/email/

# Wolle # copy public constants and email templates into the Docker image to brand it
# COPY configurations/${CONFIGURATION}/branding/constants/ src/config/
# COPY configurations/${CONFIGURATION}/branding/constants/ src/constants/
# # links.ts does only work in frontend, not backend
# RUN rm -Rf src/config/links.ts src/constants/links.ts
# COPY configurations/${CONFIGURATION}/branding/email/ src/middleware/helpers/email/

Comment on lines +14 to +26
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# alt Wolle # copy public constants and email templates into the Docker image to brand it
# COPY configurations/${CONFIGURATION}/branding/constants/emails.ts src/config/
# COPY configurations/${CONFIGURATION}/branding/constants/logos.ts src/config/
# COPY configurations/${CONFIGURATION}/branding/constants/metadata.ts src/config/
# COPY configurations/${CONFIGURATION}/branding/email/ src/middleware/helpers/email/
# Wolle # copy public constants and email templates into the Docker image to brand it
# COPY configurations/${CONFIGURATION}/branding/constants/ src/config/
# COPY configurations/${CONFIGURATION}/branding/constants/ src/constants/
# # links.ts does only work in frontend, not backend
# RUN rm -Rf src/config/links.ts src/constants/links.ts
# COPY configurations/${CONFIGURATION}/branding/email/ src/middleware/helpers/email/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I's a work in progress comment. I'll remove it later.

# copy public constants and email templates into the Docker image to brand it
COPY configurations/${CONFIGURATION}/branding/constants/emails.ts src/config/
COPY configurations/${CONFIGURATION}/branding/constants/logos.ts src/config/
COPY configurations/${CONFIGURATION}/branding/constants/metadata.ts src/config/
COPY configurations/${CONFIGURATION}/branding/config-all/ src/branding/
COPY configurations/${CONFIGURATION}/branding/config-backend/ src/branding/
COPY configurations/${CONFIGURATION}/branding/email/ src/middleware/helpers/email/

##################################################################################
Expand Down
7 changes: 5 additions & 2 deletions deployment/src/docker/maintenance.Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@ ARG CONFIGURATION=example

# copy public constants into the Docker image to brand it
COPY configurations/${CONFIGURATION}/branding/static/ static/
COPY configurations/${CONFIGURATION}/branding/constants/ constants/
RUN /bin/sh -c 'cd constants && for f in *.ts; do mv -- "$f" "${f%.ts}.js"; done'
# Wolle: COPY configurations/${CONFIGURATION}/branding/constants/ constants/
# Wolle: RUN /bin/sh -c 'cd constants && for f in *.ts; do mv -- "$f" "${f%.ts}.js"; done'
Comment on lines +16 to +17
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Wolle: COPY configurations/${CONFIGURATION}/branding/constants/ constants/
# Wolle: RUN /bin/sh -c 'cd constants && for f in *.ts; do mv -- "$f" "${f%.ts}.js"; done'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I's a work in progress comment. I'll remove it later.

COPY configurations/${CONFIGURATION}/branding/config-all/ branding/
COPY configurations/${CONFIGURATION}/branding/config-webapp/ branding/
RUN /bin/sh -c 'cd branding && for f in *.ts; do mv -- "$f" "${f%.ts}.js"; done'

# locales
COPY configurations/${CONFIGURATION}/branding/locales/*.json locales/tmp/
Expand Down
6 changes: 4 additions & 2 deletions deployment/src/docker/webapp.Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ ARG CONFIGURATION=example

# copy public constants into the Docker image to brand it
COPY configurations/${CONFIGURATION}/branding/static/ static/
COPY configurations/${CONFIGURATION}/branding/constants/ constants/
RUN /bin/sh -c 'cd constants && for f in *.ts; do mv -- "$f" "${f%.ts}.js"; done'
# Wolle: COPY configurations/${CONFIGURATION}/branding/constants/ constants/
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Wolle: COPY configurations/${CONFIGURATION}/branding/constants/ constants/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I's a work in progress comment. I'll remove it later.

COPY configurations/${CONFIGURATION}/branding/config-all/ branding/
COPY configurations/${CONFIGURATION}/branding/config-webapp/ branding/
RUN /bin/sh -c 'cd branding && for f in *.ts; do mv -- "$f" "${f%.ts}.js"; done'
COPY configurations/${CONFIGURATION}/branding/locales/html/ locales/html/
COPY configurations/${CONFIGURATION}/branding/assets/styles/imports/ assets/styles/imports/
COPY configurations/${CONFIGURATION}/branding/assets/fonts/ assets/fonts/
Expand Down
1 change: 1 addition & 0 deletions webapp/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ COPY --from=build ${DOCKER_WORKDIR}/nuxt.config.js ./nuxt.config.js
# Copy static files
# TODO - this seems not be needed anymore for the new rebranding
# TODO - this should be one Folder containign all stuff needed to be copied
# Wolle: what shall happen here?
COPY --from=build ${DOCKER_WORKDIR}/config/ ./config/
COPY --from=build ${DOCKER_WORKDIR}/constants ./constants
COPY --from=build ${DOCKER_WORKDIR}/static ./static
Comment on lines +100 to 103
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

???

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a leftover of the former deployment to me and is probably superfluous!?

Expand Down
5 changes: 5 additions & 0 deletions webapp/branding/groupsBranding.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// this file is duplicated in backend/src/branding/groupsBranding' and 'webapp/branding/groupsBranding'
// configurable values see: 'backend/src/constants/groupsDefault', 'webapp/constants/groupsDefault'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// configurable values see: 'backend/src/constants/groupsDefault', 'webapp/constants/groupsDefault'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can remove this comments. 👍🏼

// configurable values see: 'backend/src/constants/groups', 'webapp/constants/groups'

export const DUMMY = '' // to avoid import error ts(2306): File X is not a module.
4 changes: 4 additions & 0 deletions webapp/components/Group/GroupForm.vue
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,10 @@ import Editor from '~/components/Editor/Editor'
import ActionRadiusSelect from '~/components/Select/ActionRadiusSelect'
import { queryLocations } from '~/graphql/location'

// Wolle console.log('NAME_LENGTH_MIN: ', NAME_LENGTH_MIN)
// console.log('NAME_LENGTH_MAX: ', NAME_LENGTH_MAX)
// console.log('DESCRIPTION_WITHOUT_HTML_LENGTH_MIN: ', DESCRIPTION_WITHOUT_HTML_LENGTH_MIN)

Comment on lines +181 to +184
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Wolle console.log('NAME_LENGTH_MIN: ', NAME_LENGTH_MIN)
// console.log('NAME_LENGTH_MAX: ', NAME_LENGTH_MAX)
// console.log('DESCRIPTION_WITHOUT_HTML_LENGTH_MIN: ', DESCRIPTION_WITHOUT_HTML_LENGTH_MIN)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I's a work in progress comment. I'll remove it later.

let timeout

export default {
Expand Down
17 changes: 12 additions & 5 deletions webapp/constants/groups.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
// this file is duplicated in `backend/src/constants/group.js` and `webapp/constants/group.js`
export const NAME_LENGTH_MIN = 3
export const NAME_LENGTH_MAX = 50
export const DESCRIPTION_WITHOUT_HTML_LENGTH_MIN = 3 // with removed HTML tags
export const SHOW_GROUP_BUTTON_IN_HEADER = true
// configurable values see: 'backend/src/constants/groupsDefault', 'webapp/constants/groupsDefault'
// configurable values see: 'backend/src/constants/groups', 'webapp/constants/groups'

Comment on lines +1 to +3
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// configurable values see: 'backend/src/constants/groupsDefault', 'webapp/constants/groupsDefault'
// configurable values see: 'backend/src/constants/groups', 'webapp/constants/groups'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can remove this comments. 👍🏼

import * as groupsDefault from './groupsDefault'
import * as groupsBranding from '../branding/groupsBranding'

export const {
NAME_LENGTH_MIN,
NAME_LENGTH_MAX,
DESCRIPTION_WITHOUT_HTML_LENGTH_MIN,
SHOW_GROUP_BUTTON_IN_HEADER,
} = { ...groupsDefault, ...groupsBranding }
Comment on lines +7 to +12
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export const {
NAME_LENGTH_MIN,
NAME_LENGTH_MAX,
DESCRIPTION_WITHOUT_HTML_LENGTH_MIN,
SHOW_GROUP_BUTTON_IN_HEADER,
} = { ...groupsDefault, ...groupsBranding }
export default { ...groupsDefault, ...groupsBranding }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

8 changes: 8 additions & 0 deletions webapp/constants/groupsDefault.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// this file is duplicated in 'backend/src/constants/groupsDefault' and 'webapp/constants/groupsDefault'
// configurable values see: 'backend/src/constants/groupsDefault', 'webapp/constants/groupsDefault'
// configurable values see: 'backend/src/constants/groups', 'webapp/constants/groups'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// configurable values see: 'backend/src/constants/groups', 'webapp/constants/groups'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can remove this comments. 👍🏼


export const NAME_LENGTH_MIN = 3
export const NAME_LENGTH_MAX = 50
export const DESCRIPTION_WITHOUT_HTML_LENGTH_MIN = 3 // with removed HTML tags
export const SHOW_GROUP_BUTTON_IN_HEADER = true