Skip to content

Commit 85bbd2d

Browse files
KasinhouMatus Kasakclaude
authored
ZCU-PUB/fix(static-page): return HTTP 404 for missing static pages (#1462)
* ZCU-PUB/fix(static-page): return HTTP 404 for missing static pages StaticPageComponent rendered an empty shell and answered HTTP 200 when a `/static/<file>` page did not exist: it tried to load `static-files/error.html` and, when that was empty/missing, showed nothing and never set a 404 status. UNIVERSAL-016 (dspace-ui-tests notFoundPage.spec.ts) therefore failed on the "non-existent static page shows 404 page" case. Set the SSR response to 404 via ServerResponseService and render the inline 404 page (same markup + reused `404.*` i18n keys as PageNotFoundComponent) when the content is not found. Drop the legacy error.html loading path. Behaviour now matches dtq-dev: /static/<missing> returns 404 with the "404 / Take me to the home page" page. Refs dataquest-dev/dspace-customers#566 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ZCU-PUB/test(static-page): type HtmlContentService spy (review nit) Use jasmine.SpyObj<HtmlContentService> instead of `any` for the test spy, per Copilot review. Refs dataquest-dev/dspace-customers#566 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Matus Kasak <matus.kasak@dataquest.sk> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent c5c4834 commit 85bbd2d

3 files changed

Lines changed: 51 additions & 21 deletions

File tree

‎src/app/static-page/static-page.component.html‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,21 @@
1-
<div class="container" >
1+
<!-- Loading spinner while the static file is being fetched -->
2+
<div class="container text-center my-5" *ngIf="contentState === 'loading'">
3+
<ds-themed-loading [spinner]="true" [showMessage]="false"></ds-themed-loading>
4+
</div>
5+
6+
<!-- Show static page content when found -->
7+
<div class="container" *ngIf="contentState === 'found'">
28
<div [innerHTML]="(htmlContent | async) | dsSafeHtml" (click)="processLinks($event)"></div>
39
</div>
10+
11+
<!-- Show 404 error when content not found (matches PageNotFoundComponent design) -->
12+
<div class="container page-not-found" *ngIf="contentState === 'not-found'">
13+
<h1>404</h1>
14+
<h2><small>{{"404.page-not-found" | translate}}</small></h2>
15+
<br/>
16+
<p>{{"404.help" | translate}}</p>
17+
<br/>
18+
<p class="text-center">
19+
<a routerLink="/home" class="btn btn-primary">{{"404.link.home-page" | translate}}</a>
20+
</p>
21+
</div>

‎src/app/static-page/static-page.component.spec.ts‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import { ComponentFixture, TestBed } from '@angular/core/testing';
2+
import { CommonModule } from '@angular/common';
3+
import { NO_ERRORS_SCHEMA } from '@angular/core';
24

35
import { StaticPageComponent } from './static-page.component';
46
import { HtmlContentService } from '../shared/html-content.service';
@@ -10,13 +12,15 @@ import { of } from 'rxjs';
1012
import { APP_CONFIG } from '../../config/app-config.interface';
1113
import { environment } from '../../environments/environment';
1214
import { ClarinSafeHtmlPipe } from '../shared/utils/clarin-safehtml.pipe';
15+
import { ServerResponseService } from '../core/services/server-response.service';
1316

1417
describe('StaticPageComponent', () => {
1518
let component: StaticPageComponent;
1619
let fixture: ComponentFixture<StaticPageComponent>;
1720

18-
let htmlContentService: HtmlContentService;
21+
let htmlContentService: jasmine.SpyObj<HtmlContentService>;
1922
let localeService: any;
23+
let responseService: jasmine.SpyObj<ServerResponseService>;
2024
let appConfig: any;
2125

2226
beforeEach(async () => {
@@ -26,6 +30,7 @@ describe('StaticPageComponent', () => {
2630
localeService = jasmine.createSpyObj('LocaleService', {
2731
getCurrentLanguageCode: jasmine.createSpy('getCurrentLanguageCode'),
2832
});
33+
responseService = jasmine.createSpyObj('responseService', ['setNotFound']);
2934

3035
// Do not mutate the shared `environment` object - replacing `environment.ui` would
3136
// break any later spec that reads e.g. environment.ui.nameSpace
@@ -38,14 +43,17 @@ describe('StaticPageComponent', () => {
3843
TestBed.configureTestingModule({
3944
declarations: [ StaticPageComponent, ClarinSafeHtmlPipe ],
4045
imports: [
46+
CommonModule,
4147
TranslateModule.forRoot()
4248
],
4349
providers: [
4450
{ provide: HtmlContentService, useValue: htmlContentService },
4551
{ provide: Router, useValue: new RouterMock() },
4652
{ provide: LocaleService, useValue: localeService },
53+
{ provide: ServerResponseService, useValue: responseService },
4754
{ provide: APP_CONFIG, useValue: appConfig }
48-
]
55+
],
56+
schemas: [NO_ERRORS_SCHEMA]
4957
});
5058

5159
localeService = TestBed.inject(LocaleService);
@@ -65,5 +73,14 @@ describe('StaticPageComponent', () => {
6573
it('should load html file content', async () => {
6674
await component.ngOnInit();
6775
expect(component.htmlContent.value).toBe('<div id="idShouldNotBeRemoved">TEST MESSAGE</div>');
76+
expect(component.contentState).toBe('found');
77+
});
78+
79+
// When the file is missing, set a 404 status for SSR and switch to the not-found state
80+
it('should set 404 status when content is not found', async () => {
81+
htmlContentService.fetchHtmlContent.and.returnValue(of(''));
82+
await component.ngOnInit();
83+
expect(responseService.setNotFound).toHaveBeenCalled();
84+
expect(component.contentState).toBe('not-found');
6885
});
6986
});

‎src/app/static-page/static-page.component.ts‎

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
1-
import { Component, Inject, OnInit } from '@angular/core';
1+
import { ChangeDetectorRef, Component, Inject, OnInit } from '@angular/core';
22
import { HtmlContentService } from '../shared/html-content.service';
33
import { BehaviorSubject, firstValueFrom } from 'rxjs';
44
import { Router } from '@angular/router';
55
import { isEmpty, isNotEmpty } from '../shared/empty.util';
66
import { LocaleService } from '../core/locale/locale.service';
77
import {
88
HTML_SUFFIX,
9-
STATIC_FILES_DEFAULT_ERROR_PAGE_PATH,
109
STATIC_FILES_PROJECT_PATH, STATIC_PAGE_PATH
1110
} from './static-page-routing-paths';
1211
import { APP_CONFIG, AppConfig } from '../../config/app-config.interface';
12+
import { ServerResponseService } from '../core/services/server-response.service';
1313

1414
/**
1515
* Component which load and show static files from the `static-files` folder.
@@ -23,10 +23,13 @@ import { APP_CONFIG, AppConfig } from '../../config/app-config.interface';
2323
export class StaticPageComponent implements OnInit {
2424
htmlContent: BehaviorSubject<string> = new BehaviorSubject<string>('');
2525
htmlFileName: string;
26+
contentState: 'loading' | 'found' | 'not-found' = 'loading';
2627

2728
constructor(private htmlContentService: HtmlContentService,
2829
private router: Router,
2930
private localeService: LocaleService,
31+
private responseService: ServerResponseService,
32+
private changeDetector: ChangeDetectorRef,
3033
@Inject(APP_CONFIG) protected appConfig?: AppConfig) { }
3134

3235
async ngOnInit(): Promise<void> {
@@ -48,6 +51,8 @@ export class StaticPageComponent implements OnInit {
4851
let potentialContent = await firstValueFrom(this.htmlContentService.fetchHtmlContent(url));
4952
if (isNotEmpty(potentialContent)) {
5053
this.htmlContent.next(potentialContent);
54+
this.contentState = 'found';
55+
this.changeDetector.detectChanges();
5156
return;
5257
}
5358

@@ -56,11 +61,15 @@ export class StaticPageComponent implements OnInit {
5661
potentialContent = await firstValueFrom(this.htmlContentService.fetchHtmlContent(url));
5762
if (isNotEmpty(potentialContent)) {
5863
this.htmlContent.next(potentialContent);
64+
this.contentState = 'found';
65+
this.changeDetector.detectChanges();
5966
return;
6067
}
6168

62-
// Show error page
63-
await this.loadErrorPage();
69+
// Content not found - set 404 status for SSR and show the inline 404 page
70+
this.responseService.setNotFound();
71+
this.contentState = 'not-found';
72+
this.changeDetector.detectChanges();
6473
}
6574

6675
/**
@@ -139,24 +148,10 @@ export class StaticPageComponent implements OnInit {
139148
urlInList = urlInList.filter(n => n);
140149
// if length is 1 - html file name wasn't defined.
141150
if (isEmpty(urlInList) || urlInList.length === 1) {
142-
void this.loadErrorPage();
143151
return null;
144152
}
145153

146154
// If the url is too long take just the first string after `/static` prefix.
147155
return urlInList[1]?.split('#')?.[0];
148156
}
149-
150-
/**
151-
* Load `static-files/error.html`
152-
* @private
153-
*/
154-
private async loadErrorPage() {
155-
let errorPage = await firstValueFrom(this.htmlContentService.fetchHtmlContent(STATIC_FILES_DEFAULT_ERROR_PAGE_PATH));
156-
if (isEmpty(errorPage)) {
157-
console.error('Cannot load error page from the path: ' + STATIC_FILES_DEFAULT_ERROR_PAGE_PATH);
158-
return;
159-
}
160-
this.htmlContent.next(errorPage);
161-
}
162157
}

0 commit comments

Comments
 (0)