Fix Modified timestamp in commands.js#1316
Fix Modified timestamp in commands.js#1316k-sumayya wants to merge 2 commits intoopenwisp:masterfrom
Conversation
📝 WalkthroughWalkthroughThis PR advances the openwisp-controller submodule pointer and applies two small fixes: (1) AbstractVpnClient._get_common_name no longer mutates the device's Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openwisp-controller (1)
1-54:⚠️ Potential issue | 🟠 MajorPR scope mismatch: this change set does not implement the timestamp bug fix.
From Line 1 onward, this submodule update only includes CI workflow changes (
.github/workflows/bot-ci-failure.yml). I don’t see the required frontend change inconnection/static/connection/js/commands.js(response.created→response.modified) described in the PR objective, so the reported user-facing bug would remain unaddressed in this diff.Please update the submodule pointer (or include the intended commit) that contains the
commands.jsfix.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openwisp-controller` around lines 1 - 54, The PR only updates the CI workflow and is missing the frontend timestamp fix; update the submodule pointer (or include the missing commit) that contains the change in connection/static/connection/js/commands.js where the code should use response.modified instead of response.created (search for the symbol response.created in commands.js and replace it with response.modified), then commit the updated submodule reference (or add the file change) so the PR includes both the CI updates and the frontend bugfix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@openwisp-controller`:
- Around line 1-54: The PR only updates the CI workflow and is missing the
frontend timestamp fix; update the submodule pointer (or include the missing
commit) that contains the change in connection/static/connection/js/commands.js
where the code should use response.modified instead of response.created (search
for the symbol response.created in commands.js and replace it with
response.modified), then commit the updated submodule reference (or add the file
change) so the PR includes both the CI updates and the frontend bugfix.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ac9b0fdc-561e-4441-9330-baaac54d89c4
📒 Files selected for processing (3)
openwisp-controlleropenwisp_controller/config/base/vpn.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). (8)
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.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/static/connection/js/commands.jsopenwisp_controller/config/base/vpn.py
**/*.{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/static/connection/js/commands.jsopenwisp_controller/config/base/vpn.py
**/*.{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/static/connection/js/commands.jsopenwisp_controller/config/base/vpn.py
**/*.{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/config/base/vpn.py
🧠 Learnings (4)
📚 Learning: 2026-02-17T19:13:17.774Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/whois/commands.py:0-0
Timestamp: 2026-02-17T19:13:17.774Z
Learning: The openwisp-controller repository does not use Ruff as a linting tool. Do not suggest code changes based on Ruff warnings for this repository.
Applied to files:
openwisp-controller
📚 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/config/base/vpn.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/config/base/vpn.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/config/base/vpn.py
🔇 Additional comments (3)
openwisp_controller/connection/static/connection/js/commands.js (1)
507-509: Correct mapping for “Modified” timestamp.Line 508 now uses
response.modified, which matches the field label and backend payload, so the Recent Commands UI will display the right timestamp.As per coding guidelines, "Ensure variables, functions, classes, and files have descriptive and consistent names," and this change restores consistent field-to-value mapping.
openwisp_controller/config/base/vpn.py (1)
895-901: Good fix: avoids mutatingdevice.namewhile preserving CN formatting.Line 895 and Line 900 correctly keep truncation local (
name) and pass that value into formatting, which aligns behavior without mutating model state. No new correctness/security/performance issues in this segment.openwisp-controller (1)
17-21: Good least-privilege permission scoping in workflow.Line 19 reduces global permissions, and Line 48-Line 51 scopes write permissions to the specific job that needs them. This is a solid security improvement.
Also applies to: 48-51
k-sumayya
force-pushed
the
fix-modified-timestamp
branch
from
aa5fb9c to
c40911a
Compare
Commit Message Format FailureHello @k-sumayya, The commit message is not in the correct format. The issue number referenced in the title must also be included in the body. Example of correct format: |
k-sumayya
force-pushed
the
fix-modified-timestamp
branch
from
c40911a to
b2dd709
Compare
This commit fixes an issue where the Modified timestamp was incorrectly using response.created instead of response.modified. Fixes openwisp#1263
k-sumayya
force-pushed
the
fix-modified-timestamp
branch
from
b2dd709 to
54cb1ec
Compare
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`:
- Line 1: The submodule pointer currently references commit d4d013f... which
doesn't include the bug fix for issue `#1263`; update the submodule reference in
the repository to the commit that contains the commands.js fix (54cb1ec) so the
change from response.created to response.modified is included, then run git
submodule sync && git submodule update --init --recursive (or update the
submodule SHA in the parent repo and commit that change) ensuring the new
pointer points to commit 54cb1ec that includes the commands.js modification.
🪄 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: 2c4d042e-ccb2-4758-a785-04695c04fd1a
📒 Files selected for processing (3)
openwisp-controlleropenwisp_controller/config/base/vpn.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.12 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.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/static/connection/js/commands.jsopenwisp_controller/config/base/vpn.py
**/*.{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/static/connection/js/commands.jsopenwisp_controller/config/base/vpn.py
**/*.{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/static/connection/js/commands.jsopenwisp_controller/config/base/vpn.py
**/*.{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/config/base/vpn.py
🧠 Learnings (4)
📚 Learning: 2026-02-17T19:13:17.774Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/whois/commands.py:0-0
Timestamp: 2026-02-17T19:13:17.774Z
Learning: The openwisp-controller repository does not use Ruff as a linting tool. Do not suggest code changes based on Ruff warnings for this repository.
Applied to files:
openwisp-controller
📚 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/config/base/vpn.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/config/base/vpn.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/config/base/vpn.py
🔇 Additional comments (2)
openwisp_controller/connection/static/connection/js/commands.js (1)
507-509: Correct timestamp source for Modified field (Line 508) — LGTM.Switching to
response.modifiedfixes the incorrect UI mapping and aligns this render path with backend serializer data and websocket updates.openwisp_controller/config/base/vpn.py (1)
895-902: Excellent fix to eliminate side effects!The refactoring correctly prevents mutation of
d.nameby:
- Creating a local
namevariable with the truncated value (line 895)- Using the local
namein the ZeroTier format comparison (line 898)- Overriding the
namekey when spreadingd.__dict__to ensure the format string receives the truncated value (line 900)This maintains the same logical behavior while eliminating the unintended side effect of modifying the device's name attribute.
| @@ -0,0 +1 @@ | |||
| Subproject commit d4d013f39eafc701089f287785c3aed81c07f7bd | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Submodule pointer in current branch =="
git ls-tree HEAD openwisp-controller
echo
echo "== Recent commits in submodule checkout =="
git -C openwisp-controller log --oneline -n 15 || true
echo
echo "== Files changed in target commit =="
git -C openwisp-controller show --name-only --pretty=format:'%H %s' d4d013f39eafc701089f287785c3aed81c07f7bd
echo
echo "== Inspect commands.js at target commit for created/modified usage =="
git -C openwisp-controller show d4d013f39eafc701089f287785c3aed81c07f7bd:connection/static/connection/js/commands.js | rg -n 'response\.(created|modified)|Modified:'Repository: openwisp/openwisp-controller
Length of output: 662
Submodule pointer does not include the bug fix for issue #1263.
The submodule pointer at line 1 points to commit d4d013f..., which only updates .github/workflows/bot-ci-failure.yml. The actual bug fix for issue #1263 (updating response.created to response.modified in commands.js) is in a different commit (54cb1ec). The current pointer does not deliver the promised user-facing fix.
Update the submodule pointer to include the bug fix commit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openwisp-controller` at line 1, The submodule pointer currently references
commit d4d013f... which doesn't include the bug fix for issue `#1263`; update the
submodule reference in the repository to the commit that contains the
commands.js fix (54cb1ec) so the change from response.created to
response.modified is included, then run git submodule sync && git submodule
update --init --recursive (or update the submodule SHA in the parent repo and
commit that change) ensuring the new pointer points to commit 54cb1ec that
includes the commands.js modification.
2 participants
Fixes #1263
Description
The "Modified" field in Recent Commands was incorrectly using
response.created, which caused it to display the creation timestamp instead of the last modification time.This PR updates the implementation to use
response.modified, ensuring consistency with the API response and WebSocket handler.Changes
response.createdwithresponse.modifiedin commands.jsTesting
Impact