Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Django admin workflow for multitenant batch-command execution. The workflow supports target selection, review, exclusions, validation recovery, pagination, filtering, skipped-device display, and real-time WebSocket monitoring. The change also adds model helpers, handlers, routing, menu wiring, widgets, filters, documentation, and automated coverage. Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Admin
participant BatchCommandAdmin
participant BatchCommand
participant BatchCommandConsumer
participant AdminPage
Admin->>BatchCommandAdmin: configure and confirm batch command
BatchCommandAdmin->>BatchCommand: execute selected devices
BatchCommand->>BatchCommandConsumer: publish progress updates
BatchCommandConsumer-->>AdminPage: send batch state
AdminPage->>AdminPage: update rows and pagination
Merge Risk: 🟠 High · up to The batch workflow can publish premature completion, show incomplete filtered results, and retain abandoned password-change secrets. These issues should be fixed before merge. Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description documents the workflow, linked issue, testing, documentation, screenshots, and implementation details. It omits the Anti AI Spam Policy checklist item, but the description is otherwise substantially complete. Full details: Linked Issues checkExplanation The changes address issue Full details: Out of Scope Changes checkExplanation The code, templates, frontend assets, handlers, migrations, tests, and documentation changes directly support the mass-command admin workflow and real-time monitoring objectives. No unrelated changes are evident. Full details: Ui Changes, Regression Test, DocsExplanation The PR adds end-user UI through new admin templates, CSS, JavaScript, and batch-command admin views. The description includes two recordings under “Screenshots,” but both show the new mass-command workflow and monitoring. It does not include a before-and-after comparison. The regression-test requirement is satisfied by the added admin, Selenium, consumer, API, model, and task tests. The documentation requirement is satisfied by updates to the mass-command, REST API, WebSocket API, and related documentation. Comment |
dee077
force-pushed
the
feature/1344-mass-command-execution-pipeline
branch
3 times, most recently
from
July 3, 2026 23:58
989477b to
7c80797
Compare
dee077
force-pushed
the
feature/1345-mass-command-admin-workflow
branch
from
July 3, 2026 23:59
dd0c0c1 to
432952e
Compare
dee077
marked this pull request as ready for review
July 6, 2026 21:21
dee077
force-pushed
the
feature/1344-mass-command-execution-pipeline
branch
2 times, most recently
from
July 11, 2026 21:54
c1afb08 to
d8c3ea1
Compare
dee077
changed the title
dee077
force-pushed
the
feature/1344-mass-command-execution-pipeline
branch
5 times, most recently
from
July 15, 2026 23:49
ac14b28 to
8efe276
Compare
dee077
force-pushed
the
feature/1344-mass-command-execution-pipeline
branch
from
July 16, 2026 22:03
1e4e4ee to
4994e34
Compare
Member
|
This PR implements real-time monitoring with polling. Is polling the intended final approach? #1344 includes a Mass Command WebSocket endpoint as a requirement, although it leaves the scope open to move or split. This is confusing. Please clarify. Let's implement this asap. |
Base automatically changed from
feature/1344-mass-command-execution-pipeline
to
gsoc26-mass-commands
July 22, 2026 00:54
dee077
force-pushed
the
gsoc26-mass-commands
branch
from
July 23, 2026 09:55
2c48a5e to
0249d6a
Compare
dee077
force-pushed
the
feature/1345-mass-command-admin-workflow
branch
from
July 23, 2026 15:56
b4dbb98 to
b597697
Compare
Code Style and QA FailuresHello @dee077, The CI failed due to code style and quality issues detected by
To fix these issues, please run the following command in your local environment: openwisp-qa-formatThis command will automatically format your code and sort imports according to the project's standards. |
dee077
force-pushed
the
gsoc26-mass-commands
branch
from
July 24, 2026 13:45
0249d6a to
49ee78c
Compare
dee077
force-pushed
the
feature/1345-mass-command-admin-workflow
branch
from
July 24, 2026 13:46
b597697 to
ec3dd20
Compare
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
Files Reviewed (18 files)
Fix these issues in Kilo Cloud Previous Review Summaries (26 snapshots, latest commit 167644a)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 167644a)Status: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Previous review (commit c5dd7ba)Status: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Previous review (commit 230d1e2)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous review (commit 1738698)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous review (commit 90a5698)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous review (commit 9a27af8)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous review (commit 6dcde77)Status: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Previous review (commit ddb50d0)Status: No Issues Found | Recommendation: Merge Files Reviewed (26 files)
Previous review (commit e47dc78)Status: No Issues Found | Recommendation: Merge Files Reviewed (26 files)
Previous review (commit 24c66de)Status: No Issues Found | Recommendation: Merge Files Reviewed (6 files)
Previous review (commit 701f9e6)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (4 files)
Fix these issues in Kilo Cloud Previous review (commit 377bc4f)Status: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
Previous review (commit bafc1d3)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (8 files)
Fix these issues in Kilo Cloud Previous review (commit 7e7e66e)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
Files Reviewed (8 files)
Fix these issues in Kilo Cloud Previous review (commit 74c2fb9)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
Files Reviewed (18 files)
Fix these issues in Kilo Cloud Previous review (commit f4b9b43)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
Files Reviewed (18 files)
Fix these issues in Kilo Cloud Previous review (commit 19b416e)Status: No Issues Found | Recommendation: Merge Files Reviewed (5 files)
Previous review (commit 51a2310)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous review (commit abcee2d)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous review (commit b8d5b7a)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous review (commit b963e72)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous review (commit 43e2d47)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit 6443749)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit 527751d)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
Files Reviewed (10 files)
Fix these issues in Kilo Cloud Previous review (commit f3f90e4)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
Files Reviewed (10 files)
Fix these issues in Kilo Cloud Previous review (commit ec3dd20)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
Files Reviewed (10 files)
Reviewed by balanced · Input: 123.3K · Output: 63K · Cached: 4.7M |
dee077
force-pushed
the
feature/1345-mass-command-admin-workflow
branch
2 times, most recently
from
September 13, 2026 20:58
cae04c7 to
460c2b8
Compare
dee077
force-pushed
the
gsoc26-mass-commands
branch
from
September 14, 2026 11:19
48dd660 to
d8675d4
Compare
…admin link - Add cached_property on AbstractBatchCommand (excludes skipped) - Use in changelist list_display for consistent status colors - Replace ID with label as the clickable link in admin changelist - Add CSS to command-inline.css for consistency - Add label, notes to change form fields; reorder columns (created last, affected_devices before created)
Aligns the admin workflow with the patterns used by the batch upgrade of openwisp-firmware-upgrader and fixes the issues found while reviewing the whole feature. - Reuse BatchCommand.dry_run() for the confirm page target queryset instead of duplicating the targeting rule in the admin - Return querysets from resolve_devices() and dry_run(), consuming them with iterator() where the whole result is walked - Restore the live counters: affected_devices and total_devices were cached properties, which froze the websocket payload at the value computed for the first command of the batch - Truncate the command output of the results table to its last line - Show date and time in the "Modified" column, formatted server side so that live rows and reloaded rows are identical - Fix the location filter of the skipped devices, which used a non existing device_id field of DeviceLocation and raised a 500 - Show the "Clear all filters" link for the location, group and organization filters too - Remove one COUNT query per changelist row by annotating the affected devices, and fetch the batch and the skipped devices only once per request - Use message_user(), load the swappable models at module level and drop the duplicated readonly fields for consistency with the other admin classes - Restructure batch-command.js and execute-command.js to module level functions, dropping the dead gettext fallbacks and guards - Sync the verbose name of skipped_devices in the migrations, which was left unmigrated and failed checkmigrations - Update the query count of the estimated location tests, the location foreign key of BatchCommand adds a SET NULL cascade Closes #1345
- Use _registry instead of get_model_admin(), which is Django 5.0+ while the CI matrix still runs Django 4.2 - Validate the UUID request parameters before they reach the queryset filters, a malformed id returned a 500 - Log the ValidationError swallowed when resolving the wizard targets - Store the device name and error in skipped_devices and cap the admin field to a count, a per reason breakdown and ten devices, a batch skipping thousands of devices rendered one line each - Render the skipped devices live: send bounded counts and previews on batch_status and window the skipped rows into the paginated page of the websocket resync - Use gettext instead of gettext_lazy in the websocket payload, the lazy proxy could not be serialized by the channel layer - Drop the page parameter from the change page filter links so that filtering restarts from the first page - Keep deleted devices in the skipped rows of the unfiltered table, the field and the table disagreed on the count - Add an accessible label to the device checkboxes of the confirm page - Validate the change password fields inline, the form is submitted with novalidate so the length was never checked - Restore the wizard values when going back from the confirm page - Hide the command types the organization is not allowed to run from non superusers, every other entry point already filtered them - Extract the repeated field markup of the execute page into an include and use SimpleNamespace for the status filter spec - Use the locale aware format for the "Triggered by" timestamp - Drop the full stop from the two validation messages shown in the skipped devices list - Remove three redundant queries from the execute endpoint: the devices check of an unsaved batch, the second count of the websocket payload and the emptiness check after devices.set() - Return an empty command queryset for the "skipped" status filter, it listed every command of the batch on top of the skipped devices - Drop the command input from the batch websocket payloads and mask it in the admin, the change_password plaintext was exposed until the celery task cleaned it - Reuse the affected devices count for the total rows, total_devices ran the same COUNT a second time on every command save - Submit the execute form from its submit event so that pressing Enter runs the same validation as the button
- give the confirm page device admin its own readonly_fields copy - reject an execution whose wizard token or device set no longer matches - check the view permission in the batch websocket consumer instead of add - defer batch websocket broadcasts to transaction commit and log failures - take the affected devices count from the creating loop instead of a query - cap the command output preview to the last 100 characters - build the batch filters from skipped devices too and page them lazily - preserve server totals on the change page whenever a filter is active - link live rows to the device recent commands section - scope the group and location choices to the selected organization - use the command schema widget for the mass command input, so any registered command type can be configured, reviewed and executed - keep its generated fields and validation errors consistent with the rest of the admin form
- cover the admin views, the API and the models of the mass command workflow, reaching ~99% coverage - add selenium tests for the wizard, the review page and the mass command page, with their search, filters and multitenancy
- Replaced the assert authorizing websocket connections with an explicit conditional, which python -O cannot strip - Rechecked permissions and organization scope on every websocket request and pushed update, closing the socket when access was revoked - Accepted binary frames in BatchCommandConsumer.receive() and ignored them - Revalidated batch ownership when creating child commands and before executing each one, skipping devices transferred to another organization - Made the batch status write conditional on the status the calculation started from, so a stale result cannot overwrite a newer one - Evaluated the confirm page device ids once, so the reviewed set and the executed set cannot diverge - Rotated the wizard token when the target set changes, dropping the exclusions of the previous set - Sent the active filters with the reconnect request, applied them before pagination and reconciled the table against the filtered snapshot - Moved the batch command signal receivers to connection/handlers.py - Moved skipped device counting, filtering and row building to the model - Broadcast canonical status and ISO timestamps, rendering them in the browser like the single command page does - Derived the device admin URLs from the swapped model - Restored the wizard when the confirm page Back link is used - Returned the union of the schemas available to superusers - Made the password masking of the input summary case insensitive - Restored the punctuation of the command validation messages - Pluralized the device count of the confirm page - Documented mass commands as asynchronous with a concurrency which depends on the celery workers - Removed the leftovers of the dropped Django 4.2 support
- Restored the stderr capture in the two command task tests which still used redirect_stderr after its import was removed, fixing the failing test suite - Built the filters of the detail page from the organization of the batch instead of its command rows, so that they are rendered right after the execution instead of appearing only once the worker created the commands - Hid the location and the device group filter when the batch was targeted on one of them - Formatted the initial and the live timestamps through one locale aware path, so that a row does not change format after it is updated
dee077
force-pushed
the
feature/1345-mass-command-admin-workflow
branch
from
September 14, 2026 11:21
460c2b8 to
78b4f64
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
Reference to Existing Issue
Closes #1345.
Builds on top of #1395, which added the
BatchCommandmodel and the REST APIs.Description of Changes
1. Execute page (step 1)
New admin view at
/admin/connection/batchcommand/execute/, reachable from theNetwork Operations menu, which collects the command and its targets:
Non superusers must choose at least one target and can only choose organizations,
groups and locations they manage. A valid submission is stored in the session and
redirects to the confirm page (Post/Redirect/Get), so that the device table of the
next step can be paginated with ordinary GET requests.
2. Confirm page (step 2)
Lists the devices matched by the chosen targets, resolved with
BatchCommand.dry_run()so that the admin and the background execution cannotdrift apart, and lets single devices be excluded before executing.
The table is the changelist of the
ModelAdmincurrently registered forDevice, composed at request time, so the columns added by other modules(eg. the health status of openwisp-monitoring) appear here too. The exclusions are
kept in
sessionStorage, so they survive the page changes of the device table.3. Change page
Custom change form showing the details of the mass command and a paginated table
of its results:
they were skipped
(superusers), plus a search by device name
4. Real time monitoring
New websocket endpoint:
BatchCommandConsumerreusesBaseDeviceConsumerand accepts the connectiononly for superusers and for staff users who can add and view mass commands and
manage the organization of the batch.
The change page updates without reloading: the status of each device, the output,
the counter of the affected devices and the paginator. On connect and on every
reconnection the client asks for the state of the page it is showing, so nothing
is missed while the connection is down.
Messages sent to the browser:
5. Changelist
Filters by organization, status, type, group and location, and search by label,
notes, organization, device, location and group.
6. Flow
Screenshots
Mass command workflow from the openwisp-controller admin
Mass.comand.demo.webm
Mass command workflow from the openwisp-monitoring admin
Screencast.from.2026-08-15.03-52-52.webm
Todo