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 @@ -33,7 +33,7 @@
</div>
}
@default {
<div prBgImage [bgSrc]="item | getThumbnail"></div>
<div prBgImage [bgSrc]="getThumbnailUrl(item)"></div>
}
}
<div class="name">{{ item.displayName }}</div>
Expand All @@ -50,7 +50,7 @@
}
@if (selectedRecord) {
<div class="picker-contents picker-selected-record">
<div prBgImage [bgSrc]="selectedRecord | getThumbnail"></div>
<div prBgImage [bgSrc]="getThumbnailUrl(selectedRecord)"></div>
</div>
}
<div class="picker-footer">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import { DataService } from '@shared/services/data/data.service';
import { ApiService } from '@shared/services/api/api.service';
import { FolderResponse } from '@shared/services/api/index.repo';
import { SharedModule } from '@shared/shared.module';
import { FolderVO } from '@root/app/models';
import { By } from '@angular/platform-browser';
import { BgImageSrcDirective } from '@shared/directives/bg-image-src.directive';
import { FolderVO, RecordVO } from '@root/app/models';
import { HttpTestingController } from '@angular/common/http/testing';
import { FolderPickerService } from '@core/services/folder-picker/folder-picker.service';
import { DataStatus } from '@models/data-status.enum';
Expand Down Expand Up @@ -80,4 +82,50 @@ describe('FolderPickerComponent', () => {
),
).toBeFalsy();
});

it('should read the thumbnail currently on the item', () => {
const record = new RecordVO({ folder_linkId: 2, archiveNbr: 'a-2' });

expect(component.getThumbnailUrl(record)).toBeUndefined();

record.thumbnail256 = 'https://example.com/256';

expect(component.getThumbnailUrl(record)).toBe('https://example.com/256');
});

it('should show a thumbnail that arrives after the row is rendered', () => {
const record = new RecordVO({
folder_linkId: 1,
archiveNbr: 'a-1',
displayName: 'photo.jpg',
});
const folder = new FolderVO({
folder_linkId: 9,
folderId: 9,
displayName: 'Photos',
type: 'type.folder.private.folder',
});
folder.ChildItemVOs = [record];

component.allowRecords = true;
component.currentFolder = folder;
fixture.detectChanges();

const backgrounds = fixture.debugElement.queryAll(
By.directive(BgImageSrcDirective),
);

expect(backgrounds.length).toBe(1);

const background = backgrounds[0].injector.get(BgImageSrcDirective);

expect(background.bgSrc).toBeFalsy();

// loadCurrentFolderChildData() writes the URL onto this same instance, so
// the row has to notice a mutation that leaves the reference unchanged.
record.thumbURL200 = 'https://example.com/thumb.jpg';
fixture.detectChanges();

expect(background.bgSrc).toBe('https://example.com/thumb.jpg');
});
});
10 changes: 10 additions & 0 deletions src/app/core/components/folder-picker/folder-picker.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Component, OnDestroy } from '@angular/core';
import { remove } from 'lodash';
import { DataService } from '@shared/services/data/data.service';
import { FolderVO, ItemVO, RecordVO } from '@root/app/models/index';
import { GetThumbnail } from '@models/get-thumbnail';
import { ApiService } from '@shared/services/api/api.service';
import { FolderResponse } from '@shared/services/api/index.repo';
import { FolderPickerService } from '@core/services/folder-picker/folder-picker.service';
Expand Down Expand Up @@ -113,6 +114,15 @@ export class FolderPickerComponent implements OnDestroy {
this.selectedRecord = record;
}

// Read from the item on every check rather than through the getThumbnail pipe.
// The lean fetch that follows setFolder() writes thumbnail URLs onto these same
// item instances, and a pure pipe never re-runs for a mutation that leaves the
// reference unchanged, so the picker showed no thumbnails until its rows were
// rebuilt by navigating away and back.
getThumbnailUrl(item: ItemVO): string | undefined {
return GetThumbnail(item);
}

async setFolder(folder: FolderVO) {
this.waiting = true;
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@
<div
class="profile-photo"
prBgImage
[bgSrc]="this.archive | getThumbnail"
[bgSrc]="profileThumbnail"
(click)="onProfilePictureClick()"
>
<div class="change-thumbnail">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { PromptService } from '@shared/services/prompt/prompt.service';
import { EventService } from '@shared/services/event/event.service';
import { CookieService } from 'ngx-cookie-service';
import { DIALOG_DATA, DialogRef } from '@angular/cdk/dialog';
import { FolderVO } from '@models/index';
import { ArchiveVO, FolderVO } from '@models/index';
import { RecordVO } from '@models/record-vo';
import { FolderResponse } from '@shared/services/api/folder.repo';
import { GetThumbnailPipe } from '@shared/pipes/get-thumbnail.pipe';
Expand Down Expand Up @@ -143,6 +143,18 @@ describe('ProfileEditComponent', () => {
expect(component.publicRoot.thumbURL2000).toBe('new2000');
});

it('should show a profile photo written onto the archive after the picker closes', () => {
component.archive = new ArchiveVO({ archiveNbr: 'a-1' });

expect(component.profileThumbnail).toBeNull();

// promptForProfilePicture() updates this same instance rather than
// replacing it, so the binding has to re-read an unchanged reference.
component.archive.thumbURL200 = 'new200';

expect(component.profileThumbnail).toBe('new200');
});

it('should restore original thumbArchiveNbr when chooseBannerPicture throws FolderResponse', async () => {
const originalValue = component.publicRoot.thumbArchiveNbr;

Expand Down
14 changes: 13 additions & 1 deletion src/app/core/components/profile-edit/profile-edit.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
collapseAnimation,
ngIfScaleAnimationDynamic,
} from '@shared/animations';
import { GetBanner } from '@models/get-thumbnail';
import { GetBanner, GetThumbnail } from '@models/get-thumbnail';
import debug from 'debug';
import {
PromptService,
Expand Down Expand Up @@ -77,6 +77,18 @@ export class ProfileEditComponent implements OnInit, AfterViewInit {
return this.publicRoot?.thumbArchiveNbr ? GetBanner(this.publicRoot) : null;
}

// Read through to the archive rather than piping it, the way bannerThumbnail
// reads through to publicRoot. Choosing a photo writes the new URLs onto this
// same ArchiveVO instance, and a pure pipe never re-runs for a mutation that
// leaves the reference unchanged.
get profileThumbnail(): string | null {
if (!this.archive) {
return null;
}

return GetThumbnail(this.archive) ?? null;
}

private debug = debug('component:profileEdit');

constructor(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { ElementRef, Pipe, PipeTransform } from '@angular/core';
import { of } from 'rxjs';
import { Subject, of } from 'rxjs';
import { ActivatedRoute, Router, provideRouter } from '@angular/router';

import { DataService } from '@shared/services/data/data.service';
Expand Down Expand Up @@ -41,6 +41,7 @@ describe('FileListItemComponent', () => {
let component: FileListItemComponent;
let fixture: ComponentFixture<FileListItemComponent>;
let editService: EditService;
let thumbnailUpdatedSubject: Subject<any>;

const activatedRouteMock = {
snapshot: {
Expand All @@ -67,6 +68,8 @@ describe('FileListItemComponent', () => {
};

beforeEach(async () => {
thumbnailUpdatedSubject = new Subject<any>();

await TestBed.configureTestingModule({
imports: [MockItemTypeIconPipe, MockPrDatePipe, MockPrConstantsPipe],
declarations: [FileListItemComponent, GetThumbnailPipe],
Expand All @@ -84,6 +87,7 @@ describe('FileListItemComponent', () => {
beginPreparingForNavigate: jasmine.createSpy(),
fetchLeanItems: jasmine.createSpy(),
setItemMultiSelectStatus: jasmine.createSpy(),
thumbnailUpdated$: () => thumbnailUpdatedSubject.asObservable(),
currentFolder: { type: '' },
},
},
Expand Down Expand Up @@ -333,6 +337,101 @@ describe('FileListItemComponent', () => {
(router.routerState.snapshot as any).url = '/';
});

it('should not replace the stock preview when a thumbnail arrives later', async () => {
// detectChanges() in beforeEach left an ngOnInit awaiting isUnlistedShare().
// Drain it and tear the component down so this test starts from a single
// subscription, the way a real component instance does.
await new Promise((resolve) => {
setTimeout(resolve);
});
component.ngOnDestroy();

const router = TestBed.inject(Router);
(router.routerState.snapshot as any).url = '/share/test';

const shareLinksService = TestBed.inject(ShareLinksService);
spyOn(shareLinksService, 'isUnlistedShare').and.returnValue(
Promise.resolve(false),
);
component.item.isRecord = true;
component.item.type = 'type.record.image';

await component.ngOnInit();

const stockPreview = component.recordThumbnailUrl;

expect(stockPreview).toMatch(/^assets\/img\/preview\/preview-\d+\.jpg$/);

component.item.thumbURL200 = 'https://example.com/thumb.jpg';
thumbnailUpdatedSubject.next(component.item);

expect(component.recordThumbnailUrl).toBe(stockPreview);

(router.routerState.snapshot as any).url = '/';
});

it('should not expose the real thumbnail until the share type is known', async () => {
// detectChanges() in beforeEach left an ngOnInit awaiting isUnlistedShare().
// Drain it before changing the route, or its tail resumes against the route
// this test sets and overwrites the preview state with its own stale answer.
await new Promise((resolve) => {
setTimeout(resolve);
});

const router = TestBed.inject(Router);
(router.routerState.snapshot as any).url = '/share/test';

const shareLinksService = TestBed.inject(ShareLinksService);
let resolveIsUnlistedShare: (isUnlisted: boolean) => void = () => {};
spyOn(shareLinksService, 'isUnlistedShare').and.returnValue(
new Promise<boolean>((resolve) => {
resolveIsUnlistedShare = resolve;
}),
);
component.item.isRecord = true;
component.item.type = 'type.record.image';
component.item.thumbURL200 = 'https://example.com/thumb.jpg';

// Deliberately not awaited: a listed share must not show the real
// thumbnail in the window before isUnlistedShare() settles.
const init = component.ngOnInit();

expect(component.recordThumbnailUrl).toBeUndefined();

resolveIsUnlistedShare(false);
await init;

expect(component.recordThumbnailUrl).toMatch(
/^assets\/img\/preview\/preview-\d+\.jpg$/,
);

(router.routerState.snapshot as any).url = '/';
});

it('should show the real thumbnail on an unlisted share', async () => {
// See above: let the ngOnInit started by beforeEach settle first.
await new Promise((resolve) => {
setTimeout(resolve);
});

const router = TestBed.inject(Router);
(router.routerState.snapshot as any).url = '/share/test';

const shareLinksService = TestBed.inject(ShareLinksService);
spyOn(shareLinksService, 'isUnlistedShare').and.returnValue(
Promise.resolve(true),
);
component.item.isRecord = true;
component.item.type = 'type.record.image';
component.item.thumbURL200 = 'https://example.com/thumb.jpg';

await component.ngOnInit();

expect(component.recordThumbnailUrl).toBe('https://example.com/thumb.jpg');

(router.routerState.snapshot as any).url = '/';
});

it('should always set real thumbnail URL on init', async () => {
component.item.isRecord = true;
component.item.type = 'type.record.image';
Expand All @@ -343,6 +442,55 @@ describe('FileListItemComponent', () => {
expect(component.recordThumbnailUrl).toBe('https://example.com/thumb.jpg');
});

it('should pick up a thumbnail added to the item after init', async () => {
component.ngOnDestroy();
component.item.isRecord = true;
component.item.type = 'type.record.image';

await component.ngOnInit();

expect(component.recordThumbnailUrl).toBeUndefined();

// The thumbnail refresh poll in DataService mutates the existing item
// rather than replacing it, then announces the item it wrote to.
component.item.thumbnail256 = 'https://example.com/256';
thumbnailUpdatedSubject.next(component.item);

expect(component.recordThumbnailUrl).toBe('https://example.com/256');
});

it('should ignore a thumbnail update for a different item', async () => {
component.ngOnDestroy();
component.item.isRecord = true;
component.item.type = 'type.record.image';

await component.ngOnInit();

component.item.thumbnail256 = 'https://example.com/256';
// Same folder_linkId, different instance: only the item this row renders
// counts, so the update belongs to some other row.
thumbnailUpdatedSubject.next({
folder_linkId: component.item.folder_linkId,
thumbnail256: 'https://example.com/other',
});

expect(component.recordThumbnailUrl).toBeUndefined();
});

it('should stop applying thumbnail updates once destroyed', async () => {
component.ngOnDestroy();
component.item.isRecord = true;
component.item.type = 'type.record.image';

await component.ngOnInit();
component.ngOnDestroy();

component.item.thumbnail256 = 'https://example.com/256';
thumbnailUpdatedSubject.next(component.item);

expect(component.recordThumbnailUrl).toBeUndefined();
});

it('should display displayTime instead of displayDT when displayTime is set', () => {
component.item.displayTime = '2020-06-10';
component.item.displayDT = '2023-01-01T00:00:00.000Z';
Expand Down
Loading
Loading