[T3409] Interaction resume - #2154
Conversation
There was a problem hiding this comment.
Greptile has paused reviews on this repository — it used its 100 free open-source review credits for this billing period. Reviews resume automatically on September 13. To continue before then, an organization admin can keep reviews running past the free credits — those bill as normal usage.
…pdating an interaction
08bb7dd to
b2d80ac
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
It rewrites core interaction-resume dedup/update logic and changes cross-module field types, so it needs human verification beyond the one translation-mismatch issue found.
Pull request overview
This PR improves the "interaction resume" feature so that re-fetching a contact's interactions updates existing resume entries in place instead of clearing and recreating them, and it hardens the surrounding UX. It also fixes address rendering for thank-you letters/emails and prevents sending communications that have no send mode. A large end-to-end tour test suite is added. This ties into CompassionCH/compassion-switzerland#1821.
Changes:
- Rewrote
interaction.resume.createto deduplicate via a per-source_identity/_interaction_discriminatorand update existing entries, plus arefresh_interactionsthat re-fetches all loaded pages without wiping them. - Simplified the log-interaction wizard (persist attachments by reassigning them, show a success notification instead of a temporary chatter note) and blocked sending a communication with no send mode (Python guard + dedicated button + translations).
- Made
short_address/newaddress_without_nameproperHtmlfields so line breaks render, and added JS tour tests (with eslint globals) covering interactions, calls and communications.
File summaries
| File | Description |
|---|---|
| interaction_resume/models/interaction_resume.py | Identity-based dedup and in-place update of resume entries on create |
| interaction_resume/models/abstract_interaction_source.py | Base _interaction_discriminator hook (empty by default) |
| interaction_resume/models/crm_request.py | Claim discriminator on (date, subject) for multi-entry sources |
| interaction_resume/models/crm_phonecall.py | Adds has_attachment to phone call resume data |
| interaction_resume/models/other_interaction.py | Adds has_attachment; refresh (not reset) on write, guarded by _transient |
| interaction_resume/models/res_partner.py | New refresh_interactions re-fetching every loaded page |
| interaction_resume/wizards/partner_log_other_interaction_wizard.py | Persist attachments by reassignment; return success notification |
| interaction_resume/views/interaction_resume.xml | Recolored list decorations by direction/type |
| interaction_resume/manifest.py | Registers tour test asset bundle |
| interaction_resume/static/tests/tours/interaction_resume.js | New tours for interactions, calls, communications |
| interaction_resume/tests/*.py | New HttpCase tour tests |
| partner_communication/models/communication_job.py | Blocks sending jobs without a send mode |
| partner_communication/views/communication_job_view.xml | Adds "Send Now" button for no-send-mode jobs with tooltip |
| partner_communication/i18n/{de,fr_CH,it}.po | Translations for the new send-mode messages |
| thankyou_letters/models/res_partner.py | short_address/address_without_name as Html; refactored compute |
| thankyou_letters/tests/test_partner_address.py | Tests address markup/line-break rendering |
| eslint.config.cjs | Applies module config + browser globals to all .js |
Review details
- Files reviewed: 21/21 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Add additional message to UserError for missing send mode. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
| return ( | ||
| fields.Datetime.to_datetime(vals.get("date")) or False, | ||
| vals.get("subject") or False, | ||
| ) |
There was a problem hiding this comment.
A claim can contain separate thread messages with the same timestamp and subject. This discriminator uses only those values with the claim identity, so both messages resolve to the same identity and batch creation drops the second one. The refreshed interaction timeline therefore omits a real CRM message. Include an immutable per-message value, such as the source mail.message ID. This must be resolved before merging.
Artifacts
- The authored executable loads the targeted source with a minimal Odoo stub and submits two distinct same-date, same-subject CRM claim message values to the real identity and batch-create logic, demonstrating the deduplication condition.
- The baseline command ran against origin/18.0 and shows that same-claim messages already collapsed without a discriminator, establishing the comparison scope.
- The HEAD command ran successfully and shows equal full identities, one created entry, and one omitted message, confirming the PR does not distinguish this collision.
- Python compiled the authored validation script successfully before execution, confirming the recorded reproduction used syntactically valid test code.
| if len(self) == 1: | ||
| self = self.with_context(queue_job__no_delay=True) | ||
| if self.state == "pending" and not self.send_mode: | ||
| raise UserError( | ||
| _( | ||
| "This communication has no send mode, so it cannot be sent." | ||
| " Choose below how it should go out and save." | ||
| ) | ||
| ) | ||
|
|
||
| # Filter "pending" tasks | ||
| todo = self.filtered( | ||
| lambda j: j.state == "pending" | ||
| and j.send_mode | ||
| and not (j.need_call == "before_sending" and j.activity_ids) |
There was a problem hiding this comment.
Report skipped bulk communications
The missing-send-mode error only runs for a single selected job. In a bulk send, jobs without a delivery mode are filtered out while the method returns a normal result for the whole selection. Operators can believe every selected communication was sent even though skipped jobs remain pending and their completion-dependent work never occurs. Validate every pending selected job before filtering, or explicitly report the skipped jobs. This must be resolved before merging.
Knowledge Base Used: Partner communications
Artifacts
- The executable harness extracts and invokes the exact `send()` implementation from each PR revision with eligible and missing-mode records; it demonstrates the compared behavior.
- Command output from the parent revision shows HTTP 200 OK for the mixed bulk selection, while the missing-mode record stays pending; bulk sending already silently excluded it.
- Command output from commit 5c030aa shows the same HTTP 200 OK and skipped missing-mode record, while only the singleton HTTP 400 message changed; the defect remains.
Correctly update existing resume entries, add a tours test and improve the UX.
Linked to CompassionCH/compassion-switzerland#1821.