Skip to content

feat: configurable about page#2396

Open
nitrosx wants to merge 2 commits into
masterfrom
configurable_about_page
Open

feat: configurable about page#2396
nitrosx wants to merge 2 commits into
masterfrom
configurable_about_page

Conversation

@nitrosx
Copy link
Copy Markdown
Member

@nitrosx nitrosx commented May 28, 2026

Description

This PR adds the feature to configure the about page with a customized html text, which ca nbe inserted in configuration or in a dedicated file. If the file is selected, it can be mounted in directly in the container

Motivation

the current about page contains outdated text and hard coded text.

Changes:

Please provide a list of the changes implemented by this PR

  • about.component.html
  • about.component.ts
  • configuration file
  • test file about-page.cy.js

Tests included

  • [ x ] Included for each change/fix?
  • Passing? (Merge will not be approved unless this is checked)

Documentation

  • swagger documentation updated [required]
  • official documentation updated [nice-to-have]

official documentation info

If you have updated the official documentation, please provide PR # and URL of the pages where the updates are included

Backend version

  • Does it require a specific version of the backend
  • which version of the backend is required:

AI contribution

This PR contains code created with mistral.ai (chat.mistral.ai, version n/a)

Summary by Sourcery

Make the About page content configurable via HTML supplied from application configuration or an external file, replacing hard-coded facility-specific text.

New Features:

  • Support configurable HTML content for the About page, loaded from application configuration or an external HTML file at runtime.

Enhancements:

  • Simplify the About page component template and logic to render a single block of configurable HTML content.

@nitrosx nitrosx requested a review from a team as a code owner May 28, 2026 20:18
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 2 issues, and left some high level feedback:

  • The new AppConfigInterface properties (helpEnabled, infoEnabled, infoHtmlFile, infoHtmlContent) are non-optional, which may break existing configs; consider marking them optional or assigning defaults in initializeApp to keep backward compatibility.
  • AboutComponent uses bypassSecurityTrustHtml on config-provided HTML; if this content can be edited by admins or comes from outside the trusted codebase, consider introducing basic sanitization/validation or documenting the trust assumptions to avoid XSS risks.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new AppConfigInterface properties (helpEnabled, infoEnabled, infoHtmlFile, infoHtmlContent) are non-optional, which may break existing configs; consider marking them optional or assigning defaults in initializeApp to keep backward compatibility.
- AboutComponent uses bypassSecurityTrustHtml on config-provided HTML; if this content can be edited by admins or comes from outside the trusted codebase, consider introducing basic sanitization/validation or documenting the trust assumptions to avoid XSS risks.

## Individual Comments

### Comment 1
<location path="src/app/app-config.service.ts" line_range="175-178" />
<code_context>
   statusBannerMessage?: string;
   statusBannerCode?: "INFO" | "WARN";
   autoApplyFilters?: boolean;
+  helpEnabled: boolean;
+  infoEnabled: boolean;
+  infoHtmlFile: string;
+  infoHtmlContent: string;
 }

</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider making new config fields optional or providing defaults for backward compatibility.

These new properties (`helpEnabled`, `infoEnabled`, `infoHtmlFile`, `infoHtmlContent`) are required on `AppConfigInterface`, but existing `config.json` files won’t define them. This makes the type inconsistent with real configs and can lead to confusing type narrowing or extra boilerplate. Consider marking them as optional (`?:`) or initializing them to sane defaults so the interface accurately matches what’s actually present at runtime.

Suggested implementation:

```typescript
  statusBannerMessage?: string;
  statusBannerCode?: "INFO" | "WARN";
  autoApplyFilters?: boolean;
  helpEnabled?: boolean;
  infoEnabled?: boolean;
  infoHtmlFile?: string;
  infoHtmlContent?: string;
}

```

```typescript
    if (config.helpEnabled == null) {
      config.helpEnabled = false;
    }

    if (config.infoEnabled == null) {
      config.infoEnabled = false;
    }

    if (!config.infoHtmlFile) {
      // Default info HTML file path for backward compatibility; adjust as needed.
      config.infoHtmlFile = 'assets/info.html';
    }

    if (!config.infoHtmlContent && config.infoHtmlFile) {
      try {
        config.infoHtmlContent = await firstValueFrom(
          this.http.get(config.infoHtmlFile, { responseType: 'text' }).pipe(timeout(2000)),
        );
      } catch (err) {

```

Depending on how `helpEnabled` and `infoEnabled` are used elsewhere in the app, you may also want to:
1. Replace direct property access (`config.helpEnabled`) with null-safe defaults (e.g. `config.helpEnabled ?? false`) where appropriate.
2. Update any tests that assume these fields are required so they either set explicit values or assert against the new defaults.
</issue_to_address>

### Comment 2
<location path="src/app/app-config.service.ts" line_range="288-291" />
<code_context>
       config.datasetPageSizeOptions = [5, 10, 25, 100];
     }

+    if (!config.infoHtmlContent) {
+      try {
+        config.infoHtmlContent = await firstValueFrom(
+          this.http.get(config.infoHtmlFile,{ responseType: 'text' }).pipe(timeout(2000)),
+        );
+      } catch (err) {
</code_context>
<issue_to_address>
**issue:** Guard against missing `infoHtmlFile` and consider honoring `infoEnabled`.

If `infoHtmlFile` is missing in `config.json`, this will call `http.get(undefined)` and hit `'/undefined'` before the catch assigns the default. To avoid noisy 404s and better use `infoEnabled`, only call `http.get` when `infoHtmlFile` is truthy (and/or `infoEnabled` is true); otherwise assign the default HTML directly.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +175 to +178
helpEnabled: boolean;
infoEnabled: boolean;
infoHtmlFile: string;
infoHtmlContent: string;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion (bug_risk): Consider making new config fields optional or providing defaults for backward compatibility.

These new properties (helpEnabled, infoEnabled, infoHtmlFile, infoHtmlContent) are required on AppConfigInterface, but existing config.json files won’t define them. This makes the type inconsistent with real configs and can lead to confusing type narrowing or extra boilerplate. Consider marking them as optional (?:) or initializing them to sane defaults so the interface accurately matches what’s actually present at runtime.

Suggested implementation:

  statusBannerMessage?: string;
  statusBannerCode?: "INFO" | "WARN";
  autoApplyFilters?: boolean;
  helpEnabled?: boolean;
  infoEnabled?: boolean;
  infoHtmlFile?: string;
  infoHtmlContent?: string;
}
    if (config.helpEnabled == null) {
      config.helpEnabled = false;
    }

    if (config.infoEnabled == null) {
      config.infoEnabled = false;
    }

    if (!config.infoHtmlFile) {
      // Default info HTML file path for backward compatibility; adjust as needed.
      config.infoHtmlFile = 'assets/info.html';
    }

    if (!config.infoHtmlContent && config.infoHtmlFile) {
      try {
        config.infoHtmlContent = await firstValueFrom(
          this.http.get(config.infoHtmlFile, { responseType: 'text' }).pipe(timeout(2000)),
        );
      } catch (err) {

Depending on how helpEnabled and infoEnabled are used elsewhere in the app, you may also want to:

  1. Replace direct property access (config.helpEnabled) with null-safe defaults (e.g. config.helpEnabled ?? false) where appropriate.
  2. Update any tests that assume these fields are required so they either set explicit values or assert against the new defaults.

Comment on lines +288 to +291
if (!config.infoHtmlContent) {
try {
config.infoHtmlContent = await firstValueFrom(
this.http.get(config.infoHtmlFile,{ responseType: 'text' }).pipe(timeout(2000)),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

issue: Guard against missing infoHtmlFile and consider honoring infoEnabled.

If infoHtmlFile is missing in config.json, this will call http.get(undefined) and hit '/undefined' before the catch assigns the default. To avoid noisy 404s and better use infoEnabled, only call http.get when infoHtmlFile is truthy (and/or infoEnabled is true); otherwise assign the default HTML directly.

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.

1 participant