[16.0][IMP] helpdesk_mgmt_sale: add button create ticket and display sales#947
[16.0][IMP] helpdesk_mgmt_sale: add button create ticket and display sales#947matthieusaison wants to merge 4 commits intoOCA:16.0from
Conversation
Add message on sale order
b7d8b33 to
4647bf4
Compare
|
@matthieusaison Can you increase the test coverage? |
| <button | ||
| name="action_create_helpdesk_ticket" | ||
| type="object" | ||
| string="Créer un ticket" |
There was a problem hiding this comment.
Text must be in english and add a translation to your language.
There was a problem hiding this comment.
Yes, sorry it's a mistake, I fix this in my last commit
| </button> | ||
| </xpath> | ||
| <field name="channel_id" position="after"> | ||
| <field name="sale_order_ids" widget="many2many_tags" /> |
There was a problem hiding this comment.
If sale orders are already displayed in the smart button, what is the need to add it here?
There was a problem hiding this comment.
When you create a ticket from sale, you may need to add an other sale linked. And if you create ticket from helpdesk menu, you may need to link to sale directly.
There was a problem hiding this comment.
Ok, I see your point of view. It seems strange to me to see duplicate data about sale orders in both the field and the button. However, it is true that from the button it is not possible to add existing sale orders, only to create a new one. Anyway, LGTM!
pedrobaeza
left a comment
There was a problem hiding this comment.
Sorry, I don't think this is the way, but to pass in the existing smart-button the default_... context key for using it when pressing "Create" button inside ticket list.
marcos-mendez
left a comment
There was a problem hiding this comment.
Automated Review -- Tests Failed
1. Root Cause
The test failure is due to a database connection error (Connection to the database failed) during the Odoo server startup, not directly caused by the code changes in the PR. This is likely a transient infrastructure issue in the test environment (e.g., Runboat).
2. Suggested Fix
No functional code fix is needed for the test failure itself, but to prevent future failures, ensure that:
- The test environment has a stable database connection.
- The Odoo server is started with proper database configuration.
However, in terms of code correctness:
- The
createmethod inhelpdesk.ticketcorrectly usesself.env.context.get("from_sale_order")and is safe. - The
action_create_helpdesk_ticketmethod insale.ordercorrectly sets the context withfrom_sale_order: True.
3. Additional Code Issues
-
Missing
@api.modeldecorator: Inhelpdesk.ticket.create, the methodcreateis correctly defined with@api.model_create_multi, which is fine. However, the method should not be usingselfinside the loop asselfrefers to the class, not the instance. This is a potential bug.Fix:
# Replace: for sale in tickets.sale_order_ids: sale.message_post(...) # With: for ticket in tickets: for sale in ticket.sale_order_ids: sale.message_post(...)
File:
helpdesk_mgmt_sale/models/helpdesk_ticket.py
Lines: 29–36
4. Test Improvements
The existing tests are good, but can be improved to better cover edge cases and ensure robustness:
Suggested Tests:
-
Test with multiple sale orders:
- Create a ticket with multiple
sale_order_ids. - Ensure messages are posted for each sale order.
- Create a ticket with multiple
-
Test with empty sale orders list:
- Create a ticket without any
sale_order_ids. - Ensure no message is posted.
- Create a ticket without any
-
Test
action_create_helpdesk_ticketwith different user contexts:- Use
with_user()to test behavior with different users. - Ensure correct
default_partner_idanddefault_nameare set.
- Use
-
Use
SavepointCasefor transaction safety:- The current
BaseCommon(likelyTransactionCase) is acceptable, but for better test isolation, consider usingSavepointCasefor tests involvingmessage_post.
- The current
OCA Testing Patterns:
- Use
SavepointCasefor tests involvingmessage_postor complex data changes. - Use
taggedto mark tests for specific scenarios (e.g.,@tagged('post_install')for context-based logic).
Summary
| Item | Status |
|---|---|
| Root Cause | Database connection issue (environmental) |
| Code Bug | self used instead of ticket in loop (line 30) |
| Test Coverage | Good, but can be improved with edge cases |
| Suggested Fix | Replace self with ticket in message posting loop |
Let me know if you'd like a patch for the bug fix.
Reciprocal Review Request
Hi everyone! I found some test failures on this PR and left detailed feedback above. I am happy to discuss or help debug. In the meantime, if any of you get a chance, I would appreciate a look at my open PR(s):
My open PRs across OCA:
- server-tools#3554 [MIG] datetime_formatter: Migration to 18.0
- server-tools#3548 [18.0][MIG] base_kanban_stage: Migration to 18.0
- hr-attendance#262 [16.0][ADD] Hr_attendance_idsecure: iDSecure (ControliD) attendance integration
- stock-logistics-workflow#2276 [16.0][ADD] stock_move_line_devaluation
- stock-logistics-workflow#2275 [16.0][ADD] Stock move line analytic account
- stock-logistics-workflow#2268 [16.0][ADD] stock_move_line_picking_partner
- purchase-workflow#2694 [16.0][IMP]Purchase workflow added to review state & exception fix
Reviewing each other's work helps the whole community move forward. Thank you!
Environment via OCA Neural Reviewer: Minikube + K8s Job + oca-ci/py3.10-odoo16.0 | Odoo 16.0
Automated review by OCA Neural Reviewer + qwen3-coder:30b
…ale-create_ticket_custom_msg [16.0][IMP] helpdesk_mgmt_sale: make the ticket creation message overridable
add button to create a ticket with value of active sale order.
display and default filled sale_order_ids.