From 956f519c8e29a5e27af835ae0c56d36098893ba2 Mon Sep 17 00:00:00 2001 From: Alex Swindler Date: Mon, 19 Oct 2026 13:56:12 -0600 Subject: [PATCH] Fix DAC highlighting, add CI tests --- .github/workflows/ci.yml | 3 + src/@seed/services/map/map.service.spec.ts | 67 ++++++ src/@seed/services/map/map.service.ts | 35 ++-- .../core/transloco/transloco-testing.spec.ts | 27 +++ src/app/core/transloco/transloco-testing.ts | 10 + .../inventory-list/map/map.component.html | 1 + .../inventory-list/map/map.component.spec.ts | 191 ++++++++++++++++++ .../inventory-list/map/map.component.ts | 15 +- .../salesforce-login.component.spec.ts | 14 +- 9 files changed, 335 insertions(+), 28 deletions(-) create mode 100644 src/@seed/services/map/map.service.spec.ts create mode 100644 src/app/core/transloco/transloco-testing.spec.ts create mode 100644 src/app/core/transloco/transloco-testing.ts create mode 100644 src/app/modules/inventory-list/map/map.component.spec.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 95b4a4af..1f0be848 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,5 +31,8 @@ jobs: - name: Lint run: pnpm lint + - name: Test + run: pnpm test --watch=false --browsers=ChromeHeadless + - name: Build run: pnpm build diff --git a/src/@seed/services/map/map.service.spec.ts b/src/@seed/services/map/map.service.spec.ts new file mode 100644 index 00000000..e83730c9 --- /dev/null +++ b/src/@seed/services/map/map.service.spec.ts @@ -0,0 +1,67 @@ +import { provideHttpClient } from '@angular/common/http' +import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing' +import { TestBed } from '@angular/core/testing' +import { MapService } from './map.service' + +describe('MapService', () => { + let service: MapService + let http: HttpTestingController + + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [provideHttpClient(), provideHttpClientTesting()], + }) + service = TestBed.inject(MapService) + http = TestBed.inject(HttpTestingController) + }) + + afterEach(() => { + http.verify() + }) + + it('should cache string tract IDs before notifying the caller that status is ready', () => { + const statusReady = jasmine.createSpy('statusReady').and.callFake(() => { + expect(service.isDisadvantaged('01001020100')).toBeTrue() + expect(service.isDisadvantaged('01001020200')).toBeFalse() + }) + service.checkDisadvantagedStatus(['01001020100', '01001020200']).subscribe(statusReady) + + const request = http.expectOne('/api/v3/eeej/filter_disadvantaged_tracts/') + expect(request.request.method).toBe('POST') + expect(request.request.body).toEqual({ tract_ids: ['01001020100', '01001020200'] }) + expect(statusReady).not.toHaveBeenCalled() + + request.flush({ status: 'success', disadvantaged: ['01001020100'] }) + + expect(statusReady).toHaveBeenCalledTimes(1) + }) + + it('should immediately notify the caller when all tract statuses are cached', () => { + service.disadvantaged = { '01001020100': true, '01001020200': false } + const statusReady = jasmine.createSpy('statusReady') + + service.checkDisadvantagedStatus(['01001020100', '01001020200']).subscribe(statusReady) + + expect(statusReady).toHaveBeenCalledTimes(1) + http.expectNone('/api/v3/eeej/filter_disadvantaged_tracts/') + }) + + it('should complete without a request when there are no census tracts', () => { + const complete = jasmine.createSpy('complete') + + service.checkDisadvantagedStatus([]).subscribe({ complete }) + + expect(complete).toHaveBeenCalledTimes(1) + http.expectNone('/api/v3/eeej/filter_disadvantaged_tracts/') + }) + + it('should propagate lookup errors without caching a negative status', () => { + const error = jasmine.createSpy('error') + service.checkDisadvantagedStatus(['01001020100']).subscribe({ error }) + + http.expectOne('/api/v3/eeej/filter_disadvantaged_tracts/').flush('Lookup failed', { status: 500, statusText: 'Internal Server Error' }) + + expect(error).toHaveBeenCalledTimes(1) + expect(service.disadvantaged).toEqual({}) + }) +}) diff --git a/src/@seed/services/map/map.service.ts b/src/@seed/services/map/map.service.ts index 0f8b9932..3a0fd1f8 100644 --- a/src/@seed/services/map/map.service.ts +++ b/src/@seed/services/map/map.service.ts @@ -1,33 +1,32 @@ import { HttpClient } from '@angular/common/http' import { inject, Injectable } from '@angular/core' -import { map, tap } from 'rxjs' +import type { Observable } from 'rxjs' +import { map, of } from 'rxjs' @Injectable({ providedIn: 'root' }) export class MapService { private _httpClient: HttpClient = inject(HttpClient) - disadvantaged = {} + disadvantaged: Record = {} - checkDisadvantagedStatus(tractIds: number[]) { + checkDisadvantagedStatus(tractIds: string[]): Observable { const idsToFetch = tractIds.filter((id) => !(id in this.disadvantaged)) - if (idsToFetch.length) { - const url = '/api/v3/eeej/filter_disadvantaged_tracts/' - this._httpClient - .post<{ status: string; disadvantaged: number[] }>(url, { tract_ids: tractIds }) - .pipe( - map(({ disadvantaged }) => disadvantaged), - tap((disadvantagedTracts) => { - for (const id of idsToFetch) { - this.disadvantaged[id] = disadvantagedTracts.includes(id) - } - }), - ) - .subscribe() + if (!idsToFetch.length) { + return of(undefined) } + + const url = '/api/v3/eeej/filter_disadvantaged_tracts/' + return this._httpClient.post<{ status: string; disadvantaged: string[] }>(url, { tract_ids: tractIds }).pipe( + map(({ disadvantaged }) => { + for (const id of idsToFetch) { + this.disadvantaged[id] = disadvantaged.includes(id) + } + }), + ) } - isDisadvantaged(tractId: number): boolean { + isDisadvantaged(tractId: string): boolean { if (tractId in this.disadvantaged) { - return !!this.disadvantaged[tractId] + return this.disadvantaged[tractId] } // TODO: address this comment // console.error(`Tract ${tractId} hasn't previously been fetched, run checkDisadvantagedStatus first`) diff --git a/src/app/core/transloco/transloco-testing.spec.ts b/src/app/core/transloco/transloco-testing.spec.ts new file mode 100644 index 00000000..9714cbec --- /dev/null +++ b/src/app/core/transloco/transloco-testing.spec.ts @@ -0,0 +1,27 @@ +import { TestBed } from '@angular/core/testing' +import { TranslocoService } from '@jsverse/transloco' +import { getTranslocoTestingModule } from './transloco-testing' + +describe('getTranslocoTestingModule', () => { + it('should configure English without requiring a translation dictionary', async () => { + await TestBed.configureTestingModule({ + imports: [getTranslocoTestingModule()], + }).compileComponents() + + const service = TestBed.inject(TranslocoService) + expect(service.getDefaultLang()).toBe('en_US') + expect(service.getActiveLang()).toBe('en_US') + expect(service.getTranslation('en_US')).toEqual({}) + }) + + it('should preload test-specific translations and support interpolation', async () => { + const translations = { Greeting: 'Hello {{ name }}' } + await TestBed.configureTestingModule({ + imports: [getTranslocoTestingModule(translations)], + }).compileComponents() + + const service = TestBed.inject(TranslocoService) + expect(service.getTranslation('en_US')).toEqual(translations) + expect(service.translate('Greeting', { name: 'SEED' })).toBe('Hello SEED') + }) +}) diff --git a/src/app/core/transloco/transloco-testing.ts b/src/app/core/transloco/transloco-testing.ts new file mode 100644 index 00000000..f7d77e52 --- /dev/null +++ b/src/app/core/transloco/transloco-testing.ts @@ -0,0 +1,10 @@ +import type { ModuleWithProviders } from '@angular/core' +import { TranslocoTestingModule } from '@jsverse/transloco' + +export function getTranslocoTestingModule(translations: Record = {}): ModuleWithProviders { + return TranslocoTestingModule.forRoot({ + langs: { en_US: translations }, + translocoConfig: { availableLangs: ['en_US'], defaultLang: 'en_US' }, + preloadLangs: true, + }) +} diff --git a/src/app/modules/inventory-list/map/map.component.html b/src/app/modules/inventory-list/map/map.component.html index 73b58edf..a6a3c630 100644 --- a/src/app/modules/inventory-list/map/map.component.html +++ b/src/app/modules/inventory-list/map/map.component.html @@ -111,6 +111,7 @@ Census Tracts { + let component: MapComponent + let fixture: ComponentFixture + let http: HttpTestingController + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [MapComponent, MatIconTestingModule], + providers: [ + provideHttpClient(), + provideHttpClientTesting(), + { provide: ActivatedRoute, useValue: { snapshot: { paramMap: convertToParamMap({ type: 'properties' }) } } }, + { provide: Router, useValue: {} }, + { provide: ColumnService, useValue: {} }, + { provide: GroupsService, useValue: {} }, + { provide: InventoryService, useValue: {} }, + { provide: LabelService, useValue: {} }, + { provide: OrganizationService, useValue: {} }, + { provide: UserService, useValue: { currentUser$: EMPTY } }, + ], + }) + .overrideComponent(MapComponent, { + set: { imports: [NgTemplateOutlet, MaterialImports], schemas: [NO_ERRORS_SCHEMA] }, + }) + .compileComponents() + + fixture = TestBed.createComponent(MapComponent) + component = fixture.componentInstance + component.censusTractLayer = new VectorLayer({ source: new VectorSource() }) + http = TestBed.inject(HttpTestingController) + }) + + afterEach(() => { + fixture.destroy() + component.censusTractLayer.dispose() + component.map?.dispose() + http.verify() + }) + + for (const initiallyHighlighted of [true, false]) { + it(`should immediately invalidate census tract styles when turning highlights ${initiallyHighlighted ? 'off' : 'on'}`, () => { + component.highlightDACs = initiallyHighlighted + const layer = component.censusTractLayer + const source = layer.getSource() + const revision = layer.getRevision() + const onChange = jasmine.createSpy('onChange').and.callFake(() => { + expect(component.highlightDACs).toBe(!initiallyHighlighted) + }) + layer.on('change', onChange) + + component.toggleHighlightDACs() + + expect(component.highlightDACs).toBe(!initiallyHighlighted) + expect(onChange).toHaveBeenCalledTimes(1) + expect(layer.getRevision()).toBeGreaterThan(revision) + expect(layer.getSource()).toBe(source) + }) + } + + describe('initial census tract loading', () => { + beforeEach(() => { + component.geocodedProperties = [] + component.geocodedTaxlots = [] + component.map = new Map({ + view: new View({ center: [0, 0], zoom: 12 }), + controls: [], + interactions: [], + }) + component.map.setSize([256, 256]) + spyOn(window, 'fetch').and.resolveTo( + new Response( + JSON.stringify({ + type: 'FeatureCollection', + features: [ + { type: 'Feature', properties: { GEOID10: '01001020100' }, geometry: { type: 'Point', coordinates: [0, 0] } }, + { type: 'Feature', properties: { GEOID10: '01001020200' }, geometry: { type: 'Point', coordinates: [0, 1] } }, + ], + }), + ), + ) + }) + + for (const highlightEnabled of [true, false]) { + it(`should load tract styles with highlights ${highlightEnabled ? 'enabled by default' : 'disabled while loading'}`, async () => { + expect(component.highlightDACs).toBeTrue() + const layersReady = jasmine.createSpy('layersReady') + const service = TestBed.inject(MapService) + const checkStatus = service.checkDisadvantagedStatus.bind(service) as MapService['checkDisadvantagedStatus'] + const statusRequested = new Promise((resolve) => { + spyOn(service, 'checkDisadvantagedStatus').and.callFake((tractIds) => { + resolve() + return checkStatus(tractIds) + }) + }) + const loading = component.setLayers().then(layersReady) + await statusRequested + await new Promise((resolve) => setTimeout(resolve, 0)) + + const request = http.expectOne('/api/v3/eeej/filter_disadvantaged_tracts/') + expect(request.request.body).toEqual({ tract_ids: ['01001020100', '01001020200'] }) + expect(layersReady).not.toHaveBeenCalled() + + if (!highlightEnabled) component.toggleHighlightDACs() + request.flush({ status: 'success', disadvantaged: ['01001020100'] }) + await loading + + expect(layersReady).toHaveBeenCalledTimes(1) + const layer = component.censusTractLayer + const styleFunction = layer.getStyleFunction() + for (const feature of layer.getSource().getFeatures() as Feature[]) { + const style = styleFunction(feature, 1) + expect(style).toBeInstanceOf(Style) + if (!(style instanceof Style)) throw new Error('Expected a census tract style') + const shouldHighlight = highlightEnabled && feature.get('GEOID10') === '01001020100' + expect(style.getFill()?.getColor()).toBe(shouldHighlight ? 'rgba(69, 130, 155, 0.4)' : undefined) + expect(style.getStroke()).toBeTruthy() + } + }) + } + }) + + it('should mute the entire highlight control when census tracts are hidden and restore it when shown', async () => { + component.groupId = 1 + component.data = [ + { + id: 1, + bounding_box: '', + centroid: '', + derived_data: {}, + extra_data: {}, + files: [], + labels: [], + measures: [], + scenarios: [], + }, + ] + component.geocodedProperties = [] + component.geocodedTaxlots = [] + await component.setLayers() + component.map = new Map({ layers: [component.censusTractLayer], controls: [], interactions: [] }) + fixture.detectChanges() + + const highlightControl = fixture.debugElement + .queryAll(By.directive(MatCheckbox)) + .find((element) => (element.nativeElement as HTMLElement).textContent.includes('Highlight DACs')) + expect(highlightControl).toBeDefined() + if (!highlightControl) throw new Error('Expected the highlight control') + const checkbox = highlightControl.injector.get(MatCheckbox) + const element = highlightControl.nativeElement as HTMLElement + + expect(checkbox.disabled).toBeFalse() + expect(element.classList.contains('opacity-40')).toBeFalse() + + component.toggleLayer('censusTractLayer', false) + fixture.detectChanges() + + expect(checkbox.disabled).toBeTrue() + expect(checkbox.checked).toBeTrue() + expect(element.querySelector('input')?.disabled).toBeTrue() + expect(element.classList.contains('opacity-40')).toBeTrue() + + component.toggleLayer('censusTractLayer', true) + fixture.detectChanges() + + expect(checkbox.disabled).toBeFalse() + expect(checkbox.checked).toBeTrue() + expect(element.classList.contains('opacity-40')).toBeFalse() + }) +}) diff --git a/src/app/modules/inventory-list/map/map.component.ts b/src/app/modules/inventory-list/map/map.component.ts index 36147055..964aa684 100644 --- a/src/app/modules/inventory-list/map/map.component.ts +++ b/src/app/modules/inventory-list/map/map.component.ts @@ -23,7 +23,7 @@ import Style from 'ol/style/Style' import Text from 'ol/style/Text' import View from 'ol/View' import HexBin from 'ol-ext/source/HexBin' -import { filter, finalize, last, map, mergeMap, of, range, scan, Subject, switchMap, take, takeUntil, tap } from 'rxjs' +import { filter, finalize, firstValueFrom, last, map, mergeMap, of, range, scan, Subject, switchMap, take, takeUntil, tap } from 'rxjs' import type { CurrentUser, Label, LabelOperator, OrgCycle } from '@seed/api' import { ColumnService, GroupsService, InventoryService, LabelService, OrganizationService, UserService } from '@seed/api' import { NotFoundComponent, PageComponent, ProgressBarComponent } from '@seed/components' @@ -332,11 +332,11 @@ export class MapComponent implements OnDestroy, OnInit { source: await this.censusTractSource(), zIndex: this.layers.censusTractLayer.zIndex, style: (feature: Feature) => { - const tractId = feature.get('GEOID10') as number + const tractId = feature.get('GEOID10') as string const isDisadvantaged = this._mapService.isDisadvantaged(tractId) return new Style({ stroke: new Stroke({ color: '#185189', width: 2 }), - fill: isDisadvantaged && this.highlightDACs ? new Fill({ color: '#45829b' }) : undefined, + fill: isDisadvantaged && this.highlightDACs ? new Fill({ color: 'rgba(69, 130, 155, 0.4)' }) : undefined, }) }, }) @@ -525,7 +525,6 @@ export class MapComponent implements OnDestroy, OnInit { return new VectorSource({ features }) } - // this is async. how to handle? async censusTractSource() { let geojson = { type: 'FeatureCollection', features: [] } @@ -536,11 +535,8 @@ export class MapComponent implements OnDestroy, OnInit { const [west, south, east, north] = transformExtent(extents, this.map.getView().getProjection(), 'EPSG:4326') const url = `https://services.arcgis.com/P3ePLMYs2RVChkJx/arcgis/rest/services/usa_november_2022/FeatureServer/0/query?where=1%3D1&outFields=GEOID10&geometry=${west}%2C${south}%2C${east}%2C${north}&geometryType=esriGeometryEnvelope&inSR=4326&spatialRel=esriSpatialRelIntersects&outSR=4326&f=geojson` geojson = (await (await fetch(url)).json()) as { type: string; features: { properties: { GEOID10: string } }[] } - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - const tractIds = geojson.features.reduce((acc: string[], feature: { properties: { GEOID10: string } }) => { - return acc.concat(feature.properties.GEOID10) - }, []) - this._mapService.checkDisadvantagedStatus(tractIds as number[]) + const tractIds = geojson.features.map((feature: { properties: { GEOID10: string } }) => feature.properties.GEOID10) + await firstValueFrom(this._mapService.checkDisadvantagedStatus(tractIds)) } } catch (e) { console.error(e) @@ -615,6 +611,7 @@ export class MapComponent implements OnDestroy, OnInit { toggleHighlightDACs() { this.highlightDACs = !this.highlightDACs + this.censusTractLayer.changed() } navigateToDetail(viewId: number) { diff --git a/src/app/modules/salesforce-login/salesforce-login.component.spec.ts b/src/app/modules/salesforce-login/salesforce-login.component.spec.ts index d7d0b38c..03026b50 100644 --- a/src/app/modules/salesforce-login/salesforce-login.component.spec.ts +++ b/src/app/modules/salesforce-login/salesforce-login.component.spec.ts @@ -1,8 +1,10 @@ import type { ComponentFixture } from '@angular/core/testing' import { TestBed } from '@angular/core/testing' +import { MatIconTestingModule } from '@angular/material/icon/testing' import { ActivatedRoute, Router } from '@angular/router' import { of } from 'rxjs' import { OrganizationService, SalesforcePortfolioService } from '@seed/api' +import { getTranslocoTestingModule } from 'app/core/transloco/transloco-testing' import { SalesforceLoginComponent } from './salesforce-login.component' describe('SalesforceLoginComponent', () => { @@ -11,7 +13,15 @@ describe('SalesforceLoginComponent', () => { beforeEach(async () => { await TestBed.configureTestingModule({ - imports: [SalesforceLoginComponent], + imports: [ + SalesforceLoginComponent, + MatIconTestingModule, + getTranslocoTestingModule({ + 'SEED is now logged in to Salesforce.': 'SEED is now logged in to Salesforce.', + 'Return to Portfolio Summary': 'Return to Portfolio Summary', + 'Return to Organization Settings': 'Return to Organization Settings', + }), + ], providers: [ { provide: ActivatedRoute, useValue: { queryParams: of({ code: 'test-code' }) } }, { provide: Router, useValue: { navigate: jasmine.createSpy('navigate').and.returnValue(Promise.resolve(true)) } }, @@ -30,5 +40,7 @@ describe('SalesforceLoginComponent', () => { it('should create', () => { expect(component).toBeTruthy() + const element = fixture.nativeElement as HTMLElement + expect(element.textContent).toContain('SEED is now logged in to Salesforce.') }) })