[18.0][MIG] helpdesk_mgmt_stock: Migration to 18.0 - #1008
david-banon-tecnativa wants to merge 12 commits into
Conversation
OCA-git-bot
added
series:18.0
mod:helpdesk_product
0671e74 to
0636de5
Compare
ChristianSantamaria
left a comment
ChristianSantamaria
left a comment
There was a problem hiding this comment.
Funcional and code review, LGTM 👍 👍
But i detect a minor changes:
3d772c0 to
c72d599
Compare
eduezerouali-tecnativa
left a comment
eduezerouali-tecnativa
left a comment
There was a problem hiding this comment.
Just two minor things. Everything else LGTM
|
ping @david-banon-tecnativa can you attend the comments please? |
|
/ocabot migration helpdesk_mgmt_stock |
|
Sorry @rousseldenis you are not allowed to mark the addon to be migrated. To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons. If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the |
| ) | ||
| helpdesk_tickets_count = fields.Integer( | ||
| compute="_compute_helpdesk_tickets_count", | ||
| helpdesk_ticket_count = fields.Integer( |
There was a problem hiding this comment.
IMHO this change is not needed as there are possibly several tickets
There was a problem hiding this comment.
I think its better semantically, the same way we do helpdesk_ticket_ids and not helpdesk_tickets_ids, i think count fields should be singular. Example: helpdesk_mgmt_project
There was a problem hiding this comment.
Indeed, we should avoid plurals everywhere where possible as stated in guidelines, and more in between the whole field name. count in this case already references the goal.
There was a problem hiding this comment.
We defined that for modules but not for fields...
There was a problem hiding this comment.
Nevertheless, this should be done in another commit
There was a problem hiding this comment.
The rule is for every identified in general. And this should go in the same migration commit, including the migration script with rename_fields cc @david-banon-tecnativa
There was a problem hiding this comment.
Don't agree on that. In order to backport the changes to previous version. Thanks
There was a problem hiding this comment.
The rule is for every identified in general.
Where ? I just see : https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#11modules
If it is a rule you enforced, please do a PR to improve.
There was a problem hiding this comment.
OK, if you enforce this, we don't change the variable name, but it's a constant source of misspellings.
There was a problem hiding this comment.
Rename done in a separate commit if you want to backport to v16 @rousseldenis
Labels went missing in form view due to a new group being created inside an already existing group (odoo only renders the labels for leaf groups).
Add the "action" prefix to the method called by the wizard "Create Ticket" button in order to signify more clearly that this returns an action.
Pass default picking/move id in context of the different tickets views opened in order to allow to pre-populate those fields when the user creates a new ticket from there.
f245f9c to
f969e1f
Compare
f969e1f to
862b2a9
Compare
Glue module to bridge helpdesk_mgmt_stock and helpdesk_motive
862b2a9 to
6dc7de3
Compare
OCA-git-bot
added
the
mod:helpdesk_mgmt_stock_motive
|
/ocabot migration helpdesk_mgmt_stock |
pedrobaeza
left a comment
pedrobaeza
left a comment
There was a problem hiding this comment.
@david-banon-tecnativa the commit containing the new module helpdesk_mgmt_stock should include the removal of the things in the existing one and the addition, instead of doing the removal in the migration commit.
| "views/helpdesk_ticket_motive_views.xml", | ||
| "wizards/stock_helpdesk_ticket_create_views.xml", | ||
| ], | ||
| "demo": [], |
There was a problem hiding this comment.
| "demo": [], |
can be remove
|
This PR has the |
7 participants
A migration from this PR to 18.0, changes:
helpdesk_motivedependencycc @Tecnativa
ping @rousseldenis