Skip to content

Commit 8ba7f32

Browse files
Matus Kasakclaude
andcommitted
VSB-TUO/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>
1 parent a686cd0 commit 8ba7f32

3 files changed

Lines changed: 52 additions & 22 deletions

File tree

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: 20 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';
@@ -9,12 +11,14 @@ import { of } from 'rxjs';
911
import { APP_CONFIG } from '../../config/app-config.interface';
1012
import { environment } from '../../environments/environment';
1113
import { ClarinSafeHtmlPipe } from '../shared/utils/clarin-safehtml.pipe';
14+
import { ServerResponseService } from '../core/services/server-response.service';
1215

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

17-
let htmlContentService: HtmlContentService;
20+
let htmlContentService: any;
21+
let responseService: jasmine.SpyObj<ServerResponseService>;
1822
let appConfig: any;
1923

2024
const htmlContent = '<div id="idShouldNotBeRemoved">TEST MESSAGE</div>';
@@ -25,6 +29,8 @@ describe('StaticPageComponent', () => {
2529
getHmtlContentByPathAndLocale: Promise.resolve(htmlContent)
2630
});
2731

32+
responseService = jasmine.createSpyObj('responseService', ['setNotFound']);
33+
2834
appConfig = Object.assign(environment, {
2935
ui: {
3036
namespace: 'testNamespace'
@@ -34,13 +40,16 @@ describe('StaticPageComponent', () => {
3440
TestBed.configureTestingModule({
3541
declarations: [ StaticPageComponent, ClarinSafeHtmlPipe ],
3642
imports: [
43+
CommonModule,
3744
TranslateModule.forRoot()
3845
],
3946
providers: [
4047
{ provide: HtmlContentService, useValue: htmlContentService },
4148
{ provide: Router, useValue: new RouterMock() },
49+
{ provide: ServerResponseService, useValue: responseService },
4250
{ provide: APP_CONFIG, useValue: appConfig }
43-
]
51+
],
52+
schemas: [NO_ERRORS_SCHEMA]
4453
});
4554

4655
});
@@ -58,5 +67,14 @@ describe('StaticPageComponent', () => {
5867
it('should load html file content', async () => {
5968
await component.ngOnInit();
6069
expect(component.htmlContent.value).toBe('<div id="idShouldNotBeRemoved">TEST MESSAGE</div>');
70+
expect(component.contentState).toBe('found');
71+
});
72+
73+
// When the file is missing, set a 404 status for SSR and switch to the not-found state
74+
it('should set 404 status when content is not found', async () => {
75+
htmlContentService.getHmtlContentByPathAndLocale.and.returnValue(Promise.resolve(undefined));
76+
await component.ngOnInit();
77+
expect(responseService.setNotFound).toHaveBeenCalled();
78+
expect(component.contentState).toBe('not-found');
6179
});
6280
});

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

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
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';
3-
import { BehaviorSubject, firstValueFrom } from 'rxjs';
3+
import { BehaviorSubject } from 'rxjs';
44
import { Router } from '@angular/router';
55
import { isEmpty, isNotEmpty } from '../shared/empty.util';
6-
import { STATIC_FILES_DEFAULT_ERROR_PAGE_PATH, STATIC_PAGE_PATH } from './static-page-routing-paths';
6+
import { STATIC_PAGE_PATH } from './static-page-routing-paths';
77
import { APP_CONFIG, AppConfig } from '../../config/app-config.interface';
8+
import { ServerResponseService } from '../core/services/server-response.service';
89

910
/**
1011
* Component which load and show static files from the `static-files` folder.
@@ -19,9 +20,12 @@ export class StaticPageComponent implements OnInit {
1920
static readonly no_static: string = 'no_static_';
2021
htmlContent: BehaviorSubject<string> = new BehaviorSubject<string>('');
2122
htmlFileName: string;
23+
contentState: 'loading' | 'found' | 'not-found' = 'loading';
2224

2325
constructor(private htmlContentService: HtmlContentService,
2426
private router: Router,
27+
private responseService: ServerResponseService,
28+
private changeDetector: ChangeDetectorRef,
2529
@Inject(APP_CONFIG) protected appConfig?: AppConfig) { }
2630

2731
async ngOnInit(): Promise<void> {
@@ -31,11 +35,15 @@ export class StaticPageComponent implements OnInit {
3135
const htmlContent = await this.htmlContentService.getHmtlContentByPathAndLocale(this.htmlFileName);
3236
if (isNotEmpty(htmlContent)) {
3337
this.htmlContent.next(htmlContent);
38+
this.contentState = 'found';
39+
this.changeDetector.detectChanges();
3440
return;
3541
}
3642

37-
// Show error page
38-
await this.loadErrorPage();
43+
// Content not found - set 404 status for SSR and show the inline 404 page
44+
this.responseService.setNotFound();
45+
this.contentState = 'not-found';
46+
this.changeDetector.detectChanges();
3947
}
4048

4149
/**
@@ -119,24 +127,10 @@ export class StaticPageComponent implements OnInit {
119127
urlInList = urlInList.filter(n => n);
120128
// if length is 1 - html file name wasn't defined.
121129
if (isEmpty(urlInList) || urlInList.length === 1) {
122-
void this.loadErrorPage();
123130
return null;
124131
}
125132

126133
// If the url is too long take just the first string after `/static` prefix.
127134
return urlInList[1]?.split('#')?.[0];
128135
}
129-
130-
/**
131-
* Load `static-files/error.html`
132-
* @private
133-
*/
134-
private async loadErrorPage() {
135-
let errorPage = await firstValueFrom(this.htmlContentService.fetchHtmlContent(STATIC_FILES_DEFAULT_ERROR_PAGE_PATH));
136-
if (isEmpty(errorPage)) {
137-
console.error('Cannot load error page from the path: ' + STATIC_FILES_DEFAULT_ERROR_PAGE_PATH);
138-
return;
139-
}
140-
this.htmlContent.next(errorPage);
141-
}
142136
}

0 commit comments

Comments
 (0)