Skip to content

Conversation

@omkar-ethz
Copy link
Member

Description

There were two ways to provide app config: the older AppConfigModule - which only read from environment.ts, and the newer AppConfigService - which read from /config, or /assets/config.json or finally environment.ts. Most of the config was migrated to the newer config service.

This PR migrates also the status-banner and help email related config to the new service, hence making the older AppConfigModule unnecessary.

Motivation

Two ways to provide app-config added confusion, this PR fixes it by removing the older way.

Fixes:

  • Added missing tests for AppConfigService

Changes:

  • Moved statusCode, statusMessage and contactEmail config and their initialization logic to AppConfigService
  • Deleted AppConfigModule and its occurences, also updating tests

Tests included/Docs Updated?

  • Included for each change/fix?
  • Passing? (Merge will not be approved unless this is checked)
  • Docs updated?
  • New packages used/requires npm install?

Extra Information/Screenshots

Verified that status-banner and contact email features work with new config:
Screenshot from 2025-08-04 15-10-00

Copy link
Member

@minottic minottic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! Would you mind putting a the banners etc here, just as an example (after the merge and the renovate PR)?

I think this should be a new major (it should trigger the renovate PR in scicatlive, after which we could add these extra fields as an example)

@omkar-ethz omkar-ethz merged commit bbfe5a5 into develop Aug 4, 2025
7 checks passed
@omkar-ethz omkar-ethz deleted the app-config-migrate branch August 4, 2025 13:42
@omkar-ethz
Copy link
Member Author

LGTM, thanks! Would you mind putting a the banners etc here, just as an example (after the merge and the renovate PR)?

I think this should be a new major (it should trigger the renovate PR in scicatlive, after which we could add these extra fields as an example)

Thanks! Sounds good, so I will need to create a new release of landing page. The most recent seems to v3.3 which is also used in scicatlive.
Do you have any advise on the new release version? Should it be v3.4 or v4.1?
(In the meantime i created a scicatlive PR)

@minottic
Copy link
Member

minottic commented Aug 5, 2025

Thanks! Even though it has a breaking change, I have to correct myself and I think it's enough to use 3.4. This because if one used the older prebuilt image, they couldn't anyway take advantage of environment.ts

@omkar-ethz
Copy link
Member Author

Thanks! Even though it has a breaking change, I have to correct myself and I think it's enough to use 3.4. This because if one used the older prebuilt image, they couldn't anyway take advantage of environment.ts

Good point, thanks! I created a release v3.4, but the build-release workflow to publish an image to ghcr isn't running - attempting a fix here: #401

Looking at the changelog this release seems backwards compatible. just a small clarification: environment.ts was supported and is also now. only that it has the lowest precedence (after /config and config.json - which is the same behaviour as v3.3)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants