Improved DroneDB plugin #1801

HeDo88TH · 2025-12-11T07:30:07Z

Fixed DroneDB integration
Added support to specify org/dataset when publishing

pierotofy · 2025-12-11T16:49:47Z

Nice, is this ready or still a draft?

HeDo88TH · 2025-12-11T16:52:44Z

Nice, is this ready or still a draft?

I have tested it but I need to do more thorough testing before marking it as ready

Copilot

Pull request overview

This PR enhances the DroneDB plugin by fixing integration issues and adding the ability to specify organization and dataset when sharing data to DroneDB. The changes introduce a new ShareDialog component that provides users with two sharing modes: a quick share option that auto-generates destinations, and a manual selection mode that allows choosing specific organizations and datasets.

Key Changes:

Added a new ShareDialog component with organization/dataset selection UI
Updated the share API to accept optional 'tag' and 'datasetName' parameters
Improved code formatting and replaced loose equality operators with strict equality

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
coreplugins/dronedb/public/components/ShareDialog.jsx	New modal dialog component for selecting share destination with organization/dataset dropdowns
coreplugins/dronedb/public/components/ShareDialog.scss	Styling for the new ShareDialog component
coreplugins/dronedb/public/ShareButton.jsx	Updated to integrate ShareDialog and pass tag/datasetName parameters; improved code quality with strict equality
coreplugins/dronedb/api_views.py	Modified share endpoint to accept and forward tag/datasetName parameters; improved formatting
coreplugins/dronedb/ddb.py	Updated share_init method to accept tag and dataset_name parameters; improved code formatting
coreplugins/dronedb/plugin.py	Updated build configuration to include new ShareDialog component and CSS; improved formatting

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-24T13:54:35Z

coreplugins/dronedb/public/components/ShareDialog.jsx

+        const dm = decimals || 2;
+        const sizes = ['bytes', 'KB', 'MB', 'GB', 'TB'];
+        const i = Math.floor(Math.log(bytes) / Math.log(k));
+        return parseFloat((bytes / Math.pow(k, i)).toFixed(dm)) + ' ' + sizes[i];


The variable 'dm' is assigned but never used in this function. You can remove this line and use 'decimals' directly on line 52.

Suggested change

const dm = decimals || 2;

const sizes = ['bytes', 'KB', 'MB', 'GB', 'TB'];

const i = Math.floor(Math.log(bytes) / Math.log(k));

return parseFloat((bytes / Math.pow(k, i)).toFixed(dm)) + ' ' + sizes[i];

const sizes = ['bytes', 'KB', 'MB', 'GB', 'TB'];

const i = Math.floor(Math.log(bytes) / Math.log(k));

return parseFloat((bytes / Math.pow(k, i)).toFixed(decimals || 2)) + ' ' + sizes[i];

Copilot · 2025-12-24T13:54:35Z

coreplugins/dronedb/public/components/ShareDialog.jsx

+        // Load user info
+        $.get(`${this.props.apiURL}/info`)
+            .done(result => {
+                this.setState({ info: result });
+            })
+            .fail(() => {
+                this.setState({ info: null });
+            });
+
+        // Load organizations
+        $.get(`${this.props.apiURL}/organizations`)
+            .done(result => {
+                const orgs = result.map(org => ({
+                    label: org.name !== org.slug ? `${org.name} (${org.slug})` : org.slug,
+                    value: org.slug
+                }));
+
+                this.setState({ organizations: orgs, loadingOrganizations: false });
+
+                if (orgs.length > 0) {
+                    // Try to find user's personal organization
+                    const userOrg = this.state.info ?
+                        orgs.find(org => org.value === this.state.info.username) : null;
+                    this.handleSelectOrganization(userOrg || orgs[0]);
+                }
+            })
+            .fail(() => {
+                this.setState({
+                    error: "Cannot load organizations. Check your credentials.",
+                    loadingOrganizations: false


There's a potential race condition here. The 'info' state may not be set yet when trying to access it on line 84. The API call on line 64 happens asynchronously and completes after the organizations API call on line 73. Consider chaining these requests or using Promise.all to ensure 'info' is available before trying to access it.

Suggested change

// Load user info

$.get(`${this.props.apiURL}/info`)

.done(result => {

this.setState({ info: result });

})

.fail(() => {

this.setState({ info: null });

});

// Load organizations

$.get(`${this.props.apiURL}/organizations`)

.done(result => {

const orgs = result.map(org => ({

label: org.name !== org.slug ? `${org.name} (${org.slug})` : org.slug,

value: org.slug

}));

this.setState({ organizations: orgs, loadingOrganizations: false });

if (orgs.length > 0) {

// Try to find user's personal organization

const userOrg = this.state.info ?

orgs.find(org => org.value === this.state.info.username) : null;

this.handleSelectOrganization(userOrg || orgs[0]);

}

})

.fail(() => {

this.setState({

error: "Cannot load organizations. Check your credentials.",

loadingOrganizations: false

const loadOrganizations = () => {

// Load organizations

$.get(`${this.props.apiURL}/organizations`)

.done(result => {

const orgs = result.map(org => ({

label: org.name !== org.slug ? `${org.name} (${org.slug})` : org.slug,

value: org.slug

}));

this.setState({ organizations: orgs, loadingOrganizations: false });

if (orgs.length > 0) {

// Try to find user's personal organization

const userOrg = this.state.info ?

orgs.find(org => org.value === this.state.info.username) : null;

this.handleSelectOrganization(userOrg || orgs[0]);

}

})

.fail(() => {

this.setState({

error: "Cannot load organizations. Check your credentials.",

loadingOrganizations: false

});

});

};

// Load user info, then load organizations

$.get(`${this.props.apiURL}/info`)

.done(result => {

this.setState({ info: result }, () => {

loadOrganizations();

});

})

.fail(() => {

this.setState({ info: null }, () => {

loadOrganizations();

Copilot · 2025-12-24T13:54:36Z

coreplugins/dronedb/public/components/ShareDialog.jsx

+        $.get(`${this.props.apiURL}/info`)
+            .done(result => {
+                this.setState({ info: result });
+            })
+            .fail(() => {
+                this.setState({ info: null });
+            });
+
+        // Load organizations
+        $.get(`${this.props.apiURL}/organizations`)
+            .done(result => {
+                const orgs = result.map(org => ({
+                    label: org.name !== org.slug ? `${org.name} (${org.slug})` : org.slug,
+                    value: org.slug
+                }));
+
+                this.setState({ organizations: orgs, loadingOrganizations: false });
+
+                if (orgs.length > 0) {
+                    // Try to find user's personal organization
+                    const userOrg = this.state.info ?
+                        orgs.find(org => org.value === this.state.info.username) : null;
+                    this.handleSelectOrganization(userOrg || orgs[0]);
+                }
+            })
+            .fail(() => {
+                this.setState({
+                    error: "Cannot load organizations. Check your credentials.",
+                    loadingOrganizations: false
+                });
+            });


Using jQuery for API calls in a React component is considered an anti-pattern. Consider using the Fetch API or axios instead, which provide better error handling and are more modern. This would also eliminate the dependency on jQuery being available globally.

Copilot · 2025-12-24T13:54:36Z

coreplugins/dronedb/public/components/ShareDialog.jsx

+            .fail(() => {
+                this.setState({
+                    error: "Cannot load organizations. Check your credentials.",


The error messages shown to users don't provide actionable information. "Cannot load organizations" should include more context, such as whether it's a network issue, authentication problem, or server error. Consider using the actual error message from the API response.

Suggested change

.fail(() => {

this.setState({

error: "Cannot load organizations. Check your credentials.",

.fail((jqXHR, textStatus, errorThrown) => {

let errorMessage = "Cannot load organizations.";

if (jqXHR) {

if (jqXHR.status === 0) {

errorMessage += " Network error – please check your internet connection.";

} else if (jqXHR.status === 401 || jqXHR.status === 403) {

errorMessage += " Authentication failed – please check your credentials.";

} else if (jqXHR.status >= 500) {

errorMessage += " Server error – please try again later.";

} else {

errorMessage += " Unexpected error while contacting the server.";

}

if (jqXHR.responseJSON && jqXHR.responseJSON.error) {

errorMessage += " Details: " + jqXHR.responseJSON.error;

} else if (jqXHR.responseText) {

errorMessage += " Details: " + jqXHR.responseText;

} else if (errorThrown) {

errorMessage += " Details: " + errorThrown;

}

} else if (errorThrown) {

errorMessage += " Details: " + errorThrown;

}

this.setState({

error: errorMessage,

Copilot · 2025-12-24T13:54:36Z

coreplugins/dronedb/public/components/ShareDialog.jsx

+            .fail(() => {
+                this.setState({
+                    error: "Cannot load datasets.",


Similar to the organizations error, "Cannot load datasets." doesn't provide enough context for users to understand or fix the issue. Include more specific error information.

Suggested change

.fail(() => {

this.setState({

error: "Cannot load datasets.",

.fail((jqXHR, textStatus, errorThrown) => {

let errorMessage = "Cannot load datasets.";

if (textStatus) {

errorMessage += ` (${textStatus})`;

}

if (errorThrown) {

errorMessage += `: ${errorThrown}`;

}

this.setState({

error: errorMessage,

Copilot · 2025-12-24T13:54:37Z

coreplugins/dronedb/api_views.py


+        # Get optional tag and datasetName from request
+        tag = request.data.get('tag', None)
+        dataset_name = request.data.get('datasetName', None) or task.name


The 'dataset_name' parameter is used as a fallback to 'task.name' on line 333, but if both are None or empty strings, this could lead to unexpected behavior. Consider adding validation to ensure at least one valid name is provided when creating a new dataset.

Suggested change

dataset_name = request.data.get('datasetName', None) or task.name

raw_dataset_name = request.data.get('datasetName', None) or task.name

if raw_dataset_name is None or not str(raw_dataset_name).strip():

return Response(

{"detail": "A non-empty datasetName or task name is required to create a dataset."},

status=status.HTTP_400_BAD_REQUEST,

)

dataset_name = str(raw_dataset_name).strip()

Copilot · 2025-12-24T13:54:38Z

coreplugins/dronedb/public/components/ShareDialog.jsx

+export default class ShareDialog extends Component {
+    static defaultProps = {
+        show: false,
+        taskName: '',
+        filesToShare: []
+    };
+
+    static propTypes = {
+        onHide: PropTypes.func.isRequired,
+        onShare: PropTypes.func.isRequired,
+        show: PropTypes.bool.isRequired,
+        apiURL: PropTypes.string.isRequired,
+        taskName: PropTypes.string,
+        filesToShare: PropTypes.array
+    };
+
+    constructor(props) {
+        super(props);
+        this.state = this.getInitialState();
+    }
+
+    getInitialState() {
+        return {
+            error: "",
+            shareMode: SHARE_MODE_QUICK,
+            organizations: [],
+            datasets: [],
+            loadingOrganizations: false,
+            loadingDatasets: false,
+            selectedOrganization: null,
+            selectedDataset: null,
+            newDatasetName: '',
+            createNewDataset: true,
+            info: null
+        };
+    }
+
+    formatBytes(bytes, decimals = 2) {
+        if (bytes === 0) return '0 bytes';
+        const k = 1024;
+        const dm = decimals || 2;
+        const sizes = ['bytes', 'KB', 'MB', 'GB', 'TB'];
+        const i = Math.floor(Math.log(bytes) / Math.log(k));
+        return parseFloat((bytes / Math.pow(k, i)).toFixed(dm)) + ' ' + sizes[i];
+    }
+
+    handleOnShow = () => {
+        this.setState(this.getInitialState());
+        this.setState({
+            loadingOrganizations: true,
+            newDatasetName: this.props.taskName || ''
+        });
+
+        // Load user info
+        $.get(`${this.props.apiURL}/info`)
+            .done(result => {
+                this.setState({ info: result });
+            })
+            .fail(() => {
+                this.setState({ info: null });
+            });
+
+        // Load organizations
+        $.get(`${this.props.apiURL}/organizations`)
+            .done(result => {
+                const orgs = result.map(org => ({
+                    label: org.name !== org.slug ? `${org.name} (${org.slug})` : org.slug,
+                    value: org.slug
+                }));
+
+                this.setState({ organizations: orgs, loadingOrganizations: false });
+
+                if (orgs.length > 0) {
+                    // Try to find user's personal organization
+                    const userOrg = this.state.info ?
+                        orgs.find(org => org.value === this.state.info.username) : null;
+                    this.handleSelectOrganization(userOrg || orgs[0]);
+                }
+            })
+            .fail(() => {
+                this.setState({
+                    error: "Cannot load organizations. Check your credentials.",
+                    loadingOrganizations: false
+                });
+            });
+    };
+
+    handleSelectOrganization = (e) => {
+        if (!e) return;
+        if (this.state.selectedOrganization?.value === e.value) return;
+
+        this.setState({
+            selectedOrganization: e,
+            selectedDataset: null,
+            datasets: [],
+            loadingDatasets: true
+        });
+
+        $.get(`${this.props.apiURL}/organizations/${e.value}/datasets`)
+            .done(result => {
+                const datasets = result.map(ds => ({
+                    label: ds.name !== ds.slug ?
+                        `${ds.name} (${ds.slug})` : ds.slug,
+                    value: ds.slug,
+                    name: ds.name
+                }));
+
+                // Add "Create new" option at the beginning
+                datasets.unshift({
+                    label: '+ Create new dataset',
+                    value: '__new__',
+                    isNew: true
+                });
+
+                this.setState({
+                    datasets,
+                    loadingDatasets: false,
+                    selectedDataset: datasets[0],
+                    createNewDataset: true
+                });
+            })
+            .fail(() => {
+                this.setState({
+                    error: "Cannot load datasets.",
+                    loadingDatasets: false
+                });
+            });
+    };
+
+    handleSelectDataset = (e) => {
+        if (!e) return;
+
+        const createNewDataset = e.value === '__new__';
+        this.setState({
+            selectedDataset: e,
+            createNewDataset
+        });
+    };
+
+    handleModeChange = (mode) => {
+        this.setState({ shareMode: mode });
+    };
+
+    handleNewDatasetNameChange = (e) => {
+        this.setState({ newDatasetName: e.target.value });
+    };
+
+    handleSubmit = () => {
+        const { shareMode, selectedOrganization, selectedDataset, newDatasetName, createNewDataset } = this.state;
+
+        let tag = null;
+        let datasetName = null;
+
+        if (shareMode === SHARE_MODE_SELECT && selectedOrganization) {
+            if (createNewDataset) {
+                // Will create new dataset in selected org
+                // tag format: "org/" means create new dataset in org
+                tag = selectedOrganization.value;
+                datasetName = newDatasetName || this.props.taskName || null;
+            } else if (selectedDataset && selectedDataset.value !== '__new__') {
+                // Use existing dataset
+                tag = `${selectedOrganization.value}/${selectedDataset.value}`;
+            }
+        }
+        // If SHARE_MODE_QUICK, tag remains null (backend creates personal org + random dataset)
+
+        this.props.onShare({ tag, datasetName });
+    };
+
+    getTotalSize() {
+        return this.props.filesToShare.reduce((sum, f) => sum + (f.size || 0), 0);
+    }
+
+    render() {
+        const { onHide, show, filesToShare } = this.props;
+        const {
+            shareMode,
+            organizations,
+            datasets,
+            loadingOrganizations,
+            loadingDatasets,
+            selectedOrganization,
+            selectedDataset,
+            createNewDataset,
+            newDatasetName,
+            error
+        } = this.state;
+
+        const canShare = shareMode === SHARE_MODE_QUICK ||
+            (shareMode === SHARE_MODE_SELECT && selectedOrganization &&
+                (createNewDataset || (selectedDataset && selectedDataset.value !== '__new__')));
+
+        return (
+            <Modal className="share-dialog" onHide={onHide} show={show} onShow={this.handleOnShow}>
+                <Modal.Header closeButton>
+                    <Modal.Title>
+                        <i className="ddb-icon fa-fw"></i> Share to DroneDB
+                    </Modal.Title>
+                </Modal.Header>
+                <Modal.Body>
+                    {error && (
+                        <div className="alert alert-warning">
+                            {error}
+                        </div>
+                    )}
+
+                    <FormGroup>
+                        <div className="share-mode-option">
+                            <Radio
+                                name="shareMode"
+                                checked={shareMode === SHARE_MODE_QUICK}
+                                onChange={() => this.handleModeChange(SHARE_MODE_QUICK)}
+                            >
+                                <strong>Quick share</strong>
+                                <div className="help-text">
+                                    Creates a new dataset with auto-generated name in your personal space
+                                </div>
+                            </Radio>
+                        </div>
+                        <div className="share-mode-option">
+                            <Radio
+                                name="shareMode"
+                                checked={shareMode === SHARE_MODE_SELECT}
+                                onChange={() => this.handleModeChange(SHARE_MODE_SELECT)}
+                                disabled={organizations.length === 0 && !loadingOrganizations}
+                            >
+                                <strong>Choose destination</strong>
+                                <div className="help-text">
+                                    Select organization and dataset
+                                </div>
+                            </Radio>
+                        </div>
+                    </FormGroup>
+
+                    {shareMode === SHARE_MODE_SELECT && (
+                        <div className="destination-select">
+                            <FormGroup>
+                                <label>Organization</label>
+                                <Select
+                                    className="basic-single"
+                                    classNamePrefix="select"
+                                    isLoading={loadingOrganizations}
+                                    isClearable={false}
+                                    isSearchable={true}
+                                    value={selectedOrganization}
+                                    onChange={this.handleSelectOrganization}
+                                    options={organizations}
+                                    placeholder={loadingOrganizations ? "Loading..." : "Select organization"}
+                                />
+                            </FormGroup>
+
+                            <FormGroup>
+                                <label>Dataset</label>
+                                <Select
+                                    className="basic-single"
+                                    classNamePrefix="select"
+                                    isLoading={loadingDatasets}
+                                    isClearable={false}
+                                    isSearchable={true}
+                                    value={selectedDataset}
+                                    onChange={this.handleSelectDataset}
+                                    options={datasets}
+                                    isDisabled={!selectedOrganization}
+                                    placeholder={loadingDatasets ? "Loading..." : "Select dataset"}
+                                />
+                            </FormGroup>
+
+                            {createNewDataset && (
+                                <FormGroup>
+                                    <label>New dataset name <small>(optional)</small></label>
+                                    <FormControl
+                                        type="text"
+                                        placeholder="Leave empty for auto-generated name"
+                                        value={newDatasetName}
+                                        onChange={this.handleNewDatasetNameChange}
+                                    />
+                                </FormGroup>
+                            )}
+                        </div>
+                    )}
+
+                    <div className="files-summary">
+                        <h5>Files to share</h5>
+                        <div className="files-info">
+                            <span className="badge">{filesToShare.length} files</span>
+                            <span className="badge">{this.formatBytes(this.getTotalSize())}</span>
+                        </div>
+                        <ul className="files-list">
+                            {filesToShare.slice(0, 5).map((f, i) => (
+                                <li key={i}><i className="fa fa-file"></i> {f.name}</li>
+                            ))}
+                            {filesToShare.length > 5 && (
+                                <li className="more">...and {filesToShare.length - 5} more</li>
+                            )}
+                        </ul>
+                    </div>
+                </Modal.Body>
+                <Modal.Footer>
+                    <Button onClick={onHide}>Cancel</Button>
+                    <Button
+                        bsStyle="primary"
+                        onClick={this.handleSubmit}
+                        disabled={!canShare}
+                    >
+                        <i className="fa fa-cloud-upload-alt"></i> Share
+                    </Button>
+                </Modal.Footer>
+            </Modal>
+        );
+    }
+}


The ShareDialog component doesn't implement componentWillUnmount to clean up any pending AJAX requests. If the dialog is closed while API calls are in progress, the callbacks will still execute and attempt to call setState on an unmounted component, leading to React warnings and potential memory leaks.

Copilot · 2025-12-24T13:54:38Z

coreplugins/dronedb/public/ShareButton.jsx

+            this.setState({
+                error: error.responseJSON?.error || 'Failed to start sharing',
+                taskInfo: { ...this.state.taskInfo, status: STATE_ERROR }
+            });


Component state update uses potentially inconsistent value.

Suggested change

this.setState({

error: error.responseJSON?.error || 'Failed to start sharing',

taskInfo: { ...this.state.taskInfo, status: STATE_ERROR }

});

this.setState(prevState => ({

error: error.responseJSON?.error || 'Failed to start sharing',

taskInfo: { ...(prevState.taskInfo || {}), status: STATE_ERROR }

}));

Copilot · 2025-12-24T13:54:38Z

coreplugins/dronedb/public/components/ShareDialog.jsx

+        this.setState({
+            loadingOrganizations: true,
+            newDatasetName: this.props.taskName || ''
+        });


Component state update uses potentially inconsistent value.

Copilot · 2025-12-24T13:54:38Z

coreplugins/dronedb/public/ShareButton.jsx

 ];

-export default class ShareButton extends React.Component{
+export default class ShareButton extends React.Component {


Component state property 'monitorTimeout' is written, but it is never read.

Improved dronedb plugin

7acc921

HeDo88TH changed the title ~~Improved dronedb plugin~~ Improved DroneDB plugin Dec 11, 2025

HeDo88TH mentioned this pull request Dec 16, 2025

Various improvements DroneDB/Registry#319

Merged

HeDo88TH marked this pull request as ready for review December 24, 2025 13:49

Copilot AI review requested due to automatic review settings December 24, 2025 13:49

Copilot started reviewing on behalf of HeDo88TH December 24, 2025 13:49 View session

Copilot AI reviewed Dec 24, 2025

View reviewed changes

-            .fail(() => {
-                this.setState({
-                    error: "Cannot load organizations. Check your credentials.",
+            .fail((jqXHR, textStatus, errorThrown) => {
+                let errorMessage = "Cannot load organizations.";
+                if (jqXHR) {
+                    if (jqXHR.status === 0) {
+                        errorMessage += " Network error – please check your internet connection.";
+                    } else if (jqXHR.status === 401 || jqXHR.status === 403) {
+                        errorMessage += " Authentication failed – please check your credentials.";
+                    } else if (jqXHR.status >= 500) {
+                        errorMessage += " Server error – please try again later.";
+                    } else {
+                        errorMessage += " Unexpected error while contacting the server.";
+                    }
+                    if (jqXHR.responseJSON && jqXHR.responseJSON.error) {
+                        errorMessage += " Details: " + jqXHR.responseJSON.error;
+                    } else if (jqXHR.responseText) {
+                        errorMessage += " Details: " + jqXHR.responseText;
+                    } else if (errorThrown) {
+                        errorMessage += " Details: " + errorThrown;
+                    }
+                } else if (errorThrown) {
+                    errorMessage += " Details: " + errorThrown;
+                }
+                this.setState({
+                    error: errorMessage,

-        dataset_name = request.data.get('datasetName', None) or task.name
+        raw_dataset_name = request.data.get('datasetName', None) or task.name
+        if raw_dataset_name is None or not str(raw_dataset_name).strip():
+            return Response(
+                {"detail": "A non-empty datasetName or task name is required to create a dataset."},
+                status=status.HTTP_400_BAD_REQUEST,
+            )
+        dataset_name = str(raw_dataset_name).strip()

Uh oh!

Improved DroneDB plugin #1801

Are you sure you want to change the base?

Improved DroneDB plugin #1801

Conversation

HeDo88TH commented Dec 11, 2025

Uh oh!

pierotofy commented Dec 11, 2025

Uh oh!

HeDo88TH commented Dec 11, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants