Skip to content

zoom to all fiscal polygons, not only current year, add helper methods to handle activity boundaries #584

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 15, 2025
Merged

zoom to all fiscal polygons, not only current year, add helper methods to handle activity boundaries #584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7ffd49d
Wfprev 222 fixer a (#583)
yzlucas Apr 15, 2025
e11c0e3
sonar bugs (#585)
yzlucas Apr 15, 2025
76b3ced
sonar fix (#586)
yzlucas Apr 15, 2025
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
7 changes: 6 additions & 1 deletion ...war/src/main/angular/src/app/components/edit-project/fiscal-map/fiscal-map.component.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
.map-container{
margin-top: 12px;
}
}

#fiscalMap {
height: 100%;
width: 100%;
}
48 changes: 48 additions & 0 deletions .../src/main/angular/src/app/components/edit-project/fiscal-map/fiscal-map.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,54 @@ describe('FiscalMapComponent', () => {
expect(plotSpy).not.toHaveBeenCalled();
expect(coordSpy).toHaveBeenCalled();
}));

it('should correctly map activities with fiscal data', () => {
const fiscal = { fiscalYear: 2023, projectPlanFiscalGuid: 'fiscal-guid' };
const response = {
_embedded: {
activities: [
{ activityGuid: 'a1', name: 'Activity 1' },
{ activityGuid: 'a2', name: 'Activity 2' }
]
}
};

const result = (component as any).mapFiscalActivities(response, fiscal);
Copy link
Contributor

Choose a reason for hiding this comment

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

Try to avoid using "any". You can always define a type and use that instead so it's explicit and we're not just letting anything through. This can also help with error checking

expect(result.length).toBe(2);
expect(result[0]).toEqual(jasmine.objectContaining({
activityGuid: 'a1',
fiscalYear: 2023,
projectPlanFiscalGuid: 'fiscal-guid'
}));
});

it('should correctly map activity boundary if boundary exists', () => {
const activity = {
activityGuid: 'act-1',
fiscalYear: 2022
};
const boundary = {
_embedded: {
activityBoundary: [{ geometry: { type: 'Polygon' } }]
}
};

const result = (component as any).mapActivityBoundary(boundary, activity);
expect(result).toEqual({
activityGuid: 'act-1',
fiscalYear: 2022,
boundary: boundary._embedded.activityBoundary
});
});

it('should return null if boundary is null', () => {
const activity = {
activityGuid: 'act-1',
fiscalYear: 2022
};
const result = (component as any).mapActivityBoundary(null, activity);
expect(result).toBeNull();
});


describe('openFullMap()', () => {
Expand Down
165 changes: 88 additions & 77 deletions ...v-war/src/main/angular/src/app/components/edit-project/fiscal-map/fiscal-map.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ export class FiscalMapComponent implements AfterViewInit, OnDestroy, OnInit {
};

constructor(
private projectService: ProjectService,
private route: ActivatedRoute,
readonly projectService: ProjectService,
readonly route: ActivatedRoute,
protected router: Router,
) {}

Expand Down Expand Up @@ -66,18 +66,18 @@ export class FiscalMapComponent implements AfterViewInit, OnDestroy, OnInit {
const lat = parseFloat(this.projectLatitude);
const lng = parseFloat(this.projectLongitude);

const marker = L.marker([lat, lng]).addTo(this.map);
L.marker([lat, lng]).addTo(this.map);

this.map.setView([lat, lng], 14);
}
}
})
}
getProjectBoundary() {
this.projectGuid = this.route.snapshot?.queryParamMap?.get('projectGuid') || '';
this.projectGuid = this.route.snapshot?.queryParamMap?.get('projectGuid') ?? '';
if (this.projectGuid) {
this.projectService.getProjectBoundaries(this.projectGuid).subscribe((data) => {
const boundary = data?._embedded?.projectBoundary || [];
const boundary = data?._embedded?.projectBoundary ?? [];
this.projectBoundary = boundary;

if (this.map && boundary.length > 0) {
Expand All @@ -86,71 +86,85 @@ export class FiscalMapComponent implements AfterViewInit, OnDestroy, OnInit {
});
}
}
getAllActivitiesBoundaries() {
this.projectGuid = this.route.snapshot?.queryParamMap?.get('projectGuid') || '';

if (this.projectGuid) {
this.projectService.getProjectFiscalsByProjectGuid(this.projectGuid).subscribe((data) => {
this.projectFiscals = (data._embedded?.projectFiscals || []).sort(
(a: { fiscalYear: number }, b: { fiscalYear: number }) => a.fiscalYear - b.fiscalYear
);

const activityRequests = this.projectFiscals.map(fiscal =>
this.projectService.getFiscalActivities(this.projectGuid, fiscal.projectPlanFiscalGuid).pipe(
map((response: any) => {
const activities = response?._embedded?.activities || [];
return activities.map((activity: any) => ({
...activity,
fiscalYear: fiscal.fiscalYear,
projectPlanFiscalGuid: fiscal.projectPlanFiscalGuid
}));
})
)
);

forkJoin(activityRequests).subscribe((allActivityArrays) => {
const allActivities = allActivityArrays.flat();

if (allActivities.length === 0) {
// no activites at all.
this.getProjectCoordinates();
return;
}
const boundaryRequests = allActivities.map(activity =>
this.projectService
.getActivityBoundaries(this.projectGuid, activity.projectPlanFiscalGuid, activity.activityGuid)
.pipe(
map(boundary => boundary ? ({
activityGuid: activity.activityGuid,
fiscalYear: activity.fiscalYear,
boundary: boundary?._embedded?.activityBoundary
}) : null),
)
);

forkJoin(boundaryRequests).subscribe((allResults) => {
this.allActivityBoundaries = allResults.filter(r =>
r !== null && r.boundary && Object.keys(r.boundary).length > 0
);
const hasActivityPolygons = this.allActivityBoundaries.length > 0;
const hasProjectPolygons = this.projectBoundary?.length > 0;

if (hasActivityPolygons && this.map) {
this.plotBoundariesOnMap(this.allActivityBoundaries);
}

// Only show pin if NO polygons exist at all
if (!hasActivityPolygons && !hasProjectPolygons) {
this.getProjectCoordinates();
}
});
});
});
getAllActivitiesBoundaries(): void {
this.projectGuid = this.route.snapshot?.queryParamMap?.get('projectGuid') ?? '';
if (!this.projectGuid) return;

this.projectService.getProjectFiscalsByProjectGuid(this.projectGuid).subscribe(data =>
this.handleFiscalsResponse(data)
);
}

private handleFiscalsResponse(data: any): void {
this.projectFiscals = (data._embedded?.projectFiscals ?? []).sort(
(a: { fiscalYear: number }, b: { fiscalYear: number }) => a.fiscalYear - b.fiscalYear
);

const activityRequests = this.projectFiscals.map(fiscal =>
this.projectService.getFiscalActivities(this.projectGuid, fiscal.projectPlanFiscalGuid).pipe(
map(response => this.mapFiscalActivities(response, fiscal))
)
);

forkJoin(activityRequests).subscribe(allActivityArrays =>
this.handleActivitiesResponse(allActivityArrays.flat())
);
}

private mapFiscalActivities(response: any, fiscal: any): any[] {
Copy link
Contributor

Choose a reason for hiding this comment

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

any again

const activities = response?._embedded?.activities ?? [];
return activities.map((activity: any) => ({
...activity,
fiscalYear: fiscal.fiscalYear,
projectPlanFiscalGuid: fiscal.projectPlanFiscalGuid
}));
}

private handleActivitiesResponse(allActivities: any[]): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

any again

if (allActivities.length === 0) {
this.getProjectCoordinates();
return;
}

const boundaryRequests = allActivities.map(activity =>
this.projectService
.getActivityBoundaries(this.projectGuid, activity.projectPlanFiscalGuid, activity.activityGuid)
.pipe(
map(boundary => this.mapActivityBoundary(boundary, activity))
)
);

forkJoin(boundaryRequests).subscribe(allResults =>
this.handleBoundariesResponse(allResults)
);
}

private mapActivityBoundary(boundary: any, activity: any): any {
Copy link
Contributor

Choose a reason for hiding this comment

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

You have a could to types here defined. boundary object, whatever that is, or the manually defined type, or null. Rather than any define these types and set the return to be : type | type | null vs : any

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good, I will create some objects in model.ts to store them. Will go through the application to use defined type with updated method signature. I will create a separate to address them

return boundary ? {
activityGuid: activity.activityGuid,
fiscalYear: activity.fiscalYear,
boundary: boundary?._embedded?.activityBoundary
} : null;
}

private handleBoundariesResponse(results: any[]): void {
this.allActivityBoundaries = results.filter(r => r?.boundary && Object.keys(r.boundary).length > 0);

const hasActivityPolygons = this.allActivityBoundaries.length > 0;
const hasProjectPolygons = this.projectBoundary?.length > 0;

if (hasActivityPolygons && this.map) {
this.plotBoundariesOnMap(this.allActivityBoundaries);
}

if (!hasActivityPolygons && !hasProjectPolygons) {
this.getProjectCoordinates();
}
}

plotBoundariesOnMap(boundaries: any[]): void {
const currentFiscalPolygons: L.Layer[] = [];
const allFiscalPolygons: L.Layer[] = [];

boundaries.forEach(boundaryEntry => {
const fiscalYear = boundaryEntry.fiscalYear;
Expand All @@ -167,20 +181,18 @@ export class FiscalMapComponent implements AfterViewInit, OnDestroy, OnInit {
for (const item of boundaryEntry.boundary) {
const geometry = item.geometry;
if (!geometry) continue;

const geoJsonOptions: L.GeoJSONOptions = {
style: {
color,
weight: fiscalYear === this.currentFiscalYear ? 4 : 2,
fillOpacity: 0.1
}
};

const addToMap = (geom: any) => {
const layer = L.geoJSON(geom, geoJsonOptions).addTo(this.map!);
if (fiscalYear === this.currentFiscalYear) {
currentFiscalPolygons.push(layer);
}
allFiscalPolygons.push(layer); // Track all layers
};

if (geometry.type === 'GeometryCollection') {
Expand All @@ -191,12 +203,11 @@ export class FiscalMapComponent implements AfterViewInit, OnDestroy, OnInit {
addToMap(geometry);
}
}

});

// Zoom to current fiscal year polygons
if (currentFiscalPolygons.length > 0) {
const group = L.featureGroup(currentFiscalPolygons);
// Zoom to ALL fiscal year polygons
if (allFiscalPolygons.length > 0) {
const group = L.featureGroup(allFiscalPolygons);
this.map!.fitBounds(group.getBounds(), { padding: [20, 20] });
}
}
Expand Down Expand Up @@ -247,17 +258,17 @@ export class FiscalMapComponent implements AfterViewInit, OnDestroy, OnInit {
zoomControl: true,
});

(this.map.zoomControl as L.Control.Zoom).setPosition('topright');
(this.map.zoomControl).setPosition('topright');

L.tileLayer('https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png', {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to parameterize this. It isn't likely to change much or at all, but just in case. Should we be sticking with openstreetmap or using DataBC BC basemaps?

attribution: '© OpenStreetMap contributors'
}).addTo(this.map);

const legendHelper = new LeafletLegendService();
legendHelper.addLegend(this.map!, this.fiscalColorMap);
legendHelper.addLegend(this.map, this.fiscalColorMap);
const bcBounds = L.latLngBounds([48.3, -139.1], [60.0, -114.0]);
this.map.fitBounds(bcBounds, { padding: [20, 20] });
createFullPageControl(() => this.openFullMap()).addTo(this.map!);
createFullPageControl(() => this.openFullMap()).addTo(this.map);
}

openFullMap(): void {
Expand Down
Loading