[12.0][ADD] stock_picking_mail_incorrect_qty: Send an e-mail when a stock picking is incomplete

[carmenbianca]

Task: https://gestion.coopiteasy.be/web#id=7745&view_type=form&model=project.task&action=479

Formerly coopiteasy/cie-custom#40

[carmenbianca]

@robinkeunen Added your suggestions.

A few questions also:

• auto_delete is currently set to True in the template. Is this a correct default?
• Is there anything I need to do for translations?
• There's a chance that the code may not run correctly. Summarised below…

Upstream stock has this code:

    def action_done(self):
        # TDE FIXME: remove decorator when migration the remaining
        todo_moves = self.mapped('move_lines').filtered(lambda self: self.state in ['draft', 'waiting', 'partially_available', 'assigned', 'confirmed'])
        # Check if there are ops not linked to moves yet
        for pick in self:
            # # Link existing moves or add moves when no one is related
            for ops in pick.move_line_ids.filtered(lambda x: not x.move_id):
                # Search move with this product
                moves = pick.move_lines.filtered(lambda x: x.product_id == ops.product_id)
                moves = sorted(moves, key=lambda m: m.quantity_done < m.product_qty, reverse=True)
                if moves:
                    ops.move_id = moves[0].id
                else:
                    new_move = self.env['stock.move'].create({
                                                    'name': _('New Move:') + ops.product_id.display_name,
                                                    'product_id': ops.product_id.id,
                                                    'product_uom_qty': ops.qty_done,
                                                    'product_uom': ops.product_uom_id.id,
                                                    'location_id': pick.location_id.id,
                                                    'location_dest_id': pick.location_dest_id.id,
                                                    'picking_id': pick.id,
                                                    'picking_type_id': pick.picking_type_id.id,
                                                   })
                    ops.move_id = new_move.id
                    new_move = new_move._action_confirm()
                    todo_moves |= new_move
                    #'qty_done': ops.qty_done})
        todo_moves._action_done()
        self.write({'date_done': fields.Datetime.now()})
        return True

The entire for-loop that handles a corner case (move.lines unlinked to moves, but linked to stock.picking) is not executed by our module. There may therefore be moves that are invisible to us. Ideally upstream would have SoC'd this and extracted the for-loop into a separate function create_moves_for_unlinked_move_lines(), but that's not the case. If we want to handle this case, we have to:

• Copy the for-loop verbatim.
• Submit a patch for create_moves_for_unlinked_move_lines() to upstream, but I don't believe they're taking patches for Odoo 12.
• Put the patch in OCB (the upstream one, or our own fork).

Thoughts?

[robinkeunen]

Better late than never

auto_delete is currently set to True in the template. Is this a correct default?

class MailMail(models.Model):
    """ Model holding RFC2822 email messages to send. This model also provides
        facilities to queue and send new email messages.  """
    _name = 'mail.mail'
    ...
    auto_delete = fields.Boolean(
        'Auto Delete',
        help="Permanently delete this email after sending it, to save space")

Yes, we don't need to keep the history of these warning emails.

Is there anything I need to do for translations?

No, translations are managed by analysts and customers. From time to time, they ask us to bring back the terms they translated in the GUI to the VCS.

There's a chance that the code may not run correctly. Summarised below…

Copy the for-loop verbatim.

Unfortunatly, this is the way to go because :

Submit a patch for create_moves_for_unlinked_move_lines() to upstream, but I don't believe they're taking patches for Odoo 12.

Odoo will not indeed patch odoo 12, and

Put the patch in OCB (the upstream one, or our own fork).

OCB will only take fixes that were submitted to odoo/odoo but refused. This is not really a fix you're suggesting.

And we don't patch OCB ourselves, we prefer to keep open branches and merge them with gitaggregate.
It seems fragile that this module should depend on a patched version of our OCB.

But do put your PR comment in the comments of your def action_done

[carmenbianca]

@robinkeunen Would love a review on the refactor commit. (Never mind the ugly formatting; it's prettier's new look apparently).

[robinkeunen]

I have questions about the copied code bit.

Also, I'd name the module stock_picking_mail_unexpected_incorrect_qty

[carmenbianca]

Also, I'd name the module stock_picking_mail_unexpected_incorrect_qty

I think unexpected is superfluous here, if we generously interpret 'I received a pizza when I asked for salad' to be an incorrect delivery.

[carmenbianca]

TODO: Tidy up the commits. Remove the reverted commit entirely. Also run pre-commit again.

[codecov-commenter]

Codecov Report

Merging #192 (fabe203) into 12.0 (bd06e3d) will decrease coverage by 0.83%.
The diff coverage is 35.00%.

@@            Coverage Diff             @@
##             12.0     #192      +/-   ##
==========================================
- Coverage   69.98%   69.14%   -0.84%     
==========================================
  Files         124      126       +2     
  Lines        1639     1679      +40     
  Branches      318      333      +15     
==========================================
+ Hits         1147     1161      +14     
- Misses        458      484      +26     
  Partials       34       34              

Impacted Files	Coverage Δ
...picking_mail_incorrect_qty/models/stock_picking.py	33.33% <33.33%> (ø)
...tock_picking_mail_incorrect_qty/models/__init__.py	100.00% <100.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more