From 02124307e4c79580cced0544c2a47d9fc35a397a Mon Sep 17 00:00:00 2001
From: Roland <roland@learningselfreliance.com>
Date: Sat, 12 Nov 2022 14:17:21 -0700
Subject: [PATCH] Downloads can be deleted.

---
 app/src/api.js                          | 118 ++++++++----------------
 app/src/components/admin/Downloads.js   |  53 +++++++++--
 modules/map/lib.py                      |   2 -
 modules/videos/channel/test/test_api.py |   3 +-
 wrolpi/downloader.py                    |   2 +
 wrolpi/root_api.py                      |   2 +-
 wrolpi/test/test_downloader.py          |   8 ++
 7 files changed, 100 insertions(+), 88 deletions(-)

diff --git a/app/src/api.js b/app/src/api.js
index 74f7c61f..efbf2fac 100644
--- a/app/src/api.js
+++ b/app/src/api.js
@@ -8,16 +8,13 @@ function timeoutPromise(ms, promise) {
         const timeoutId = setTimeout(() => {
             reject(new Error("promise timeout"))
         }, ms);
-        promise.then(
-            (res) => {
-                clearTimeout(timeoutId);
-                resolve(res);
-            },
-            (err) => {
-                clearTimeout(timeoutId);
-                reject(err);
-            }
-        );
+        promise.then((res) => {
+            clearTimeout(timeoutId);
+            resolve(res);
+        }, (err) => {
+            clearTimeout(timeoutId);
+            reject(err);
+        });
     })
 }
 
@@ -60,10 +57,7 @@ async function apiCall(url, method, body, ms = 20000) {
             });
         } else {
             toast({
-                type: 'error',
-                title: 'Error!',
-                description: 'See client logs',
-                time: 5000,
+                type: 'error', title: 'Error!', description: 'See client logs', time: 5000,
             });
         }
         throw e;
@@ -184,10 +178,7 @@ export async function getStatus() {
         return await response.json();
     } else {
         toast({
-            type: 'error',
-            title: 'Unexpected server response',
-            description: 'Could not get server status',
-            time: 5000,
+            type: 'error', title: 'Unexpected server response', description: 'Could not get server status', time: 5000,
         });
     }
 }
@@ -198,10 +189,7 @@ export async function getSettings() {
         return await response.json();
     } else {
         toast({
-            type: 'error',
-            title: 'Unexpected server response',
-            description: 'Could not get settings',
-            time: 5000,
+            type: 'error', title: 'Unexpected server response', description: 'Could not get settings', time: 5000,
         });
     }
 }
@@ -216,10 +204,7 @@ export async function getDownloads() {
         return await response.json();
     } else {
         toast({
-            type: 'error',
-            title: 'Unexpected server response',
-            description: 'Could not get downloads',
-            time: 5000,
+            type: 'error', title: 'Unexpected server response', description: 'Could not get downloads', time: 5000,
         });
     }
 }
@@ -241,10 +226,7 @@ export async function getStatistics() {
         return (await response.json())['statistics'];
     } else {
         toast({
-            type: 'error',
-            title: 'Unexpected server response',
-            description: 'Could not get statistics',
-            time: 5000,
+            type: 'error', title: 'Unexpected server response', description: 'Could not get statistics', time: 5000,
         });
     }
 }
@@ -275,10 +257,7 @@ export async function encryptOTP(otp, plaintext) {
         return await response.json();
     } else {
         toast({
-            type: 'error',
-            title: 'Error!',
-            description: 'Failed to encrypt OTP',
-            time: 5000,
+            type: 'error', title: 'Error!', description: 'Failed to encrypt OTP', time: 5000,
         });
     }
 }
@@ -290,10 +269,7 @@ export async function decryptOTP(otp, ciphertext) {
         return await response.json();
     } else {
         toast({
-            type: 'error',
-            title: 'Error!',
-            description: 'Failed to decrypt OTP',
-            time: 5000,
+            type: 'error', title: 'Error!', description: 'Failed to decrypt OTP', time: 5000,
         });
     }
 }
@@ -304,10 +280,7 @@ export async function getCategories() {
         return (await response.json())['categories'];
     } else {
         toast({
-            type: 'error',
-            title: 'Unexpected server response',
-            description: 'Could not get categories',
-            time: 5000,
+            type: 'error', title: 'Unexpected server response', description: 'Could not get categories', time: 5000,
         });
     }
 }
@@ -318,10 +291,7 @@ export async function getBrands() {
         return (await response.json())['brands'];
     } else {
         toast({
-            type: 'error',
-            title: 'Unexpected server response',
-            description: 'Could not get brands',
-            time: 5000,
+            type: 'error', title: 'Unexpected server response', description: 'Could not get brands', time: 5000,
         });
     }
 }
@@ -332,10 +302,7 @@ export async function getInventories() {
         return (await response.json())['inventories'];
     } else {
         toast({
-            type: 'error',
-            title: 'Unexpected server response',
-            description: 'Could not get inventories',
-            time: 5000,
+            type: 'error', title: 'Unexpected server response', description: 'Could not get inventories', time: 5000,
         });
     }
 }
@@ -346,10 +313,7 @@ export async function getInventory(inventoryId) {
         return await response.json();
     } else {
         toast({
-            type: 'error',
-            title: 'Unexpected server response',
-            description: 'Could not get inventory',
-            time: 5000,
+            type: 'error', title: 'Unexpected server response', description: 'Could not get inventory', time: 5000,
         });
     }
 }
@@ -393,10 +357,7 @@ export async function deleteArchives(archiveIds) {
     } catch (e) {
         console.error(e);
         toast({
-            type: 'error',
-            title: 'Unexpected server response',
-            description: 'Unable to delete archives',
-            time: 5000,
+            type: 'error', title: 'Unexpected server response', description: 'Unable to delete archives', time: 5000,
         });
     }
 }
@@ -453,10 +414,7 @@ export async function getArchive(archiveId) {
         return [data['file'], data['alternatives']];
     } else {
         toast({
-            type: 'error',
-            title: 'Archive Error',
-            description: 'Unable to get Archive.  See server logs.',
-            time: 5000,
+            type: 'error', title: 'Archive Error', description: 'Unable to get Archive.  See server logs.', time: 5000,
         });
     }
 }
@@ -500,6 +458,22 @@ export async function getDownloaders() {
     }
 }
 
+export async function deleteDownload(downloadId) {
+    try {
+        let response = await apiDelete(`${API_URI}/download/${downloadId}`);
+        if (response.status !== 204) {
+            toast({
+                type: 'error',
+                title: 'Unable to delete',
+                description: 'Unable to delete the download.  See server logs.',
+                time: 5000,
+            });
+        }
+    } catch (e) {
+        return null;
+    }
+}
+
 const replaceFileDatetimes = (files) => {
     for (let i = 0; i < files.length; i++) {
         let file = files[i];
@@ -595,10 +569,7 @@ export async function setHotspot(on) {
             });
         } else {
             toast({
-                type: 'error',
-                title: 'Error!',
-                description: 'Could not modify hotspot.  See server logs.',
-                time: 5000,
+                type: 'error', title: 'Error!', description: 'Could not modify hotspot.  See server logs.', time: 5000,
             });
         }
     }
@@ -629,10 +600,7 @@ export async function setThrottle(on) {
             });
         } else {
             toast({
-                type: 'error',
-                title: 'Error!',
-                description: 'Could not modify throttle.  See server logs.',
-                time: 5000,
+                type: 'error', title: 'Error!', description: 'Could not modify throttle.  See server logs.', time: 5000,
             });
         }
     }
@@ -644,10 +612,7 @@ export async function getMapImportStatus() {
         return await response.json();
     } else {
         toast({
-            type: 'error',
-            title: 'Unexpected server response',
-            description: 'Could not get import status',
-            time: 5000,
+            type: 'error', title: 'Unexpected server response', description: 'Could not get import status', time: 5000,
         });
     }
 }
@@ -659,10 +624,7 @@ export async function importMapFiles(paths) {
         return null;
     } else {
         toast({
-            type: 'error',
-            title: 'Error!',
-            description: 'Could not start import!  See server logs.',
-            time: 5000,
+            type: 'error', title: 'Error!', description: 'Could not start import!  See server logs.', time: 5000,
         });
     }
 }
diff --git a/app/src/components/admin/Downloads.js b/app/src/components/admin/Downloads.js
index 3372bd89..d11b244c 100644
--- a/app/src/components/admin/Downloads.js
+++ b/app/src/components/admin/Downloads.js
@@ -1,8 +1,9 @@
 import React, {useContext} from "react";
-import {clearCompletedDownloads, clearFailedDownloads, killDownload, postDownload} from "../../api";
+import {clearCompletedDownloads, clearFailedDownloads, deleteDownload, killDownload, postDownload} from "../../api";
 import {Link} from "react-router-dom";
 import {
-    DisableDownloadsToggle, isEmpty,
+    DisableDownloadsToggle,
+    isEmpty,
     secondsToElapsedPopup,
     secondsToFrequency,
     textEllipsis,
@@ -87,13 +88,39 @@ class DownloadRow extends React.Component {
         super(props);
         this.state = {
             errorModalOpen: false,
+            deleteOpen: false,
         }
     }
 
+    openDelete = (e) => {
+        if (e) {
+            e.preventDefault();
+        }
+        this.setState({deleteOpen: true});
+    }
+
+    closeDelete = (e) => {
+        if (e) {
+            e.preventDefault();
+        }
+        this.setState({deleteOpen: false});
+    }
+
+    handleDelete = async (e) => {
+        if (e) {
+            e.preventDefault();
+        }
+        const {id} = this.props;
+        try {
+            await deleteDownload(id);
+        } finally {
+            this.closeDelete();
+        }
+    }
 
     render() {
         let {url, frequency, last_successful_download, status, location, next_download, error} = this.props;
-        const {errorModalOpen} = this.state;
+        const {errorModalOpen, deleteOpen} = this.state;
 
         const link = location ?
             (text) => <Link to={location}>{text}</Link> :
@@ -104,7 +131,7 @@ class DownloadRow extends React.Component {
             onClose={() => this.setState({errorModalOpen: false})}
             onOpen={() => this.setState({errorModalOpen: true})}
             open={errorModalOpen}
-            trigger={<Button icon='exclamation circle' color='red'/>}
+            trigger={<Button icon='exclamation circle' color='orange'/>}
         >
             <Modal.Header>Download Error</Modal.Header>
             <Modal.Content>
@@ -115,6 +142,17 @@ class DownloadRow extends React.Component {
             </Modal.Actions>
         </Modal>;
 
+        const deleteButton = <>
+            <Button icon='trash' onClick={this.openDelete} color='red'/>
+            <Confirm
+                open={deleteOpen}
+                content='Are you sure you want to delete this download?'
+                confirmButton='Delete'
+                onCancel={this.closeDelete}
+                onConfirm={this.handleDelete}
+            />
+        </>;
+
         return <TableRow>
             <TableCell>
                 {link(textEllipsis(url, 50))}
@@ -125,7 +163,10 @@ class DownloadRow extends React.Component {
                 {status === 'pending' ? <Loader active inline size='tiny'/> : null}
             </TableCell>
             <TableCell>{secondsToElapsedPopup(next_download)}</TableCell>
-            <TableCell>{error && errorModal}{link('View')}</TableCell>
+            <TableCell>
+                {error && errorModal}
+                {deleteButton}
+            </TableCell>
         </TableRow>
     }
 }
@@ -295,7 +336,7 @@ export function Downloads() {
                 <TableHeaderCell>Download Frequency</TableHeaderCell>
                 <TableHeaderCell>Completed At</TableHeaderCell>
                 <TableHeaderCell>Next</TableHeaderCell>
-                <TableHeaderCell>View</TableHeaderCell>
+                <TableHeaderCell>Control</TableHeaderCell>
             </TableRow>
         </TableHeader>
     );
diff --git a/modules/map/lib.py b/modules/map/lib.py
index 445cecef..ebbbd2f9 100644
--- a/modules/map/lib.py
+++ b/modules/map/lib.py
@@ -79,8 +79,6 @@ def get_or_create_map_file(pbf_path: Path, session: Session) -> MapFile:
 
 import_logger = logger.getChild('import')
 
-OSMIUM_BIN = which('osmium', '/usr/bin/osmium')
-
 
 async def import_files(paths: List[str]):
     if IMPORT_EVENT.is_set():
diff --git a/modules/videos/channel/test/test_api.py b/modules/videos/channel/test/test_api.py
index b2b637dc..10fe963e 100644
--- a/modules/videos/channel/test/test_api.py
+++ b/modules/videos/channel/test/test_api.py
@@ -110,7 +110,8 @@ def test_channel_frequency_update(download_channel, test_client, test_session, t
     assert len(list(downloads)) == 1
 
 
-def test_channel_download_crud(test_session, simple_channel):
+@pytest.mark.asyncio
+async def test_channel_download_crud(test_session, simple_channel, test_downloader_config):
     """Modifying a Channel modifies it's Download."""
     assert simple_channel.url
     assert simple_channel.download_frequency is None
diff --git a/wrolpi/downloader.py b/wrolpi/downloader.py
index 913b9487..10501c0b 100644
--- a/wrolpi/downloader.py
+++ b/wrolpi/downloader.py
@@ -785,6 +785,8 @@ class DownloadManager:
         if download:
             session.delete(download)
             session.commit()
+            # Save the config because a download was deleted.
+            background_task(save_downloads_config())
             return True
         return False
 
diff --git a/wrolpi/root_api.py b/wrolpi/root_api.py
index 7975ec06..27f4f805 100644
--- a/wrolpi/root_api.py
+++ b/wrolpi/root_api.py
@@ -275,7 +275,7 @@ async def clear_failed(_: Request):
     return response.empty()
 
 
-@api_bp.delete('/download/<download_id:[0-9]+>')
+@api_bp.delete('/download/<download_id:[0-9,]+>')
 @openapi.description('Delete a download')
 @wrol_mode_check
 async def delete_download(_: Request, download_id: int):
diff --git a/wrolpi/test/test_downloader.py b/wrolpi/test/test_downloader.py
index 9b200461..0cfcc9f7 100644
--- a/wrolpi/test/test_downloader.py
+++ b/wrolpi/test/test_downloader.py
@@ -394,10 +394,18 @@ def test_crud_download(test_client, test_session, test_download_manager):
     assert 'recurring_downloads' in response.json
     assert [i['url'] for i in response.json['recurring_downloads']] == ['https://example.com']
 
+    request, response = test_client.delete('/api/download/123')
+    assert response.status_code == HTTPStatus.NOT_FOUND
+    assert test_session.query(Download).count() == 1
+
     request, response = test_client.delete(f'/api/download/{download.id}')
     assert response.status_code == HTTPStatus.NO_CONTENT
     assert test_session.query(Download).count() == 0
 
+    request, response = test_client.delete(f'/api/download/{download.id}')
+    assert response.status_code == HTTPStatus.NOT_FOUND
+    assert test_session.query(Download).count() == 0
+
 
 @pytest.mark.asyncio
 async def test_downloads_config(test_client, test_session, test_download_manager, test_download_manager_config):