Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,19 @@ const parent = Object.assign(new Collection(), {
metadata: {
'dc.title': [
{
value: 'parent title',
value: 'community title > collection title',
},
],
},
});

describe('JournalIssueSidebarSearchListElementComponent',
createSidebarSearchListElementTests(JournalIssueSidebarSearchListElementComponent, object, parent, 'parent title', 'title', '5 - 7'),
createSidebarSearchListElementTests(
JournalIssueSidebarSearchListElementComponent,
object,
parent,
'community title > collection title',
'title',
'5 - 7',
),
);
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,18 @@ const parent = Object.assign(new Collection(), {
metadata: {
'dc.title': [
{
value: 'parent title',
value: 'community title > collection title',
},
],
},
});

describe('JournalVolumeSidebarSearchListElementComponent',
createSidebarSearchListElementTests(JournalVolumeSidebarSearchListElementComponent, object, parent, 'parent title', 'title', 'journal title (1) (2)'),
createSidebarSearchListElementTests(
JournalVolumeSidebarSearchListElementComponent,
object, parent,
'community title > collection title',
'title',
'journal title (1) (2)'),
);

Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,19 @@ const parent = Object.assign(new Collection(), {
metadata: {
'dc.title': [
{
value: 'parent title',
value: 'parent community-title > parent collection-title',
},
],
},
});

describe('JournalSidebarSearchListElementComponent',
createSidebarSearchListElementTests(JournalSidebarSearchListElementComponent, object, parent, 'parent title', 'title', '1234, 5678'),
createSidebarSearchListElementTests(
JournalSidebarSearchListElementComponent,
object,
parent,
'parent community-title > parent collection-title',
'title',
'1234, 5678',
),
);
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ const parent = Object.assign(new Collection(), {
metadata: {
'dc.title': [
{
value: 'parent title',
value: 'community title > collection title',
},
],
},
});

describe('OrgUnitSidebarSearchListElementComponent',
createSidebarSearchListElementTests(OrgUnitSidebarSearchListElementComponent, object, parent, 'parent title', 'title', 'description'),
createSidebarSearchListElementTests(OrgUnitSidebarSearchListElementComponent, object, parent, 'community title > collection title', 'title', 'description'),
);
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ const parent = Object.assign(new Collection(), {
metadata: {
'dc.title': [
{
value: 'parent title',
value: 'community title > collection title',
},
],
},
});

describe('PersonSidebarSearchListElementComponent',
createSidebarSearchListElementTests(PersonSidebarSearchListElementComponent, object, parent, 'parent title', 'family name, given name', 'job title', [
]),
createSidebarSearchListElementTests(PersonSidebarSearchListElementComponent, object, parent, 'community title > collection title', 'family name, given name', 'job title', []),
);

Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,13 @@ const parent = Object.assign(new Collection(), {
metadata: {
'dc.title': [
{
value: 'parent title',
value: 'community title > collection title',
},
],
},
});

describe('ProjectSidebarSearchListElementComponent',
createSidebarSearchListElementTests(ProjectSidebarSearchListElementComponent, object, parent, 'parent title', 'title', undefined),
createSidebarSearchListElementTests(ProjectSidebarSearchListElementComponent, object, parent, 'community title > collection title', 'title', undefined),
);

Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ const object = Object.assign(new CollectionSearchResult(), {
},
}),
});
object.indexableObject.getParentLinkKey = () => 'parentCommunity';

const parent = Object.assign(new Community(), {
id: 'test-community',
metadata: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ const object = Object.assign(new CommunitySearchResult(), {
},
}),
});
object.indexableObject.getParentLinkKey = () => 'parentCommunity';

const parent = Object.assign(new Community(), {
id: 'test-parent-community',
metadata: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ const parent = Object.assign(new Collection(), {
metadata: {
'dc.title': [
{
value: 'parent title',
value: 'communtity title > collection title',
},
],
},
});

describe('PublicationSidebarSearchListElementComponent',
createSidebarSearchListElementTests(PublicationSidebarSearchListElementComponent, object, parent, 'parent title', 'title', '(publisher, date) author'),
createSidebarSearchListElementTests(PublicationSidebarSearchListElementComponent, object, parent, 'communtity title > collection title', 'title', '(publisher, date) author'),
);
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<ds-truncatable-part [maxLines]="1" [background]="isCurrent() ? 'primary' : 'default'" [showToggle]="false">
<ds-truncatable-part [background]="isCurrent() ? 'primary' : 'default'" [showToggle]="false">
<div [ngClass]="isCurrent() ? 'text-light' : 'text-body'"
[innerHTML]="(parentTitle$ && parentTitle$ | async) ? (parentTitle$ | async) : ('home.breadcrumbs' | translate)"></div>
[innerHTML]="(hierarchicalTitle$ && hierarchicalTitle$ | async) ? (hierarchicalTitle$ | async) : ('home.breadcrumbs' | translate)"></div>
</ds-truncatable-part>
<ds-truncatable-part [maxLines]="1" [background]="isCurrent() ? 'primary' : 'default'" [showToggle]="false">
<div class="fw-bold"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export function createSidebarSearchListElementTests(
componentClass: any,
object: SearchResult<DSpaceObject & ChildHALResource>,
parent: DSpaceObject,
expectedParentTitle: string,
expectedHierarchicalTitle: string,
expectedTitle: string,
expectedDescription: string,
extraProviders: any[] = [],
Expand All @@ -39,6 +39,7 @@ export function createSidebarSearchListElementTests(
[object.indexableObject.getParentLinkKey()]: createSuccessfulRemoteDataObject$(parent),
}),
});

TestBed.configureTestingModule({
imports: [TranslateModule.forRoot(), RouterTestingModule.withRoutes([]), VarDirective],
providers: [
Expand All @@ -59,10 +60,16 @@ export function createSidebarSearchListElementTests(
fixture.detectChanges();
});

it('should contain the correct parent title', (done) => {
component.parentTitle$.subscribe((title) => {
expect(title).toEqual(expectedParentTitle);
done();
it('should contain the correct hierarchical title', (done) => {
component.hierarchicalTitle$.subscribe({
next: (title) => {
expect(title).toEqual(expectedHierarchicalTitle);
done();
},
error: (err) => {
fail('hierarchicalTitle$ threw an error: ' + err);
done();
},
});
});

Expand All @@ -75,3 +82,4 @@ export function createSidebarSearchListElementTests(
});
};
}

Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@ import {
} from '@dspace/shared/utils/empty.util';
import { TranslateModule } from '@ngx-translate/core';
import {
from,
Observable,
of,
} from 'rxjs';
import {
catchError,
find,
map,
switchMap,
} from 'rxjs/operators';

import { TruncatableService } from '../../truncatable/truncatable.service';
Expand All @@ -49,9 +51,9 @@ import { SearchResultListElementComponent } from '../search-result-list-element/
*/
export class SidebarSearchListElementComponent<T extends SearchResult<K>, K extends DSpaceObject> extends SearchResultListElementComponent<T, K> implements OnInit {
/**
* Observable for the title of the parent object (displayed above the object's title)
* Observable for the hierarchical title i.e community > subcommunity > collection
*/
parentTitle$: Observable<string>;
hierarchicalTitle$: Observable<string>;

/**
* A description to display below the title
Expand All @@ -71,7 +73,7 @@ export class SidebarSearchListElementComponent<T extends SearchResult<K>, K exte
ngOnInit(): void {
super.ngOnInit();
if (hasValue(this.dso)) {
this.parentTitle$ = this.getParentTitle();
this.hierarchicalTitle$ = this.getHierarchicalName();
this.description = this.getDescription();
}
}
Expand All @@ -85,25 +87,66 @@ export class SidebarSearchListElementComponent<T extends SearchResult<K>, K exte

/**
* Get the title of the object's parent
* Retrieve the parent by using the object's parent link and retrieving its 'dc.title' metadata
* keep on Retrieving recursively the parent by using the object's parent link and retrieving its 'dc.title' metadata
* and build a heirarchical name by concating the parent's names
*/
getParentTitle(): Observable<string> {
getHierarchicalName(): Observable<string> {
Copy link
Member

Choose a reason for hiding this comment

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

As noted by CodeCov validation below (see the "files changed" tab in GitHub), you are missing specs for the changes to this sidebar-search-list-element.component.ts. Ideally, we should have tests in sidebar-search-list-element.component.spec.ts which verify the behavior of this new getHierarchicalName() method.

Copy link
Member

@tdonohue tdonohue Apr 24, 2025

Choose a reason for hiding this comment

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

My prior feedback has NOT been addressed on this getHierarchicalName() method. We need to add new tests to sidebar-search-list-element.component.spec.ts which prove that this method is properly building the hierarchical name at all times. Otherwise, there is a risk that it will accidentally break in the future & we won't realize it.

return this.getParent().pipe(
map((parentRD: RemoteData<DSpaceObject>) => {
return hasValue(parentRD) && hasValue(parentRD.payload) ? this.dsoNameService.getName(parentRD.payload, true) : undefined;
switchMap((initialRD: RemoteData<DSpaceObject>) => {
if (!hasValue(initialRD) || !initialRD.hasSucceeded || !hasValue(initialRD.payload)) {
return of('');
}

return from((async () => {
const names: string[] = [];
let current: DSpaceObject | null = initialRD.payload;

const visited = new Set<string>(); // prevent cycles

while (current && !visited.has(current.id)) {
visited.add(current.id);
const name = this.dsoNameService.getName(current);
if (name) {
names.unshift(name);
}

const instance = this.createInstanceFromDSpaceObject(current);
const parentRD: RemoteData<DSpaceObject> = await instance.getParent().toPromise().catch(() => null);

if (hasValue(parentRD) && parentRD.hasSucceeded && hasValue(parentRD.payload)) {
current = parentRD.payload;
} else {
current = null;
}
}

return names.join(' > ');
})());
}),
catchError(() => of('')),
);
}

/**
* Utility method to create an instance of the current class from a DSpaceObject
*/
private createInstanceFromDSpaceObject(dso: DSpaceObject): this {
const instance = Object.create(this);
instance.dso = dso;
return instance;
}

/**
* Get the parent of the object
*/
getParent(): Observable<RemoteData<DSpaceObject>> {
if (typeof (this.dso as any).getParentLinkKey === 'function') {
const propertyName = (this.dso as any).getParentLinkKey();
return this.linkService.resolveLink(this.dso, followLink(propertyName))[propertyName].pipe(
find((parentRD: RemoteData<ChildHALResource & DSpaceObject>) => parentRD.hasSucceeded || parentRD.statusCode === 204),
);
if (this.linkService.resolveLink(this.dso, followLink(propertyName))[propertyName]) {
return this.linkService.resolveLink(this.dso, followLink(propertyName))[propertyName].pipe(
find((parentRD: RemoteData<ChildHALResource & DSpaceObject>) => parentRD.hasSucceeded || parentRD.statusCode === 204),
);
}
}
return of(undefined);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,8 @@
.removeFaded.content::after {
display: none;
}

.content {
white-space: normal;
overflow: visible;
}
Loading