[fix] Use correct modified timestamp for Recent Commands#1305
[fix] Use correct modified timestamp for Recent Commands#1305Yashika0724 wants to merge 1 commit intoopenwisp:masterfrom
Conversation
📝 WalkthroughWalkthroughA bug fix in the "Recent Commands" list UI rendering updates the "Modified" field to display the Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openwisp_controller/connection/base/models.py`:
- Around line 543-548: Wrap the user-facing fallback message "No device
connection available" in Django's translation function (use _(...)) and add a
warning log before marking the run as failed; specifically, before assigning
self.status = "failed" and self.output = (self.connection.failure_reason ...
else "...") replace the literal fallback with _("No device connection
available") and call the module logger (e.g. logger.warning(...)) to log that
the no-connection failure path was taken, doing the same change for the other
occurrence at the similar block (lines 573-574) so both failure paths are
localized and warn-logged prior to persisting the failed status.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2c8f87d4-ad20-4190-acd5-807f4925c446
📒 Files selected for processing (2)
openwisp_controller/connection/base/models.pyopenwisp_controller/connection/static/connection/js/commands.js
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp}
📄 CodeRabbit inference engine (Custom checks)
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp}: Flag potential security vulnerabilities in code
Avoid unnecessary comments or docstrings for code that is already clear
Code formatting is compact and readable. Do not add excessive blank lines, especially inside function or method bodies
Flag unused or redundant code
Ensure variables, functions, classes, and files have descriptive and consistent names
New code must handle errors properly: log errors that cannot be resolved by the user with error level, log unusual conditions with warning level, log important background actions with info level, and provide user-facing messages for errors that the user can solve autonomously
Files:
openwisp_controller/connection/base/models.pyopenwisp_controller/connection/static/connection/js/commands.js
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp,sql}
📄 CodeRabbit inference engine (Custom checks)
Flag obvious performance regressions, such as heavy loops, repeated I/O, or unoptimized queries
Files:
openwisp_controller/connection/base/models.pyopenwisp_controller/connection/static/connection/js/commands.js
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp,sh,bash,sql}
📄 CodeRabbit inference engine (Custom checks)
Cryptic or non-obvious code (regex, complex bash commands, or hard-to-read code) must include a concise comment explaining why it is needed and why the complexity is acceptable
Files:
openwisp_controller/connection/base/models.pyopenwisp_controller/connection/static/connection/js/commands.js
**/*.{py,html}
📄 CodeRabbit inference engine (Custom checks)
For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework
Files:
openwisp_controller/connection/base/models.py
🧠 Learnings (3)
📚 Learning: 2026-01-15T15:05:49.557Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/management/commands/clear_last_ip.py:38-42
Timestamp: 2026-01-15T15:05:49.557Z
Learning: In Django projects, when using select_related() to traverse relations (for example, select_related("organization__config_settings")), the traversed relation must not be deferred. If you also use .only() in the same query, include the relation name or FK field (e.g., "organization" or "organization_id") in the .only() list to avoid the error "Field X cannot be both deferred and traversed using select_related at the same time." Apply this guideline to Django code in openwisp_controller/config/management/commands/clear_last_ip.py and similar modules by ensuring any select_related with an accompanying only() includes the related field names to prevent deferred/traversed conflicts.
Applied to files:
openwisp_controller/connection/base/models.py
📚 Learning: 2026-02-17T19:13:10.088Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/whois/commands.py:0-0
Timestamp: 2026-02-17T19:13:10.088Z
Learning: In reviews for the openwisp/openwisp-controller repository, do not propose changes based on Ruff warnings. The project does not use Ruff as its linter; ignore Ruff-related suggestions and follow the repository’s established linting and configuration rules. This guidance applies to all Python files under the openwisp_controller directory.
Applied to files:
openwisp_controller/connection/base/models.py
📚 Learning: 2026-01-15T15:07:17.354Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/geo/estimated_location/tests/tests.py:172-175
Timestamp: 2026-01-15T15:07:17.354Z
Learning: In this repository, flake8 enforces E501 (line too long) via setup.cfg (max-line-length = 88) while ruff ignores E501 via ruff.toml. Therefore, use '# noqa: E501' on lines that intentionally exceed 88 characters to satisfy flake8 without affecting ruff checks. This applies to Python files across the project (any .py) and is relevant for tests as well. Use sparingly and only where breaking lines is not feasible without hurting readability or functionality.
Applied to files:
openwisp_controller/connection/base/models.py
🔇 Additional comments (1)
openwisp_controller/connection/static/connection/js/commands.js (1)
507-509: Correct timestamp field mapping for “Modified” is now accurate.Using
response.modifiedhere matches the API contract and fixes the incorrect duplicate created-time display.
| self.status = "failed" | ||
| self.output = self.connection.failure_reason | ||
| self.output = ( | ||
| self.connection.failure_reason | ||
| if self.connection and self.connection.failure_reason | ||
| else "No device connection available" | ||
| ) |
There was a problem hiding this comment.
Localize the fallback message and log the no-connection failure path.
The new fallback text is user-facing but not translatable, and this unusual failure condition is not logged. Please wrap the message in _() and emit a warning before persisting the failed status.
Proposed patch
if exit_code is None:
self.status = "failed"
+ logger.warning(
+ "Command execution failed: no working device connection",
+ extra={"command_id": str(self.pk), "device_id": str(self.device_id)},
+ )
self.output = (
self.connection.failure_reason
if self.connection and self.connection.failure_reason
- else "No device connection available"
+ else _("No device connection available")
)As per coding guidelines: "**/*.{py,html}: For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework" and "New code must handle errors properly: ... log unusual conditions with warning level ...".
Also applies to: 573-574
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openwisp_controller/connection/base/models.py` around lines 543 - 548, Wrap
the user-facing fallback message "No device connection available" in Django's
translation function (use _(...)) and add a warning log before marking the run
as failed; specifically, before assigning self.status = "failed" and self.output
= (self.connection.failure_reason ... else "...") replace the literal fallback
with _("No device connection available") and call the module logger (e.g.
logger.warning(...)) to log that the no-connection failure path was taken, doing
the same change for the other occurrence at the similar block (lines 573-574) so
both failure paths are localized and warn-logged prior to persisting the failed
status.
Signed-off-by: Yashika0724 <ssyashika1311@gmail.com>
2 participants
The "Modified" field in the Recent Commands inline was using
response.createdinstead ofresponse.modifiedwhen building the HTML element on the client side.This meant that both the "Created" and "Modified" fields showed the same timestamp (the creation time), even after the command status was updated.
Fix
In
commands.js, thegetElementfunction now correctly readsresponse.modifiedfor the Modified field.Fixes #1263