[19.0][MIG] srm: Migration to 19.0#732
Conversation
ivantodorovich
left a comment
ivantodorovich
left a comment
There was a problem hiding this comment.
Can we take the opportunity to add unit tests? 🙏🏻
| ) | ||
| action.setdefault("context", {}) | ||
| action["context"]["default_request_type"] = request_type | ||
| return action |
There was a problem hiding this comment.
returning a Domain instance here works?
Is it converted to a list automatically in the JSON response?
honest question.
There was a problem hiding this comment.
Yes, this should be ok as I can see several examples in core:
https://github.com/odoo/odoo/blob/441a6d1b928a44b9a760f926180a925159edff3e/addons/stock/models/product.py#L593
There was a problem hiding this comment.
Excellent, didn't know that. Thanks!
| ("exist", "Link to an existing vendor"), | ||
| ("nothing", "Do not link to a vendor"), | ||
| ], | ||
| selection="_get_selection_action", |
There was a problem hiding this comment.
Is there a reason to switch to a separated hook method instead of the inline selection options?
The inline selection options are preferred usually
There was a problem hiding this comment.
Translations work just fine with regular inline list in selection fields
There was a problem hiding this comment.
Yep I remember that. I've returned back the selection 😃
I've taken an opportunity and added some tests 😄 |
|
Hello @simahawk I'm ready to update both to speed up process. |
|
This PR has the |
luisDIXMIT
left a comment
luisDIXMIT
left a comment
There was a problem hiding this comment.
I got this error on SRM > Purchases > My Pipeline
yankinmax
force-pushed
the
19.0-mig-srm
branch
2 times, most recently
from
856c8ac to
5d61f53
Compare
fixed now, thanks for the review |
luisDIXMIT
left a comment
luisDIXMIT
left a comment
There was a problem hiding this comment.
Code review and tested on runboat, LGTM!
|
Hello @OCA/crm-sales-marketing-maintainers can you pls take a look and possibly trigger merge? |
|
Hello @mmequignon can you pls take a look and possibly trigger merge? |
|
/ocabot migration srm |
|
On my way to merge this fine PR! |
|
Congratulations, your PR was merged at 30749a3. Thanks a lot for contributing to OCA. ❤️ |
7 participants
No description provided.