[IMP] cetmix_tower_server: implement UI updates #185
Conversation
WalkthroughThis pull request introduces several enhancements to the Cetmix Tower Server models and views. The changes primarily focus on expanding the Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (2)
cetmix_tower_server/models/cx_tower_plan_line.py (1)
95-111: Enhance code quality with constants and improved documentation.Consider the following improvements:
- Define constants for action types and default messages
- Enhance docstring with parameter and return type documentation
+ # Constants + ACTION_FILE_TEMPLATE = "file_using_template" + ACTION_PLAN = "plan" + DEFAULT_TEMPLATE_MSG = "No template code available" + DEFAULT_PLAN_MSG = "No related lines available" + DEFAULT_PREVIEW_MSG = "No preview available" @api.depends("command_id", "action") def _compute_command_code(self): """ Compute the preview of the command based on its action. + + Computes a preview text based on the command's action type: + - For file_using_template: Shows the template code + - For plan: Shows a list of flight plan line names + - For other actions: Shows a default message + + Dependencies: + - command_id: The related command record + - action: The type of action to be performed """ for line in self: - if line.action == "file_using_template": + if line.action == ACTION_FILE_TEMPLATE: line.command_code = ( - line.command_id.code or "No template code available" + line.command_id.code or DEFAULT_TEMPLATE_MSG ) - elif line.action == "plan": + elif line.action == ACTION_PLAN: flight_plan_lines = line.command_id.flight_plan_id.line_ids preview_lines = [line.name for line in flight_plan_lines] - line.command_code = "\n".join(preview_lines) if preview_lines else "No related lines available" + line.command_code = "\n".join(preview_lines) if preview_lines else DEFAULT_PLAN_MSG else: - line.command_code = "No preview available" + line.command_code = DEFAULT_PREVIEW_MSG🧰 Tools
🪛 Ruff (0.8.2)
108-108: Line too long (111 > 88)
(E501)
🪛 GitHub Actions: pre-commit
[error] 108-108: Line too long (111 > 88 characters)
cetmix_tower_server/views/cx_tower_server_template_view.xml (1)
59-63: Consider UX enhancements for the server count display.The implementation follows Odoo patterns and provides clear visibility of the count. Consider these UX improvements:
<div attrs="{'invisible': [('server_count', '=', False)]}"> <strong>Servers:</strong> - <span class="badge badge-primary"> + <span class="badge badge-primary" + t-att-title="'Number of servers using this template'"> <t t-esc="record.server_count.value" /> </span> </div>🧰 Tools
🪛 GitHub Actions: pre-commit
[warning] File was modified by prettier formatting hook
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cetmix_tower_server/models/cx_tower_command_log.py(1 hunks)cetmix_tower_server/models/cx_tower_plan_line.py(2 hunks)cetmix_tower_server/views/cx_tower_command_log_view.xml(2 hunks)cetmix_tower_server/views/cx_tower_command_view.xml(1 hunks)cetmix_tower_server/views/cx_tower_plan_log_view.xml(1 hunks)cetmix_tower_server/views/cx_tower_server_template_view.xml(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: pre-commit
cetmix_tower_server/views/cx_tower_server_template_view.xml
[warning] File was modified by prettier formatting hook
cetmix_tower_server/models/cx_tower_plan_line.py
[error] 108-108: Line too long (111 > 88 characters)
🪛 Ruff (0.8.2)
cetmix_tower_server/models/cx_tower_plan_line.py
108-108: Line too long (111 > 88)
(E501)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test with OCB
🔇 Additional comments (7)
cetmix_tower_server/models/cx_tower_plan_line.py (1)
47-51: LGTM! Field declaration follows Odoo conventions.The change from a related field to a computed field is well-structured and aligns with the requirement to show dynamic content based on the action type.
cetmix_tower_server/models/cx_tower_command_log.py (1)
63-83: LGTM! Well-structured related fields implementation.The new related fields for plan_log_id tracking are well-implemented with:
- Proper field types matching the related model
- Appropriate store=True for query optimization
- Clear string labels for UI display
cetmix_tower_server/views/cx_tower_plan_log_view.xml (1)
84-84: LGTM! Sorting implementation matches requirements.The default ordering by start_date desc, id desc aligns with the PR objective to sort Flight Plan Logs by "Started" date in descending order.
cetmix_tower_server/views/cx_tower_command_log_view.xml (2)
77-83: LGTM! Well-structured form view group for plan log tracking.The new group of fields provides clear visibility of related Flight Plan Log details, matching the PR objective.
109-109: LGTM! Consistent sorting implementation.The default ordering matches the Flight Plan Log view, maintaining consistency across the application.
cetmix_tower_server/views/cx_tower_command_view.xml (1)
253-257: LGTM! Search filter enhances command filtering capabilities.The new filter for Run Flight Plan commands improves the search functionality by allowing specific filtering of flight plan related commands.
cetmix_tower_server/views/cx_tower_server_template_view.xml (1)
14-14: LGTM! Verify the computed field implementation.The field declaration follows Odoo conventions and is properly positioned with related fields.
Run this script to verify the computed field implementation:
✅ Verification successful
✓ Computed field implementation verified and correct
The server_count field is properly implemented with a clean One2many relationship and follows Odoo best practices. The computation is lightweight and suitable for real-time display in views.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify server_count computed field implementation # Expected: Find the compute method definition and dependencies # Search for server_count compute method ast-grep --pattern 'server_count = fields.$_($$$, compute="$_")' # Search for potential dependencies in compute method rg -A 10 "def _compute_server_count"Length of output: 1293
Script:
#!/bin/bash # Verify server_ids field definition and related performance settings ast-grep --pattern 'server_ids = fields.$_($$$)' # Check for potential performance impact by finding server count usage rg "server_count" -A 2Length of output: 5385
🧰 Tools
🪛 GitHub Actions: pre-commit
[warning] File was modified by prettier formatting hook
This commit implements the following UI updates:
1. Sort Flight Plan Log and Command Log by "Started" (newest first).
- Added `default_order="start_date desc, id desc"` in tree views for both logs.
2. Show related Flight Plan Log line in the Command Log form.
- Added related fields in the Command Log model and updated the form view.
3. Update the "Code" field in the Flight Plan Line form.
- Implemented a computed field `command_code` to display:
- File template code for the action "Upload File from Template".
- Related flight plan lines for the action "Execute Flight Plan".
4. Display the number of servers in the Server Template Kanban view.
- Used the `server_count` computed field to show the count as a numeric value in Kanban cards.
5. Add a pre-defined "Flight Plan" filter to the Command search view.
- Added a filter for the `action='plan'` in the search view of the Command model.
Task ID: 4263
tendil
force-pushed
the
14.0-t4263-cetmix_tower_server-ui-updates
branch
from
280b587 to
54fcb67
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cetmix_tower_server/models/cx_tower_plan_line.py (1)
102-103: Consider optimizing memory usage for large flight plans.For large flight plans, building the preview list could be memory intensive. Consider limiting the number of lines shown.
- flight_plan_lines = line.command_id.flight_plan_id.line_ids - preview_lines = [line.name for line in flight_plan_lines] + # Limit preview to first 10 lines for better performance + flight_plan_lines = line.command_id.flight_plan_id.line_ids[:10] + preview_lines = [line.name for line in flight_plan_lines] + if len(line.command_id.flight_plan_id.line_ids) > 10: + preview_lines.append("... and more lines")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cetmix_tower_server/models/cx_tower_command_log.py(1 hunks)cetmix_tower_server/models/cx_tower_plan_line.py(2 hunks)cetmix_tower_server/views/cx_tower_command_log_view.xml(2 hunks)cetmix_tower_server/views/cx_tower_command_view.xml(1 hunks)cetmix_tower_server/views/cx_tower_plan_log_view.xml(1 hunks)cetmix_tower_server/views/cx_tower_server_template_view.xml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- cetmix_tower_server/views/cx_tower_command_view.xml
- cetmix_tower_server/views/cx_tower_plan_log_view.xml
- cetmix_tower_server/views/cx_tower_command_log_view.xml
- cetmix_tower_server/models/cx_tower_command_log.py
- cetmix_tower_server/views/cx_tower_server_template_view.xml
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test with OCB
🔇 Additional comments (1)
cetmix_tower_server/models/cx_tower_plan_line.py (1)
47-49: LGTM! Field declaration follows best practices.The change from a related field to a computed field is well-structured and follows Odoo field declaration conventions.
| @api.depends("command_id", "action") | ||
| def _compute_command_code(self): | ||
| """ | ||
| Compute the preview of the command based on its action. | ||
| """ | ||
| for line in self: | ||
| if line.action == "file_using_template": | ||
| line.command_code = line.command_id.code or "No template code available" | ||
| elif line.action == "plan": | ||
| flight_plan_lines = line.command_id.flight_plan_id.line_ids | ||
| preview_lines = [line.name for line in flight_plan_lines] | ||
| line.command_code = ( | ||
| "\n".join(preview_lines) | ||
| if preview_lines | ||
| else "No related lines available" | ||
| ) | ||
| else: | ||
| line.command_code = "No preview available" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation checks for better error handling.
The compute method should validate the existence of required related records to prevent potential errors.
@api.depends("command_id", "action")
def _compute_command_code(self):
"""
Compute the preview of the command based on its action.
"""
for line in self:
+ if not line.command_id:
+ line.command_code = "No command selected"
+ continue
+
if line.action == "file_using_template":
line.command_code = line.command_id.code or "No template code available"
elif line.action == "plan":
+ if not line.command_id.flight_plan_id:
+ line.command_code = "No flight plan selected"
+ continue
+
flight_plan_lines = line.command_id.flight_plan_id.line_ids
preview_lines = [line.name for line in flight_plan_lines]
line.command_code = (
"\n".join(preview_lines)
if preview_lines
else "No related lines available"
)
else:
line.command_code = "No preview available"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @api.depends("command_id", "action") | |
| def _compute_command_code(self): | |
| """ | |
| Compute the preview of the command based on its action. | |
| """ | |
| for line in self: | |
| if line.action == "file_using_template": | |
| line.command_code = line.command_id.code or "No template code available" | |
| elif line.action == "plan": | |
| flight_plan_lines = line.command_id.flight_plan_id.line_ids | |
| preview_lines = [line.name for line in flight_plan_lines] | |
| line.command_code = ( | |
| "\n".join(preview_lines) | |
| if preview_lines | |
| else "No related lines available" | |
| ) | |
| else: | |
| line.command_code = "No preview available" | |
| @api.depends("command_id", "action") | |
| def _compute_command_code(self): | |
| """ | |
| Compute the preview of the command based on its action. | |
| """ | |
| for line in self: | |
| if not line.command_id: | |
| line.command_code = "No command selected" | |
| continue | |
| if line.action == "file_using_template": | |
| line.command_code = line.command_id.code or "No template code available" | |
| elif line.action == "plan": | |
| if not line.command_id.flight_plan_id: | |
| line.command_code = "No flight plan selected" | |
| continue | |
| flight_plan_lines = line.command_id.flight_plan_id.line_ids | |
| preview_lines = [line.name for line in flight_plan_lines] | |
| line.command_code = ( | |
| "\n".join(preview_lines) | |
| if preview_lines | |
| else "No related lines available" | |
| ) | |
| else: | |
| line.command_code = "No preview available" |
There was a problem hiding this comment.
For now, don't as part of this task.
There was a problem hiding this comment.
@tendil Understood, I'll skip the validation checks for now. Would you like me to create a follow-up issue to track this improvement for future implementation?
| @@ -60,6 +60,20 @@ class CxTowerCommandLog(models.Model): | |||
| plan_log_id = fields.Many2one(comodel_name="cx.tower.plan.log", ondelete="cascade") | |||
| triggered_plan_log_id = fields.Many2one(comodel_name="cx.tower.plan.log") | |||
|
|
|||
| # -- Related fields for plan_log_id | |||
| plan_start_date = fields.Datetime( | |||
There was a problem hiding this comment.
No need to show all these fields. Just show the related flight_plan_line.
<field name="plan_log_id" attrs="{'invisible': [('plan_log_id', '=', False)]}"/>
| @@ -44,7 +44,9 @@ class CxTowerPlanLine(models.Model): | |||
| help="Actions trigger based on command result." | |||
| " If empty next command will be executed", | |||
| ) | |||
| command_code = fields.Text(related="command_id.code", readonly=True) | |||
| command_code = fields.Text( | |||
There was a problem hiding this comment.
This approach doesn't allow to check those lines if required.
So we need to add a compute o2m field for lines and hide/show it based on the command action.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cetmix_tower_server/models/cx_tower_plan_line.py(2 hunks)cetmix_tower_server/views/cx_tower_command_log_view.xml(2 hunks)cetmix_tower_server/views/cx_tower_plan_line_view.xml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cetmix_tower_server/views/cx_tower_command_log_view.xml
🔇 Additional comments (3)
cetmix_tower_server/views/cx_tower_plan_line_view.xml (1)
42-50: LGTM! Well-structured visibility rules.The visibility rules for
command_codeandplan_line_idsfields are correctly implemented based on theactionfield value, following the feedback from the previous review.cetmix_tower_server/models/cx_tower_plan_line.py (2)
110-120: LGTM! Well-implemented plan lines computation.The
_compute_plan_line_idsmethod correctly:
- Handles the 'plan' action case
- Includes proper null check for flight_plan
- Sets False for non-plan actions
47-55: LGTM! Field definitions are clear and well-structured.The computed fields are properly defined with correct parameters and string labels.
tendil
force-pushed
the
14.0-t4263-cetmix_tower_server-ui-updates
branch
from
5111416 to
8fec17f
Compare
Task ID: 4263
tendil
force-pushed
the
14.0-t4263-cetmix_tower_server-ui-updates
branch
from
8fec17f to
76d5c23
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cetmix_tower_server/models/cx_tower_plan_line.py (1)
99-111: Simplify the command code computation logic.The condition
elif line.action != "plan"is redundant since we already handle the 'plan' action in the previous condition.@api.depends("command_id", "action") def _compute_command_code(self): """ Compute the preview of the command based on its action. """ for line in self: if line.action == "file_using_template": line.command_code = line.command_id.code or "No template code available" elif line.action == "plan": line.command_code = False - elif line.action != "plan": + else: line.command_code = "No preview available"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cetmix_tower_server/models/cx_tower_plan_line.py(2 hunks)cetmix_tower_server/views/cx_tower_command_log_view.xml(2 hunks)cetmix_tower_server/views/cx_tower_plan_line_view.xml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- cetmix_tower_server/views/cx_tower_plan_line_view.xml
- cetmix_tower_server/views/cx_tower_command_log_view.xml
🔇 Additional comments (2)
cetmix_tower_server/models/cx_tower_plan_line.py (2)
47-55: LGTM! Field declarations are well-structured.The computed fields
command_codeandplan_line_idsare properly defined with appropriate field types and compute methods.
112-123: 🛠️ Refactor suggestionAdd validation for command_id in plan line computation.
The method could raise an AttributeError if
command_idis False. Add validation to handle this case gracefully.@api.depends("command_id", "action") def _compute_plan_line_ids(self): """ Compute the related plan lines if the action is "plan". """ for line in self: - if line.action == "plan": + if not line.command_id: + line.plan_line_ids = False + elif line.action == "plan": flight_plan = line.command_id.flight_plan_id line.plan_line_ids = flight_plan.line_ids if flight_plan else [] else: line.plan_line_ids = FalseLikely invalid or redundant comment.
| @@ -60,6 +60,20 @@ class CxTowerCommandLog(models.Model): | |||
| plan_log_id = fields.Many2one(comodel_name="cx.tower.plan.log", ondelete="cascade") | |||
| triggered_plan_log_id = fields.Many2one(comodel_name="cx.tower.plan.log") | |||
|
|
|||
| # -- Related fields for plan_log_id | |||
| plan_start_date = fields.Datetime( | |||
| @@ -88,6 +96,31 @@ def _check_command_id(self): | |||
| visited_plans = set() | |||
| self._check_recursive_plan(line.command_id, visited_plans) | |||
|
|
|||
| @api.depends("command_id", "action") | |||
| def _compute_command_code(self): | |||
There was a problem hiding this comment.
No need to compute anything. Just hide this field if command action is not SSH or Python.
This commit implements the following UI updates:
Sort Flight Plan Log and Command Log by "Started" (newest first).
default_order="start_date desc, id desc"in tree views for both logs.Show related Flight Plan Log line in the Command Log form.
Update the "Code" field in the Flight Plan Line form.
command_codeto display:Display the number of servers in the Server Template Kanban view.
server_countcomputed field to show the count as a numeric value in Kanban cards.Add a pre-defined "Flight Plan" filter to the Command search view.
action='plan'in the search view of the Command model.Task ID: 4263
Summary by CodeRabbit
Release Notes
New Features
Improvements
User Interface