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 @@ -13,6 +13,9 @@ import { 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';
import { FeatureFlagService } from '@root/app/feature-flag/services/feature-flag.service';
import { LocationPickerComponent } from '@fileBrowser/components/location-picker/location-picker.component';
import { UncertainLocationPickerComponent } from '@fileBrowser/components/uncertain-location-picker/uncertain-location-picker.component';
import { ProfileEditComponent } from './profile-edit.component';

describe('ProfileEditComponent', () => {
Expand All @@ -24,6 +27,9 @@ describe('ProfileEditComponent', () => {
mockDialogService.open.and.returnValue(mockDialogRef);

const mockCookieService = jasmine.createSpyObj('CookieService', ['check']);
const mockFeatureFlagService = jasmine.createSpyObj('FeatureFlagService', [
'isEnabled',
]);
mockCookieService.check.and.returnValue(false);

const mockProfileService = jasmine.createSpyObj('ProfileService', [
Expand Down Expand Up @@ -55,6 +61,9 @@ describe('ProfileEditComponent', () => {
};

beforeEach(async () => {
mockDialogService.open.calls.reset();
mockFeatureFlagService.isEnabled.and.returnValue(false);

TestBed.configureTestingModule({
declarations: [ProfileEditComponent, GetThumbnailPipe],
providers: [
Expand All @@ -74,6 +83,7 @@ describe('ProfileEditComponent', () => {
{ provide: CookieService, useValue: mockCookieService },
{ provide: DIALOG_DATA, useValue: {} },
{ provide: DialogRef, useValue: mockDialogRef },
{ provide: FeatureFlagService, useValue: mockFeatureFlagService },
],
}).compileComponents();

Expand All @@ -100,21 +110,45 @@ describe('ProfileEditComponent', () => {

component.showFirstTimeDialog();

expect(mockDialogService.open).toHaveBeenCalledWith(jasmine.any(Function), {
width: '760px',
height: 'auto',
});
expect(mockDialogService.open).toHaveBeenCalledOnceWith(
jasmine.any(Function),
{
width: '760px',
height: 'auto',
},
);
});

it('should open LocationPickerComponent when chooseLocationForItem is called', async () => {
const item = {} as any;
await component.chooseLocationForItem(item);

expect(mockDialogService.open).toHaveBeenCalledWith(jasmine.any(Function), {
data: { profileItem: item },
height: 'auto',
width: '600px',
});
expect(mockDialogService.open).toHaveBeenCalledOnceWith(
LocationPickerComponent,
{
data: { profileItem: item },
height: 'auto',
width: '600px',
},
);
});

it('should open UncertainLocationPickerComponent when the uncertain-locations flag is enabled', async () => {

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.

toHaveBeenCalledWith satisfies any spy that has been called at some point, with the correct dialog, but this this needs to be made specific for every test.

We should add mockDialogService.open.calls.reset() to the beforeEach in the profile-edit spec, and switch all four to toHaveBeenCalledOnceWith(...). This way, we make sure every call to the mock dialog service is done specifically for every test.

const item = {} as any;
mockFeatureFlagService.isEnabled.and.callFake(
(flag: string) => flag === 'uncertain-locations',
);

await component.chooseLocationForItem(item);

expect(mockDialogService.open).toHaveBeenCalledOnceWith(
UncertainLocationPickerComponent,
{
data: { profileItem: item },
height: 'auto',
width: '600px',
},
);
});

it('should update banner picture when chooseBannerPicture succeeds', async () => {
Expand Down
11 changes: 10 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 @@ -33,6 +33,9 @@ import { CookieService } from 'ngx-cookie-service';
import { copyFromInputElement } from '@shared/utilities/forms';
import { EventService } from '@shared/services/event/event.service';
import { LocationPickerComponent } from '@fileBrowser/components/location-picker/location-picker.component';
import { UncertainLocationPickerComponent } from '@fileBrowser/components/uncertain-location-picker/uncertain-location-picker.component';
import { FeatureFlagService } from '@root/app/feature-flag/services/feature-flag.service';
import { ComponentType } from '@angular/cdk/portal';
import { DIALOG_DATA, DialogRef } from '@angular/cdk/dialog';
import { ArchiveSettingsDialogComponent } from '@core/components/archive-settings-dialog/archive-settings-dialog.component';
import {
Expand Down Expand Up @@ -91,6 +94,7 @@ export class ProfileEditComponent implements OnInit, AfterViewInit {
private message: MessageService,
private cookies: CookieService,
private event: EventService,
private feature: FeatureFlagService,
) {}

async ngOnInit(): Promise<void> {
Expand Down Expand Up @@ -251,7 +255,12 @@ export class ProfileEditComponent implements OnInit, AfterViewInit {

async chooseLocationForItem(item: ProfileItemVOData) {
try {
this.dialog.open(LocationPickerComponent, {
const picker: ComponentType<
LocationPickerComponent | UncertainLocationPickerComponent
> = this.feature.isEnabled('uncertain-locations')
? UncertainLocationPickerComponent
: LocationPickerComponent;
this.dialog.open(picker, {
data: { profileItem: item },
height: 'auto',
width: '600px',
Expand Down
51 changes: 51 additions & 0 deletions src/app/core/services/edit/edit.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ import { DeviceService } from '@shared/services/device/device.service';
import { DialogCdkService } from '@root/app/dialog-cdk/dialog-cdk.service';
import { SharingComponent } from '@fileBrowser/components/sharing/sharing.component';
import { SharingDialogComponent } from '@fileBrowser/components/sharing-dialog/sharing-dialog.component';
import { LocationPickerComponent } from '@fileBrowser/components/location-picker/location-picker.component';
import { UncertainLocationPickerComponent } from '@fileBrowser/components/uncertain-location-picker/uncertain-location-picker.component';
import { FeatureFlagService } from '@root/app/feature-flag/services/feature-flag.service';
import { FolderPickerService } from '../folder-picker/folder-picker.service';

const mockDataService = {
Expand All @@ -32,6 +35,7 @@ describe('EditService', () => {
let shareLinksApiService: jasmine.SpyObj<ShareLinksApiService>;
let deviceService: jasmine.SpyObj<DeviceService>;
let dialogService: jasmine.SpyObj<DialogCdkService>;
let featureFlagService: jasmine.SpyObj<FeatureFlagService>;

beforeEach(() => {
apiService = jasmine.createSpyObj('ApiService', [
Expand Down Expand Up @@ -69,6 +73,10 @@ describe('EditService', () => {

deviceService = jasmine.createSpyObj('DeviceService', ['isMobile']);
dialogService = jasmine.createSpyObj('DialogCdkService', ['open']);
featureFlagService = jasmine.createSpyObj('FeatureFlagService', [
'isEnabled',
]);
featureFlagService.isEnabled.and.returnValue(false);

const config = cloneDeep(Testing.BASE_TEST_CONFIG);
config.imports.push(NgbTooltipModule);
Expand All @@ -83,6 +91,7 @@ describe('EditService', () => {
{ provide: ShareLinksApiService, useValue: shareLinksApiService },
{ provide: DeviceService, useValue: deviceService },
{ provide: DialogCdkService, useValue: dialogService },
{ provide: FeatureFlagService, useValue: featureFlagService },
],
});

Expand Down Expand Up @@ -527,6 +536,48 @@ describe('EditService', () => {
});
});

describe('openLocationDialog', () => {
it('should open LocationPickerComponent when the flag is disabled', async () => {
const record = new RecordVO({ recordId: 123 });
featureFlagService.isEnabled.and.returnValue(false);

await service.openLocationDialog(record);

expect(featureFlagService.isEnabled).toHaveBeenCalledWith(
'uncertain-locations',
);

expect(dialogService.open).toHaveBeenCalledOnceWith(
LocationPickerComponent,
{
data: { item: record },
panelClass: 'dialog',
height: 'auto',
width: '600px',
},
);
});

it('should open UncertainLocationPickerComponent when the flag is enabled', async () => {
const record = new RecordVO({ recordId: 123 });
featureFlagService.isEnabled.and.callFake(
(flag: string) => flag === 'uncertain-locations',
);

await service.openLocationDialog(record);

expect(dialogService.open).toHaveBeenCalledOnceWith(
UncertainLocationPickerComponent,
{
data: { item: record },
panelClass: 'dialog',
height: 'auto',
width: '600px',
},
);
});
});

describe('openShareDialog', () => {
const mockShareLink: ShareLink = {
id: 'link1',
Expand Down
11 changes: 10 additions & 1 deletion src/app/core/services/edit/edit.service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Injectable } from '@angular/core';
import { ComponentType } from '@angular/cdk/portal';
import { partition } from 'lodash';
import { Subject } from 'rxjs';
import debug from 'debug';
Expand Down Expand Up @@ -40,6 +41,8 @@ import { SharingComponent } from '@fileBrowser/components/sharing/sharing.compon
import { PublishComponent } from '@fileBrowser/components/publish/publish.component';
import { EditTagsComponent } from '@fileBrowser/components/edit-tags/edit-tags.component';
import { LocationPickerComponent } from '@fileBrowser/components/location-picker/location-picker.component';
import { UncertainLocationPickerComponent } from '@fileBrowser/components/uncertain-location-picker/uncertain-location-picker.component';
import { FeatureFlagService } from '@root/app/feature-flag/services/feature-flag.service';
import { SharingDialogComponent } from '@fileBrowser/components/sharing-dialog/sharing-dialog.component';
import { FolderPickerService } from '../folder-picker/folder-picker.service';

Expand Down Expand Up @@ -137,6 +140,7 @@ export class EditService {
private device: DeviceService,
private secrets: SecretsService,
private event: EventService,
private feature: FeatureFlagService,
) {
this.loadGoogleMapsApi();
}
Expand Down Expand Up @@ -627,7 +631,12 @@ export class EditService {
}

async openLocationDialog(item: ItemVO) {
this.dialog.open(LocationPickerComponent, {
const picker: ComponentType<
LocationPickerComponent | UncertainLocationPickerComponent
> = this.feature.isEnabled('uncertain-locations')
? UncertainLocationPickerComponent
: LocationPickerComponent;
this.dialog.open(picker, {
data: { item },
panelClass: 'dialog',
height: 'auto',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<div class="dialog-content">
<div class="header">
<span>Edit location</span>
<button class="btn" (click)="cancel()" aria-label="Close">
<i class="material-icons">close</i>
</button>
</div>
<div class="dialog-body">
<p>
We're building a new way to record locations, including places that are
approximate or uncertain.
</p>
<p>Location editing is not available here yet.</p>
</div>
<div class="dialog-footer">
<button class="btn btn-primary" (click)="cancel()">Close</button>
</div>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
@import 'variables';

:host {
display: block;
flex: 1;
max-width: 100%;
background-color: $PR-brand-white;
}

// The global `pr-dialog .dialog-content` rules do not apply to CDK-opened
// dialogs, so the flex structure is declared here alongside the background.
.dialog-content {
display: flex;
flex-direction: column;
height: 100%;
background-color: $PR-brand-white;
}

.header {
@include tabbedDialogHeader(none, $body-color);
font-weight: $font-weight-bold;
}

.dialog-body {
flex: 1 1 auto;
overflow-y: auto;
padding: 0 $grid-unit;

p:last-child {
margin-bottom: 0;
}
}

.dialog-footer {
flex: 0 0 auto;
display: flex;
justify-content: flex-end;
padding: $grid-unit;

// `button.btn` is globally `display: block; margin: 10px auto; width: 100%`,
// and those auto margins would win over `justify-content` above.
button {
margin: 0;
width: auto;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core';
import { TestBed, ComponentFixture } from '@angular/core/testing';
import { DIALOG_DATA, DialogRef } from '@angular/cdk/dialog';
import { RecordVO } from '@models';
import { UncertainLocationPickerComponent } from './uncertain-location-picker.component';

describe('UncertainLocationPickerComponent', () => {
let fixture: ComponentFixture<UncertainLocationPickerComponent>;
let component: UncertainLocationPickerComponent;
let dialogRef: jasmine.SpyObj<DialogRef>;
const item = new RecordVO({ recordId: 123 });

beforeEach(async () => {
dialogRef = jasmine.createSpyObj('DialogRef', ['close']);

await TestBed.configureTestingModule({
declarations: [UncertainLocationPickerComponent],
providers: [
{ provide: DIALOG_DATA, useValue: { item } },
{ provide: DialogRef, useValue: dialogRef },
],
schemas: [CUSTOM_ELEMENTS_SCHEMA],
}).compileComponents();

fixture = TestBed.createComponent(UncertainLocationPickerComponent);
component = fixture.componentInstance;
fixture.detectChanges();
});

it('should read the item off the dialog data', () => {
expect(component.item).toBe(item);
});

it('should render a titled dialog', () => {
const header = fixture.nativeElement.querySelector('.header span');

expect(header.textContent.trim()).toBe('Edit location');
});

it('should close the dialog from the header close button', () => {
const close = fixture.nativeElement.querySelector('.header button');
close.click();

expect(dialogRef.close).toHaveBeenCalled();
});

it('should close the dialog on cancel', () => {
component.cancel();

expect(dialogRef.close).toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { Component, Inject, Optional } from '@angular/core';
import { DIALOG_DATA, DialogRef } from '@angular/cdk/dialog';
import { ItemVO } from '@models';
import { ProfileItemVOData } from '@models/profile-item-vo';

@Component({
selector: 'pr-uncertain-location-picker',
templateUrl: './uncertain-location-picker.component.html',
styleUrls: ['./uncertain-location-picker.component.scss'],
standalone: false,
})
export class UncertainLocationPickerComponent {
public item: ItemVO;
public profileItem: ProfileItemVOData;

constructor(
@Optional() @Inject(DIALOG_DATA) public dialogData: any,
@Optional() private dialogRef: DialogRef,
) {
if (this.dialogData) {
this.item = this.dialogData.item;
this.profileItem = this.dialogData.profileItem;
}
}

public cancel(): void {
this.dialogRef?.close();
}
}
Loading