fix(spp_change_request_v2): fix batch approval wizard line deletion#130
fix(spp_change_request_v2): fix batch approval wizard line deletion#130
Conversation
Replace act_window bindings with server actions that create the wizard record before opening the form, so One2many line operations work without "Please save your changes first" errors.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug in the batch approval wizard where One2many line operations, such as deleting rows, failed because the wizard was initialized in an unsaved state. The solution ensures that the wizard record is persisted to the database immediately upon its creation, thereby enabling all expected functionalities for its lines and improving the user experience for batch approvals. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively resolves the issue with deleting lines in the batch approval wizard by switching from default_get to a create_from_selection method called by server actions. This is a robust and standard Odoo approach for this kind of problem. The changes are well-implemented across the Python logic, XML views, and tests. The refactoring of the tests to use a helper method for wizard creation is a nice improvement for maintainability. I have one suggestion to improve internationalization by making error messages translatable.
| if change_request.approval_state != "pending": | ||
| return f"Not pending approval (state: {change_request.display_state})" | ||
| if not change_request.can_approve: | ||
| return "You are not authorized to approve this request" |
There was a problem hiding this comment.
The error messages returned by this method are not translatable. To support internationalization, they should be wrapped in _(). The previous implementation had a note about avoiding this in default_get, but this concern is likely not applicable anymore now that the logic is in create_from_selection which is called from a server action.
| if change_request.approval_state != "pending": | |
| return f"Not pending approval (state: {change_request.display_state})" | |
| if not change_request.can_approve: | |
| return "You are not authorized to approve this request" | |
| if change_request.approval_state != "pending": | |
| return _("Not pending approval (state: %s)") % change_request.display_state | |
| if not change_request.can_approve: | |
| return _("You are not authorized to approve this request") |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 19.0 #130 +/- ##
==========================================
+ Coverage 69.96% 70.01% +0.04%
==========================================
Files 832 835 +3
Lines 48813 49139 +326
==========================================
+ Hits 34154 34406 +252
- Misses 14659 14733 +74
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Add tests for create_from_selection error paths, action types, return value validation, error messages, and line removal.
Move tests that don't need pending CRs into a separate class so they run even without approval definitions, improving patch coverage.
1 participant
Why is this change needed?
The batch approval wizard opened in "new" (unsaved) mode via
default_get, causing all One2many line operations (delete rows, object buttons) to fail with "Please save your changes first".How was the change implemented?
default_getwithcreate_from_selection()method that explicitly creates the wizard record in the DB before returning the window actionir.actions.act_windowtoir.actions.serverthat callcreate_from_selection()action_remove_linebutton — Odoo's built-in One2many delete works now that the record is savedcreate_from_selectioninstead ofcreate({})New unit tests
Existing tests updated to use the new creation flow.
Unit tests executed by the author
How to test manually
Related links