-
Notifications
You must be signed in to change notification settings - Fork 1
Use new location metadata fields #1002
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,25 +1,162 @@ | ||
| // import { async, ComponentFixture, TestBed } from '@angular/core/testing'; | ||
|
|
||
| // import { LocationPickerComponent } from './location-picker.component'; | ||
|
|
||
| // describe('LocationPickerComponent', () => { | ||
| // let component: LocationPickerComponent; | ||
| // let fixture: ComponentFixture<LocationPickerComponent>; | ||
|
|
||
| // beforeEach(async(() => { | ||
| // TestBed.configureTestingModule({ | ||
| // declarations: [ LocationPickerComponent ] | ||
| // }) | ||
| // .compileComponents(); | ||
| // })); | ||
|
|
||
| // beforeEach(() => { | ||
| // fixture = TestBed.createComponent(LocationPickerComponent); | ||
| // component = fixture.componentInstance; | ||
| // fixture.detectChanges(); | ||
| // }); | ||
|
|
||
| // it('should create', () => { | ||
| // expect(component).toBeTruthy(); | ||
| // }); | ||
| // }); | ||
| import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core'; | ||
| import { TestBed, ComponentFixture } from '@angular/core/testing'; | ||
| import { ApiService } from '@shared/services/api/api.service'; | ||
| import { MessageService } from '@shared/services/message/message.service'; | ||
| import { EditService } from '@core/services/edit/edit.service'; | ||
| import { ProfileService } from '@shared/services/profile/profile.service'; | ||
| import { PrLocationPipe } from '@shared/pipes/pr-location.pipe'; | ||
| import { LocationPickerComponent } from './location-picker.component'; | ||
|
|
||
| const fakeGoogleMaps = { | ||
| LatLng: class { | ||
| constructor(public input: unknown) {} | ||
| }, | ||
| places: { | ||
| Autocomplete: class { | ||
| setFields(): void {} | ||
| addListener(): void {} | ||
| getPlace(): unknown { | ||
| return null; | ||
| } | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| const buildAddressComponents = ( | ||
| overrides: Partial< | ||
| Record<string, { long_name: string; short_name: string }> | ||
| > = {}, | ||
| ): google.maps.GeocoderAddressComponent[] => { | ||
| const defaults: Record<string, { long_name: string; short_name: string }> = { | ||
| street_number: { long_name: '55', short_name: '55' }, | ||
| route: { long_name: 'Rue Plumet', short_name: 'Rue Plumet' }, | ||
| locality: { long_name: 'Paris', short_name: 'Paris' }, | ||
| postal_code: { long_name: '75007', short_name: '75007' }, | ||
| administrative_area_level_1: { | ||
| long_name: 'Ile-de-France', | ||
| short_name: 'IDF', | ||
| }, | ||
| country: { long_name: 'France', short_name: 'FR' }, | ||
| }; | ||
| const merged = { ...defaults, ...overrides }; | ||
| return Object.entries(merged) | ||
| .filter(([, value]) => value !== undefined) | ||
| .map(([type, value]) => ({ | ||
| long_name: value.long_name, | ||
| short_name: value.short_name, | ||
| types: [type], | ||
| })) as google.maps.GeocoderAddressComponent[]; | ||
| }; | ||
|
|
||
| const buildPlace = ( | ||
| overrides: Partial<google.maps.places.PlaceResult> = {}, | ||
| addressOverrides: | ||
| | Parameters<typeof buildAddressComponents>[0] | ||
| | undefined = undefined, | ||
| ): google.maps.places.PlaceResult => | ||
| ({ | ||
| name: "Jean Valjean's House", | ||
| address_components: buildAddressComponents(addressOverrides), | ||
| geometry: { | ||
| location: { | ||
| lat: () => 48.83, | ||
| lng: () => 2.3, | ||
| }, | ||
| }, | ||
| ...overrides, | ||
| }) as unknown as google.maps.places.PlaceResult; | ||
|
cecilia-donnelly marked this conversation as resolved.
|
||
|
|
||
| describe('LocationPickerComponent', () => { | ||
| let fixture: ComponentFixture<LocationPickerComponent>; | ||
| let component: LocationPickerComponent; | ||
| const testWindow = window as unknown as { google?: unknown }; | ||
| let previousGoogle: unknown; | ||
| let hadGoogle = false; | ||
|
|
||
| beforeAll(() => { | ||
| hadGoogle = 'google' in testWindow; | ||
| previousGoogle = testWindow.google; | ||
| testWindow.google = { maps: fakeGoogleMaps }; | ||
| }); | ||
|
|
||
| afterAll(() => { | ||
| if (hadGoogle) { | ||
| testWindow.google = previousGoogle; | ||
| } else { | ||
| delete testWindow.google; | ||
| } | ||
| }); | ||
|
|
||
| beforeEach(async () => { | ||
| await TestBed.configureTestingModule({ | ||
| declarations: [LocationPickerComponent], | ||
| providers: [ | ||
| { provide: ApiService, useValue: {} }, | ||
| { provide: MessageService, useValue: {} }, | ||
| { provide: EditService, useValue: {} }, | ||
| { provide: ProfileService, useValue: {} }, | ||
| { provide: PrLocationPipe, useValue: { transform: () => ({}) } }, | ||
| ], | ||
| schemas: [CUSTOM_ELEMENTS_SCHEMA], | ||
| }).compileComponents(); | ||
|
|
||
| fixture = TestBed.createComponent(LocationPickerComponent); | ||
| component = fixture.componentInstance; | ||
| }); | ||
|
|
||
| describe('createLocnFromPlace', () => { | ||
| it('populates the spec-aligned fields', () => { | ||
| const locn = component.createLocnFromPlace(buildPlace()); | ||
|
|
||
| expect(locn.sublocation).toBe('55 Rue Plumet'); | ||
| expect(locn.city).toBe('Paris'); | ||
| expect(locn.adminOneName).toBe('Ile-de-France'); | ||
| expect(locn.country).toBe('France'); | ||
| expect(locn.postalCode).toBe('75007'); | ||
| expect(locn.latitude).toBe(48.83); | ||
| expect(locn.longitude).toBe(2.3); | ||
| }); | ||
|
|
||
| it('does not write deprecated legacy fields', () => { | ||
| const locn = component.createLocnFromPlace(buildPlace()); | ||
|
|
||
| expect(locn.streetNumber).toBeUndefined(); | ||
| expect(locn.streetName).toBeUndefined(); | ||
|
cecilia-donnelly marked this conversation as resolved.
|
||
| expect(locn.locality).toBeUndefined(); | ||
| expect(locn.countryCode).toBeUndefined(); | ||
| expect(locn.adminOneCode).toBeUndefined(); | ||
| expect(locn.adminTwoName).toBeUndefined(); | ||
| expect(locn.adminTwoCode).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('sets sublocation to null when no street number or street name is available', () => { | ||
| const locn = component.createLocnFromPlace( | ||
| buildPlace({}, { street_number: undefined, route: undefined }), | ||
| ); | ||
|
|
||
| expect(locn.sublocation).toBeNull(); | ||
| }); | ||
|
|
||
| it('falls back to streetName alone when streetNumber is absent', () => { | ||
| const locn = component.createLocnFromPlace( | ||
| buildPlace({}, { street_number: undefined }), | ||
| ); | ||
|
|
||
| expect(locn.sublocation).toBe('Rue Plumet'); | ||
| }); | ||
|
|
||
| it('writes name when the place name does not include the sublocation', () => { | ||
| const locn = component.createLocnFromPlace(buildPlace()); | ||
|
|
||
| expect(locn.name).toBe("Jean Valjean's House"); | ||
| }); | ||
|
|
||
| it('omits name when the place name already matches the sublocation', () => { | ||
| const locn = component.createLocnFromPlace( | ||
| buildPlace({ name: '55 Rue Plumet' }), | ||
| ); | ||
|
|
||
| expect(locn.name).toBeUndefined(); | ||
| }); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -229,32 +229,32 @@ export class LocationPickerComponent implements OnInit, AfterViewInit { | |
|
|
||
| createLocnFromPlace(place: google.maps.places.PlaceResult) { | ||
| const addr = place.address_components; | ||
| const streetNumber = getComponentName(addr, 'street_number'); | ||
| const streetName = getComponentName(addr, 'route'); | ||
| const sublocation = | ||
| [streetNumber, streetName].filter(Boolean).join(' ') || null; | ||
|
cecilia-donnelly marked this conversation as resolved.
|
||
| const locn: LocnVOData = { | ||
| latitude: place.geometry.location.lat(), | ||
| longitude: place.geometry.location.lng(), | ||
| streetNumber: getComponentName(addr, 'street_number'), | ||
| streetName: getComponentName(addr, 'route'), | ||
| postalCode: getComponentName(addr, 'postal_code'), | ||
| locality: getComponentName(addr, 'locality'), | ||
| adminOneName: getComponentName(addr, 'administrative_area_level_1'), | ||
| adminOneCode: getComponentName(addr, 'administrative_area_level_1', true), | ||
| adminTwoName: getComponentName(addr, 'administrative_area_level_2'), | ||
| adminTwoCode: getComponentName(addr, 'administrative_area_level_2', true), | ||
| country: getComponentName(addr, 'country'), | ||
| countryCode: getComponentName(addr, 'country', true), | ||
| sublocation, | ||
| city: getComponentName(addr, 'locality'), | ||
| }; | ||
|
|
||
| if (place.name && (!sublocation || !place.name.includes(sublocation))) { | ||
| locn.name = place.name; | ||
| } | ||
|
|
||
| return locn; | ||
|
|
||
| function getComponentName( | ||
| addressComponents: google.maps.GeocoderAddressComponent[], | ||
| type, | ||
| getShortName = true, | ||
| type: string, | ||
| ) { | ||
| const component = find(addressComponents, (c) => c.types.includes(type)); | ||
| return component | ||
| ? component[getShortName ? 'short_name' : 'long_name'] | ||
| : null; | ||
| return component ? component.long_name : null; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What was the short name?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @slifty is going to figure out if we want to keep that after all.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The idea is that short vs long name isn't part of the new location specification, and so we just use the long name now because it's the only thing that will exist in the future. Short name might have been "CA" when long name was "California" |
||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.