Skip to content

feat:connecting trawl lines between linked buoys #1276

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

Merged
merged 4 commits into from
Apr 23, 2025
Merged
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
73 changes: 73 additions & 0 deletions src/BuoyTrawlLineLayer/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import React from 'react';
import { useSelector } from 'react-redux';
import { lineString } from '@turf/turf';

import { getMapSubjectFeatureCollectionWithVirtualPositioning } from '../selectors/subjects';
import useMapLayers from '../hooks/useMapLayers';
import useMapSources from '../hooks/useMapSources';

const lineLayout = {
'line-join': 'round',
'line-cap': 'round',
};

const linePaint = {
'line-color': 'black',
'line-opacity': 0.7,
'line-gap-width': 1,
'line-width': 2,
};

const TRAWL_SOURCE_ID = 'trawl-lines-source';
const TRAWL_LAYER_ID = 'trawl-lines-layer';

const subjectIsBuoyLineEligible = (subjectFeature = {}, _index, allSubjects = []) => {
const subject = subjectFeature.properties;

const isBuoy = subject.subject_subtype === 'ropeless_buoy_device';
if (!isBuoy) return false;

const devices = subject.additional?.devices ?? [];
const isLine = devices.length > 1;

if (!isLine) return false;

const lineContainsValidSubjects = devices.every(({ device_id }) => allSubjects.find(({ properties }) => properties.name === device_id));

return lineContainsValidSubjects;
};

const createTrawlLineGeoJSON = (buoySubjectFeatures) => {
return buoySubjectFeatures.reduce((accumulator, { properties }) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Honestly, I've stared at this function for a while haha. I feel like we could make it a bit easier to understand maybe by changing some variable names or not extracting properties from objects like: ({ properties }) => properties.additional... -> (buoySubjectFeature) => buoySubjectFeature.properties.additional etc...

Copy link
Collaborator Author

@JoshuaVulcan JoshuaVulcan Apr 8, 2025

Choose a reason for hiding this comment

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

Summary version:

We can say that the devices listed for a buoy feature's additional.devices should be the names of the subjects comprising that trawl.

We can't say that the coordinates included in the additional data devices array data is up-to-date.

So the eligibility-checker function first ensures a trawl line is completely valid and comprised of available subjects for the user.

Then createTrawlLineGeoJSON maps those device names back to their subject's last known position and builds out the linestring coordinates from there.

Copy link
Contributor

Choose a reason for hiding this comment

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

That makes sense. Maybe a couple tweaks or a brief comment could make that more obvious in the code for future reference.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a descriptive inline comment

const coordinates = // build the coordinates from each subject's location
properties.additional.devices.map(({ device_id }) =>
buoySubjectFeatures.find(({ properties }) =>
properties.name === device_id)?.geometry?.coordinates ?? []
);

accumulator.features.push(lineString(coordinates));
return accumulator;

}, { type: 'FeatureCollection', features: [] });
};

const BuoyLineLayer = (_props) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: why the (_props) => { instead of simply () => {?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

just a matter of preference!

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean, it's a default linter rule (no-unused-vars) that we explicitly skip by prefixing the _. It feels weird to add it intentionally for nothing 🤔

const mapSubjects = useSelector(getMapSubjectFeatureCollectionWithVirtualPositioning);

const buoySubjects = mapSubjects.features.filter(subjectIsBuoyLineEligible);
const trawlLineGeoJSON = createTrawlLineGeoJSON(buoySubjects);

useMapSources([{ id: TRAWL_SOURCE_ID, data: trawlLineGeoJSON }]);
useMapLayers([{
id: TRAWL_LAYER_ID,
type: 'line',
sourceId: TRAWL_SOURCE_ID,
layout: lineLayout,
paint: linePaint,
}]);

return null;

};

export default BuoyLineLayer;
127 changes: 127 additions & 0 deletions src/BuoyTrawlLineLayer/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import React from 'react';
import { render } from '@testing-library/react';
import { useSelector } from 'react-redux';
import BuoyLineLayer from './';
import useMapLayers from '../hooks/useMapLayers';
import useMapSources from '../hooks/useMapSources';

jest.mock('react-redux', () => ({
useSelector: jest.fn(),
}));

jest.mock('../hooks/useMapLayers', () => jest.fn());
jest.mock('../hooks/useMapSources', () => jest.fn());

const createMockSubject = (name, subtype, devices, coordinates) => ({
type: 'Feature',
properties: {
name,
subject_subtype: subtype,
additional: { devices },
},
geometry: {
type: 'Point',
coordinates,
},
});

describe('BuoyLineLayer', () => {

beforeEach(() => {
jest.clearAllMocks();
});

test('should not render any lines when no buoy subjects are present', () => {
useSelector.mockReturnValue({
features: [],
});

render(<BuoyLineLayer />);

expect(useMapSources).toHaveBeenCalledWith([
{
id: 'trawl-lines-source',
data: { type: 'FeatureCollection', features: [] },
},
]);
expect(useMapLayers).toHaveBeenCalled();
});

test('should filter out non-buoy subjects', () => {
const mockFeatures = [
createMockSubject('buoy1', 'other_type', [], [10, 20]),
createMockSubject('buoy2', 'ropeless_buoy_device', [{ device_id: 'device1' }], [30, 40]),
];

useSelector.mockReturnValue({
features: mockFeatures,
});

render(<BuoyLineLayer />);

expect(useMapSources).toHaveBeenCalledWith([
{
id: 'trawl-lines-source',
data: { type: 'FeatureCollection', features: [] },
},
]);
});

test('should create trawl lines for valid buoy subjects', () => {
const device1 = createMockSubject('device1', 'ropeless_buoy_device', [{ device_id: 'device1' }, { device_id: 'device2' }], [10, 20]);
const device2 = createMockSubject('device2', 'ropeless_buoy_device', [{ device_id: 'device1' }, { device_id: 'device2' }], [30, 40]);

const mockFeatures = [
device1,
device2,
];

useSelector.mockReturnValue({
features: mockFeatures,
});

render(<BuoyLineLayer />);

expect(useMapSources).toHaveBeenCalledWith([
{
id: 'trawl-lines-source',
data: {
type: 'FeatureCollection',
features: expect.arrayContaining([
{
type: 'Feature',
properties: {},
geometry: {
type: 'LineString',
coordinates: [[10, 20], [30, 40]]
}
}
])
}
}
]);
});

test('should handle invalid device references', () => {
const mockFeatures = [
createMockSubject('device1', 'ropeless_buoy_device', [], [10, 20]),
createMockSubject('buoy1', 'ropeless_buoy_device', [
{ device_id: 'device1' },
{ device_id: 'device2' }, // invalid reference
], [50, 60]),
];

useSelector.mockReturnValue({
features: mockFeatures,
});

render(<BuoyLineLayer />);

expect(useMapSources).toHaveBeenCalledWith([
{
id: 'trawl-lines-source',
data: { type: 'FeatureCollection', features: [] },
},
]);
});
});
23 changes: 13 additions & 10 deletions src/Map/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import DelayedUnmount from '../DelayedUnmount';
import EarthRangerMap, { withMap } from '../EarthRangerMap';
import EventsLayer from '../EventsLayer';
import SubjectsLayer from '../SubjectsLayer';
import BuoyTrawlLineLayer from '../BuoyTrawlLineLayer';
import StaticSensorsLayer from '../StaticSensorsLayer';
import TracksLayer from '../TracksLayer';
import PatrolStartStopLayer from '../PatrolStartStopLayer';
Expand Down Expand Up @@ -150,7 +151,7 @@ const Map = ({
const timeSliderActive = timeSliderState.active;

const isDrawingEventGeometry = mapLocationSelection.isPickingLocation
&& mapLocationSelection.mode === MAP_LOCATION_SELECTION_MODES.EVENT_GEOMETRY;
&& mapLocationSelection.mode === MAP_LOCATION_SELECTION_MODES.EVENT_GEOMETRY;

const isSelectingEventLocation = mapLocationSelection.isPickingLocation
&& mapLocationSelection.event
Expand Down Expand Up @@ -216,7 +217,7 @@ const Map = ({
.then((latestMapSubjects) => timeSliderActive
? fetchMapSubjectTracksForTimeslider(latestMapSubjects)
: Promise.resolve(latestMapSubjects))
.catch(() => {});
.catch(() => { });
},
[
dispatch,
Expand Down Expand Up @@ -363,8 +364,8 @@ const Map = ({
);
}, [dispatch]);

const onCloseReportHeatmap = useCallback(() => {
dispatch (
const onCloseReportHeatmap = useCallback(() => {
dispatch(
setReportHeatmapVisibility(false)
);
}, [dispatch]);
Expand Down Expand Up @@ -547,7 +548,7 @@ const Map = ({
hidePopup(popup.id);
}
}
// eslint-disable-next-line react-hooks/exhaustive-deps
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [map, timeSliderState.virtualDate]);

useEffect(() => {
Expand Down Expand Up @@ -612,10 +613,10 @@ const Map = ({
<ClustersLayer onShowClusterSelectPopup={onShowClusterSelectPopup} />

<EventsLayer
mapImages={mapImages}
onEventClick={onSelectEvent}
bounceEventIDs={bounceEventIDs}
/>
mapImages={mapImages}
onEventClick={onSelectEvent}
bounceEventIDs={bounceEventIDs}
/>

<SubjectsLayer mapImages={mapImages} onSubjectClick={onSelectSubject} />

Expand All @@ -629,7 +630,7 @@ const Map = ({

<DelayedUnmount isMounted={!currentTab && !mapLocationSelection.isPickingLocation}>
<div className='floating-report-filter'>
<EventFilter className='report-filter'/>
<EventFilter className='report-filter' />
</div>
</DelayedUnmount>

Expand Down Expand Up @@ -657,6 +658,8 @@ const Map = ({
{subjectTracksVisible && <TracksLayer onPointClick={onTimepointClick} showTimepoints={showTrackTimepoints} />}
{patrolTracksVisible && <PatrolStartStopLayer />}

<BuoyTrawlLineLayer />

{patrolTracksVisible && <PatrolTracks onPointClick={onTimepointClick} />}

<FeatureLayer
Expand Down