Transit panel: add a button to delete all unused nodes

locales/en/transit.json

@@ -50,10 +50,12 @@
         },
         "MultipleDelete": "Delete nodes",
         "ConfirmMultipleDelete": "Do you confirm deletion of those stop nodes? Only the nodes through which no path passes will be deleted.",
+        "ConfirmAllDelete": "Do you confirm deletion of all unused stop nodes? Only the nodes through which no path passes will be deleted.",
         "deleteSelectedNodes": "Delete unused selected nodes",
-        "deleteSelectedPolygon":"Cancel selection",
-        "editSelectedNodes":"Edit selected nodes",
-        "editFrozenSelectedNodesWarning":"only unfrozen nodes will be saved"
+        "deleteSelectedPolygon": "Cancel selection",
+        "editSelectedNodes": "Edit selected nodes",
+        "editFrozenSelectedNodesWarning": "only unfrozen nodes will be saved",
+        "DeleteAllUnusedNodes": "Delete all unused nodes"
     },
     "transitAgency": {
         "List": "Agencies",
@@ -466,7 +468,6 @@
         "ShowPathEditHelp": "Show path edit help",
         "HidePathEditHelp": "Hide path edit help",
         "warningFromGtfs": "Warning! This transit path was imported from a GTFS. Recomputing its path could change the geography and timings.",
-
         "directions": {
             "loop": "Loop",
             "outbound": "Outbound",
@@ -656,7 +657,7 @@
             "DataSourceDoesNotExists": "The specified data source does not exists.",
             "DataSourceAlreadyExists": "A data source with that name already exists for OD calculations.",
             "InvalidOdTripsDataSource": "Invalid OD calculations data source.",
-            "ErrorCalculatingLocation":  "Error calculating location {{id}}",
+            "ErrorCalculatingLocation": "Error calculating location {{id}}",
             "UserDiskQuotaReached": "Maximum allowed disk space has been reached. Please delete old tasks to delete their files.",
             "RoutingParametersInvalidForBatch": "Routing parameters are invalid. Make sure the scenario is set and times have a valid value."
         },
@@ -722,9 +723,7 @@
         "HH_MM__HH_MM_SS": "HH:MM or HH:MM:SS",
         "HMM": "HMM",
         "SecondsSinceMidnight": "Seconds after midnight"
-
     },
-
     "gtfs": {
         "ButtonImport": "Import",
         "Import": "Import from a GTFS feed",

locales/fr/transit.json

@@ -50,10 +50,12 @@
         },
         "MultipleDelete": "Supprimer les noeuds d'arrêt",
         "ConfirmMultipleDelete": "Confirmez-vous que vous désirez bien supprimer ces noeuds d'arrêt? Seuls les noeuds ne desservant aucun trajet seront supprimés.",
+        "ConfirmAllDelete": "Confirmez-vous que vous désirez bien supprimer tous les noeuds d'arrêts inutilisés? Seuls les noeuds ne desservant aucun trajet seront supprimés.",
         "deleteSelectedNodes": "Supprimer les noeuds sélectionnés et inutilisés",
         "deleteSelectedPolygon": "Annuler la sélection",
         "editSelectedNodes": "Modifier les noeuds sélectionnés",
-        "editFrozenSelectedNodesWarning": "Seulement les noeuds non verrouillés seront sauvegardés"
+        "editFrozenSelectedNodesWarning": "Seulement les noeuds non verrouillés seront sauvegardés",
+        "DeleteAllUnusedNodes": "Supprimer tous les noeuds non utilisés"
     },
     "transitAgency": {
         "List": "Agences",

packages/transition-backend/src/api/__tests__/transit.routes.test.ts

@@ -10,19 +10,19 @@ import { v4 as uuidV4 } from 'uuid';
 import * as Status from 'chaire-lib-common/lib/utils/Status';
 import transitRoutes from '../transit.socketRoutes';
 import transitScheduleQueries from '../../models/db/transitSchedules.db.queries';
+import transitNodesDbQueries from '../../models/db/transitNodes.db.queries';
 
 const socketStub = new EventEmitter();
 transitRoutes(socketStub);
 
-const mockedDbQuery = jest.fn();
-
 jest.mock('../../models/db/transitNodes.db.queries', () => {
     return {
-        getAssociatedPathIds: jest.fn().mockImplementation(async () => {
-            return mockedDbQuery();
-        })
+        getAssociatedPathIds: jest.fn(),
+        deleteMultipleUnused: jest.fn()
     }
 });
+const mockedDbQuery = transitNodesDbQueries.getAssociatedPathIds as jest.MockedFunction<typeof transitNodesDbQueries.getAssociatedPathIds>;
+const deleteUnusedMock = transitNodesDbQueries.deleteMultipleUnused as jest.MockedFunction<typeof transitNodesDbQueries.deleteMultipleUnused>;
 
 jest.mock('../../models/db/transitSchedules.db.queries', () => {
     return {
@@ -111,4 +111,54 @@ describe('Schedules: get schedules for line', () => {
             done();
         });
     });
+});
+
+describe('transitNodes: delete unused', () => {
+
+    const nodeId1 = uuidV4();
+    const nodeId2 = uuidV4();
+
+    beforeEach(() => {
+        deleteUnusedMock.mockClear();
+    })
+
+    test('Delete some unused', (done) => {
+        deleteUnusedMock.mockResolvedValueOnce([nodeId1]);
+        socketStub.emit('transitNodes.deleteUnused', [nodeId1, nodeId2], function (status) {
+            expect(Status.isStatusOk(status));
+            expect(Status.unwrap(status)).toEqual([nodeId1]);
+            expect(deleteUnusedMock).toHaveBeenCalledWith([nodeId1, nodeId2]);
+            done();
+        });
+    });
+
+    test('Delete all unused', (done) => {
+        deleteUnusedMock.mockResolvedValueOnce([nodeId1]);
+        socketStub.emit('transitNodes.deleteUnused', undefined, function (status) {
+            expect(Status.isStatusOk(status));
+            expect(Status.unwrap(status)).toEqual([nodeId1]);
+            expect(deleteUnusedMock).toHaveBeenCalledWith('all');
+            done();
+        });
+    });
+
+    test('Delete all unused with null', (done) => {
+        deleteUnusedMock.mockResolvedValueOnce([nodeId1]);
+        socketStub.emit('transitNodes.deleteUnused', null, function (status) {
+            expect(Status.isStatusOk(status));
+            expect(Status.unwrap(status)).toEqual([nodeId1]);
+            expect(deleteUnusedMock).toHaveBeenCalledWith('all');
+            done();
+        });
+    });
+
+    test('Delete some unused, with error', (done) => {
+        deleteUnusedMock.mockRejectedValueOnce('error deleting nodes');
+        socketStub.emit('transitNodes.deleteUnused', [nodeId1, nodeId2], function (status) {
+            expect(!Status.isStatusOk(status)).toBe(true);
+            expect(Status.isStatusError(status)).toBe(true);
+            expect((status as any).error).toBe('Error deleting unused nodes');
+            done();
+        });
+    });
 });

packages/transition-backend/src/api/transit.socketRoutes.ts

@@ -14,6 +14,7 @@ import serviceLocator from 'chaire-lib-common/lib/utils/ServiceLocator';
 import * as Status from 'chaire-lib-common/lib/utils/Status';
 import nodesDbQueries from '../models/db/transitNodes.db.queries';
 import schedulesDbQueries from '../models/db/transitSchedules.db.queries';
+import { TransitApi } from 'transition-common/lib/api/transit';
 
 /**
  * Add routes specific to the transit objects
@@ -80,4 +81,25 @@ export default function (socket: EventEmitter) {
             }
         }
     );
+
+    socket.on(
+        TransitApi.DELETE_UNUSED_NODES,
+        async (nodeIds: string[] | undefined, callback: (status: Status.Status<string[]>) => void) => {
+            try {
+                const deletedNodeIds = await nodesDbQueries.deleteMultipleUnused(
+                    nodeIds === undefined || nodeIds === null ? 'all' : nodeIds
+                );
+                callback(Status.createOk(deletedNodeIds));
+            } catch (error) {
+                console.error(
+                    `An error occurred while deleting ${
+                        nodeIds === undefined || nodeIds === null ? 'all' : nodeIds.length
+                    } unused nodes: ${error}`
+                );
+                if (typeof callback === 'function') {
+                    callback(Status.createError('Error deleting unused nodes'));
+                }
+            }
+        }
+    );
 }

packages/transition-backend/src/models/db/__tests__/transitNodes.db.test.ts

@@ -272,7 +272,22 @@ describe(`${objectName}`, () => {
 
         // Delete multiple nodes, some of which have paths associated, only the
         // one without path should be deleted
-        const ids = await dbQueries.deleteMultiple([newObjectAttributes.id, newObjectAttributes2.id, newObjectAttributes3.id]);
+        const ids = await dbQueries.deleteMultipleUnused([newObjectAttributes.id, newObjectAttributes2.id, newObjectAttributes3.id]);
+        expect(ids).toEqual([newObjectAttributes3.id]);
+        expect(await dbQueries.exists(newObjectAttributes.id)).toBe(true);
+        expect(await dbQueries.exists(newObjectAttributes2.id)).toBe(true);
+        expect(await dbQueries.exists(newObjectAttributes3.id)).toBe(false);
+
+    });
+
+    test('should not delete all nodes nodes from database if paths exist', async () => {
+        // Add a new node, not associated with a path
+        const newObject = new ObjectClass(newObjectAttributes3, true);
+        await dbQueries.create(newObject.attributes);
+        expect(await dbQueries.exists(newObjectAttributes3.id)).toBe(true);
+
+        // Delete all unused nodes
+        const ids = await dbQueries.deleteMultipleUnused('all');
         expect(ids).toEqual([newObjectAttributes3.id]);
         expect(await dbQueries.exists(newObjectAttributes.id)).toBe(true);
         expect(await dbQueries.exists(newObjectAttributes2.id)).toBe(true);
@@ -288,7 +303,7 @@ describe(`${objectName}`, () => {
         expect(id).toBe(newObjectAttributes.id);
         expect(await dbQueries.exists(newObjectAttributes.id)).toBe(false);
 
-        const ids = await dbQueries.deleteMultiple([newObjectAttributes.id, newObjectAttributes2.id]);
+        const ids = await dbQueries.deleteMultipleUnused([newObjectAttributes.id, newObjectAttributes2.id]);
         expect(ids).toEqual([newObjectAttributes2.id]);
 
     });

packages/transition-backend/src/models/db/transitNodes.db.queries.ts

@@ -240,12 +240,14 @@ const deleteIfUnused = async (id: string): Promise<string | undefined> => {
     }
 };
 
-const deleteMultipleUnused = function (ids: string[]): Promise<string[]> {
+const deleteMultipleUnused = function (ids: string[] | 'all'): Promise<string[]> {
     return new Promise((resolve, reject) => {
         const notInQuery = knex.distinct(knex.raw('unnest(nodes)')).from(pathTableName);
-        return knex(tableName)
-            .whereIn('id', ids)
-            .whereNotIn('id', notInQuery)
+        const deleteQuery = knex(tableName).whereNotIn('id', notInQuery);
+        if (ids !== 'all') {
+            deleteQuery.whereIn('id', ids);
+        }
+        return deleteQuery
             .del()
             .returning('id')
             .then((ret) => {
@@ -281,7 +283,7 @@ export default {
         return updateMultiple(knex, tableName, attributesCleaner, updatedObjects, returning);
     },
     delete: deleteIfUnused,
-    deleteMultiple: deleteMultipleUnused,
+    deleteMultipleUnused,
     truncate: truncate.bind(null, knex, tableName),
     destroy: destroy.bind(null, knex),
     collection,

packages/transition-common/src/api/transit.ts

@@ -0,0 +1,15 @@
+/*
+ * Copyright 2023, Polytechnique Montreal and contributors
+ *
+ * This file is licensed under the MIT License.
+ * License text available at https://opensource.org/licenses/MIT
+ */
+export class TransitApi {
+    /**
+     * Socket route name to delete the unused transit nodes
+     *
+     * @static
+     * @memberof TransitApi
+     */
+    static readonly DELETE_UNUSED_NODES = 'transitNodes.deleteUnused';
+}

packages/transition-frontend/src/components/forms/node/TransitNodePanel.tsx

@@ -20,6 +20,7 @@ import TransitNodeCollectionEdit from './TransitNodeCollectionEdit';
 import NodesImportForm from './TransitNodeImportForm';
 import NodeCollection from 'transition-common/lib/services/nodes/NodeCollection';
 import Node from 'transition-common/lib/services/nodes/Node';
+import { deleteUnusedNodes } from '../../../services/transitNodes/transitNodesUtils';
 
 // Using a state object instead of 2 useState hooks because we want this object
 // to be modified and cause a re-render if the selection or collection was
@@ -36,6 +37,7 @@ interface NodePanelState {
 const NodePanel: React.FunctionComponent<WithTranslation> = (props: WithTranslation) => {
     const [importerSelected, setImporterSelected] = React.useState(false);
     const [lastOptionIsSelectedNodes, setLastOptionIsSelectedNodes] = React.useState(false);
+    const [confirmDeleteModalIsOpened, setConfirmDeleteModalIsOpened] = React.useState(false);
     const [state, setState] = React.useState<NodePanelState>({
         stationCollection: serviceLocator.collectionManager.get('stations'),
         nodeCollection: serviceLocator.collectionManager.get('nodes'),
@@ -118,6 +120,27 @@ const NodePanel: React.FunctionComponent<WithTranslation> = (props: WithTranslat
         }
     };
 
+    const deleteAllUnusedNodes = async (e) => {
+        if (e && typeof e.stopPropagation === 'function') {
+            e.stopPropagation();
+        }
+        serviceLocator.eventManager.emit('progress', { name: 'DeletingNodes', progress: 0.0 });
+
+        try {
+            await deleteUnusedNodes();
+            // FIXME This should be somehow automatic, we should not need to do those 2 calls
+            serviceLocator.collectionManager.refresh('nodes');
+            serviceLocator.eventManager.emit('map.updateLayers', {
+                transitNodes: serviceLocator.collectionManager.get('nodes').toGeojson()
+            });
+        } catch (error) {
+            // TODO Log errors
+            console.log('Error deleting unused nodes', error);
+        } finally {
+            serviceLocator.eventManager.emit('progress', { name: 'DeletingNodes', progress: 1.0 });
+        }
+    };
+
     // TODO Review the conditions to define which part is opened. This is a bit complicated wrt the state. Can there really be both selectedNode and selectedNodes?
     return (
         <div id="tr__form-transit-nodes-panel" className="tr__form-transit-nodes-panel tr__panel">
@@ -196,6 +219,23 @@ const NodePanel: React.FunctionComponent<WithTranslation> = (props: WithTranslat
                 </div>
             )}
 
+            {!state.selectedNode &&
+                !state.selectedNodes &&
+                !state.selectedStation &&
+                !importerSelected &&
+                state.nodeCollection &&
+                state.nodeCollection.size() > 0 && (
+                <div className="tr__form-buttons-container">
+                    <Button
+                        color="red"
+                        icon={faCheck}
+                        iconClass="_icon"
+                        label={props.t('transit:transitNode:DeleteAllUnusedNodes')}
+                        onClick={() => setConfirmDeleteModalIsOpened(true)}
+                    />
+                </div>
+            )}
+
             {!state.selectedNode && !state.selectedNodes && !state.selectedStation && !importerSelected && (
                 <CollectionSaveToCacheButtons collection={state.nodeCollection} labelPrefix={'transit:transitNode'} />
             )}
@@ -204,6 +244,17 @@ const NodePanel: React.FunctionComponent<WithTranslation> = (props: WithTranslat
                 <CollectionDownloadButtons collection={state.nodeCollection} />
             )}
 
+            {confirmDeleteModalIsOpened && (
+                <ConfirmModal
+                    title={props.t('transit:transitNode:ConfirmAllDelete')}
+                    confirmAction={deleteAllUnusedNodes}
+                    isOpen={true}
+                    confirmButtonColor="red"
+                    confirmButtonLabel={props.t('transit:transitNode:MultipleDelete')}
+                    closeModal={() => setConfirmDeleteModalIsOpened(false)}
+                />
+            )}
+
             {/* disabled for now because it is too slow and unreliable (valhalla problem with some snodes) */}
             {/*!nodeSelected && !stationSelected && this.state.nodeCollection.size() > 0 && <div className="tr__form-buttons-container">
           <Button color="green" icon={faCheck} iconClass="_icon" label={this.props.t('transit:transitNode:UpdateNodesWalkingAccessibilityMap')} action={function() {

packages/transition-frontend/src/components/map/TransitionMainMap.tsx

@@ -28,6 +28,7 @@ import _cloneDeep from 'lodash/cloneDeep';
 import { featureCollection as turfFeatureCollection } from '@turf/turf';
 import { LayoutSectionProps } from 'chaire-lib-frontend/lib/services/dashboard/DashboardContribution';
 import { MapEventHandlerDescription } from 'chaire-lib-frontend/lib/services/map/IMapEventHandler';
+import { deleteUnusedNodes } from '../../services/transitNodes/transitNodesUtils';
 
 MapboxGL.accessToken = process.env.MAPBOX_ACCESS_TOKEN || '';
 
@@ -418,23 +419,26 @@ class MainMap extends React.Component<MainMapProps, MainMapState> {
     };
 
     onDeleteSelectedNodes = () => {
-        serviceLocator.eventManager.emit('progress', { name: 'DeletingNode', progress: 0.0 });
+        serviceLocator.eventManager.emit('progress', { name: 'DeletingNodes', progress: 0.0 });
         const selectedNodes = serviceLocator.selectedObjectsManager.get('selectedNodes');
 
-        Promise.all(
-            selectedNodes.map((node: Node) => {
-                return node.delete(serviceLocator.socketEventManager);
+        deleteUnusedNodes(selectedNodes.map((n) => n.getId()))
+            .then((response) => {
+                serviceLocator.selectedObjectsManager.deselect('node');
+                serviceLocator.collectionManager.refresh('nodes');
+                serviceLocator.eventManager.emit('map.updateLayers', {
+                    transitNodes: serviceLocator.collectionManager.get('nodes').toGeojson(),
+                    transitNodesSelected: turfFeatureCollection([])
+                });
             })
-        ).then((response) => {
-            serviceLocator.selectedObjectsManager.deselect('node');
-            serviceLocator.collectionManager.refresh('nodes');
-            serviceLocator.eventManager.emit('map.updateLayers', {
-                transitNodes: serviceLocator.collectionManager.get('nodes').toGeojson(),
-                transitNodesSelected: turfFeatureCollection([])
+            .catch((error) => {
+                // TODO Log errors
+                console.log('Error deleting unused nodes', error);
+            })
+            .finally(() => {
+                this.deleteSelectedPolygon();
+                serviceLocator.eventManager.emit('progress', { name: 'DeletingNodes', progress: 1.0 });
             });
-        });
-        this.deleteSelectedPolygon();
-        serviceLocator.eventManager.emit('progress', { name: 'DeletingNode', progress: 1.0 });
     };
 
     handleDrawControl = (section: string) => {