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
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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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