-
-
Notifications
You must be signed in to change notification settings - Fork 654
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[16.0][MIG] stock_picking_batch_extended: Migration to 16.0 #1236
[16.0][MIG] stock_picking_batch_extended: Migration to 16.0 #1236
Conversation
07720a7
to
15feeec
Compare
@rousseldenis can you add to issue #1101 |
/ocabot migration stock_picking_batch_extended |
@BorisFL Could you check this https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-16.0? Could you follow especially Full process part ? Thanks |
15feeec
to
aa93955
Compare
Sure! I just uploaded the corrections. Can you review them to make sure they are correct? |
e012f47
to
0b2abca
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BorisFL Thanks for this.
Unfortunately, you cannot change the authorship everywhere. Please keep them. You can add yourself in code parts you improve only.
Could you also limit your migration changes in migration commit, then, do other commit(s) for improvements ?
Thanks
f8ad4d7
to
b3a772d
Compare
@rousseldenis I have already fixed the bugs and sorry for deleting the other authors. |
b3a772d
to
5cc3df7
Compare
@sergio-teruel I accidentally recovered the file. Initially, I was migrating from version 14, and that's why the file got recovered. I have just deleted it now. |
Hi @rousseldenis . I made the corrections you asked for. Please take a look at them when you can. |
@@ -10,11 +10,11 @@ | |||
name="binding_model_id" | |||
ref="stock_picking_batch.model_stock_picking_batch" | |||
/> | |||
<field name="type">ir.actions.server</field> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to add this as this is the default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
action = env.ref('stock.action_report_delivery').report_action(pickings) | ||
<field name="code"> | ||
pickings = records.mapped('picking_ids').filtered(lambda p: p.state != 'cancel') | ||
action = env['ir.actions.report'].sudo()._get_report_from_name('stock.report_delivery').report_action(pickings) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BorisFL Are you sure about the sudo ? As it is done in _get_report_from_name()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
@@ -6,5 +6,5 @@ | |||
|
|||
def post_init_hook(cr, registry): | |||
env = api.Environment(cr, SUPERUSER_ID, {}) | |||
|
|||
env["res.company"].search([]).write({"use_oca_batch_validation": True}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why changing this?
c39c508
to
42562f7
Compare
@@ -99,6 +105,22 @@ def action_print_picking(self): | |||
def remove_undone_pickings(self): | |||
"""Remove of this batch all pickings which state is not done / cancel.""" | |||
self.mapped("active_picking_ids").write({"batch_id": False}) | |||
self.verify_state() | |||
|
|||
def verify_state(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last but not least, this is an addition in behavior from previous version.
IMHO, this module deserves a good DESCRIPTION.rst in readme folder to explain what it does as 'extended' is too general term to understand at first sight. Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have fixed it. I have cleaned up some errors from an initial import I made from v14, and the only change is the removal of "remove_undone_pickings." This change ensures that when a picking is canceled, its grouping is removed in v16
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, nevertheless, a good module description could be a great improvement 😃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added a description to the readme and index files
fe1c794
to
9217cb2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, minor suggestion
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl). | ||
|
||
{ | ||
"name": "Stock batch picking extended", | ||
"summary": "Allows manage a lot of pickings in batch", | ||
"version": "15.0.2.2.0", | ||
"version": "16.0.1.0.0", | ||
"author": "Camptocamp, " "Tecnativa, " "Odoo Community Association (OCA)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @BorisFL , thanks for contributing.
can you fix this old pre-commit result?
"author": "Camptocamp, " "Tecnativa, " "Odoo Community Association (OCA)", | |
"author": "Camptocamp, Tecnativa, Odoo Community Association (OCA)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have fixed the concatenation of the authors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks :)
9217cb2
to
989b691
Compare
@pedrobaeza @sergio-teruel Could you update your review ? |
Hello @pedrobaeza and @sergio-teruel . I have made the mentioned modifications. Can you review them? |
Good morning @pedrobaeza and @sergio-teruel , I apologize for insisting, but have you received any notification? Can you review your comments? |
Hello, @sergio-teruel Could you review the changes made and leave an approval if you think it's appropriate? If you see the need for any additional changes, please let me know. From now on, I will take care of this PR, so if anyone thinks we should make any changes, please let me know as well! Thanks to all! |
Hello everyone, the person who developed and maintained this PR is no longer collaborating with our company. We have detect some minor issues so we have decided to close it, reanalyze it, and develop it again. We will upload the new one as soon as possible. PD: I do not have the permissions to close this PR. If someone could do it, I would greatly appreciate it. |
@xAdrianC-FactorLibre any news about the new PR of the module? |
Hello @nicomacr , we have conducted a preliminary analysis and believe that changes need to be made compared to the previous MR. However, I cannot provide much information regarding actions or possible dates as we are currently focused on other priorities. |
@nicomacr @rousseldenis @FrancoMaxime |
@xAdrianCif Thank you for taking care of the migration of the module. |
No description provided.