MarketSquare / MarketSquare/robotframework-dashboard

Clicking the "outputs to remove by limit" number-input spinner arrows deletes runs immediately — no button press, no confirmation, and down-arrow deletes ALL outputs

Open
#333 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
HTML
Stars
87
Forks
21
Avg merge
14h 55m
Merged PRs (30d)
13

Description

**Component:** Admin page (`/admin`) → "Remove output.xml(s) From Database" → outputs to remove by limit

**Severity:** Critical (irreversible data loss triggered by a single accidental click, no confirmation)

**Environment:** robotframework-dashboard v2.2.0, served via Docker, `http://localhost:8080/admin`

### Root Cause
In `templates/admin.html`, the "outputs to remove by limit" control is a plain ``; the actual "Remove" button is `#removeOutputs`. However, in `js/admin_page/admin_eventlisteners.js`:
```js
document.getElementById("removeOutputs").addEventListener("click", remove_outputs);
document.getElementById("removeLimit").addEventListener("click", remove_outputs);
```
A `click` listener calling `remove_outputs()` is attached directly to the **number input**, not just to the Remove button. Clicking anywhere on that input — including its native up/down spinner arrows — fires a full `DELETE /remove-outputs` request using whatever value the click left in the field, with no explicit "Remove" click required and no `confirm_action()` step (unlike "Remove All Outputs", which does confirm).

Because clicking the spinner arrows also changes the field's value as part of the same click:
- **Up arrow** on an empty field → value becomes `1` → request sent with `limit=1` → keeps only the single newest run, deletes everything else.
- **Down arrow** on an empty field → value becomes `-1` → request sent with `limit=-1` → `_remove_by_limit`'s guard `if limit >= len(candidates)` never trips for a negative number, and the removal slice `candidates[: len(candidates) - limit]` evaluates to `candidates[: len(candidates) + 1]`, i.e. the entire list → **deletes every run in the database**.

### Steps to Reproduce
1. Open `/admin` with existing runs in the database.
2. Without typing anything or clicking any "Remove" button, click the **down arrow** of the "outputs to remove by limit" spin-input.
3. Observe: all runs are deleted immediately, response `... limit=-1 ...`, no confirmation prompt shown.
- (Clicking the up arrow instead leaves exactly one run.)

### Actual Behavior
- A single click on the input's spinner arrows (not the "Remove" button) is sufficient to trigger deletion.
- Negative `limit` values are accepted and cause full-database deletion via slice underflow logic in `_remove_by_limit`.
- No confirmation dialog is shown for this destructive action.

### Expected Behavior
- The `click` handler should be attached only to the `#removeOutputs` button, not to the `#removeLimit` input — interacting with the input (typing or spinner arrows) must never itself submit the removal request.
- `limit` should be validated (client- and server-side) to reject values `<= 0`, instead of silently deleting all runs.
- Destructive removals (especially anything that can remove all/nearly-all runs) should go through the same `confirm_action()` confirmation used by "Remove All Outputs".

### Suggested Fix
1. In `admin_eventlisteners.js`, remove the listener bound to `#removeLimit` (the input) — only `#removeOutputs` (the button) should call `remove_outputs()`.
2. Add `min="1"` to the `#removeLimit` input, and validate `limit >= 1` in `_remove_by_limit()` server-side (reject/warn on `limit <= 0` instead of proceeding).
3. Add a confirmation step in `remove_outputs()` before submitting, at least when the resulting operation would remove all or nearly all runs.

### Relevant Code
- `templates/admin.html` (lines ~246–260) — `#removeLimit` is the input, `#removeOutputs` is the button
- `js/admin_page/admin_eventlisteners.js` — `click` listener incorrectly attached to `#removeLimit`
- `database.py` — `_remove_by_limit()` — no guard for `limit <= 0`

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in js/admin_page/admin_eventlisteners.js and templates/admin.html to trace the #removeOutputs and #removeLimit handlers, then inspect database.py::_remove_by_limit for negative-limit behavior. Done means input interaction alone sends no deletion request, submitted limits reject values <=0, and destructive removal uses confirmation without changing the intended button flow.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, python
Domain
backend, database, frontend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.