Skip to content
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.

Sign up for GitHub

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][IMP] edi_account_oca: add new edi_account_move_exchange_type_id field on res.partner #39

Closed
wants to merge 1 commit into from
Closed

[16.0][IMP] edi_account_oca: add new edi_account_move_exchange_type_id field on res.partner #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions edi_account_oca/models/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
from . import account_move
from . import res_partner
15 changes: 15 additions & 0 deletions edi_account_oca/models/res_partner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Copyright 2023 Camptocamp SA
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl)

from odoo import fields, models


class ResPartner(models.Model):
_inherit = "res.partner"

edi_account_move_out_exchange_type_id = fields.Many2one(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But where are you using it? shouldn't you define it on a component or something like that? Also... I would define several exchange types and make a filter for it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this:

https://github.com/OCA/l10n-spain/blob/16.0/l10n_es_facturae_face/components/account_move_l10n_es_facturae_listener.py#L22-L43

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But where are you using it?

This is not relevant. You use it whenever you want to send out an invoice.

shouldn't you define it on a component or something like that?

Having an hardcoded type in the listener? That's not what you want.
Plus, you might want to customize the exchange type by partner.
This can also be the base to replace or integrate the transmit method module (which is doing kind of the same thing: define on the partner how you want to send invoices).

TBH I don't get what's so terrible in having an explicit configuration per partner to send out invoices :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is not relevant? I mean, if we are adding a field with no more interest than use it somewhere else, maybe we need to define it in the relevant module, isn't it? Also, if we use a similar component in two different modules to define the usage of this field, we might send the invoice twice, isn't it? IMO, we should define the field here if we define the logic, otherwise, I cannot find a justification.

Another though, in my experience, I have found some customers that require to send the information in two different ways. It might have sense to make it Many2many

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it might have sense to define a tuple: company, backend, exchange type, because we might use the same exchange type for several backend's, isn't it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we are adding a field with no more interest than use it somewhere else, maybe we need to define it in the relevant module, isn't it?

No. This is exactly what we do in base modules. This one IS a base module for EDI framework + invoice.
Hence, from my POV it makes perfectly sense to keep it here.

we might use the same exchange type for several backend's, isn't it?

IMO if you assign a type to a partner it must have a backend.

Regarding an advanced configuration: we should think properly about it. M2M won't be enough.
For a more granular thing we probably need an model like the type rule that would allow you to configure different behaviors. If we do it, we can also replace the transmit method stuff (which is only for sending).

However, this will take quite some time to implement. IMO for this basic scenario you can rely on this field and add a new one per each additional exchange you want to send (like in your case) or keep it hardcoded if you prefer.
Meanwhile, we can create an RFC and define specs and roadmap for this improvement.
We can also make a call - as usual - to clarify all the aspects.

Sounds good?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@simahawk Let me show you an example:

In spain we have a format for Electronic invoicing called Facturae and can be delivered in several ways FACe, FACeB2B, e.Fact.... We are reusing the same exchange type, because the format is exactly the same. For that reason, I was talking of exchange type and backend. This field could not be used in this case.

Also, some customers require to send the Facturae file in the standard way and another format to verify all the data with extra information. We know that customers usually like to manage it in the hard way. another problem arises, this standard field cannot be used, because we need to send in two different ways.

I was suggesting to add a table with 3 fields partner_id, exchange_type_id and backend_id
and a listener with a code like:

# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).

from odoo.addons.component.core import Component


class AccountMoveL10nEsFacturaeListener(Component):
    _name = "account.move.edi.listener"
    _inherit = "base.event.listener"
    _apply_on = ["account.move"]

    def on_post_account_move(self, records):
        for record in records:
            if record.edi_disable_auto:
                continue
            partner = record.partner_id
            # TODO: maybe we need to manage it in the table too...
            if record.move_type not in ["out_invoice", "out_refund"]:
                continue
           for configuration in partner.edi_account_configuration_ids:
               exchange_type = configuration.exchange_type_id
               backend = configuration.backend_id or configuration.exchange_type_id.backend_id
               if record._has_exchange_record(exchange_type, backend):
                    continue
                exchange_record = backend.create_record(
                    exchange_type.code, self._get_exchange_record_vals(record)
                )
                exchange_record.with_delay().action_exchange_generate()

I think it is not too hard.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrap up notes of our call...
We could have a generic edi.configuration model with these characteristics:

    description: describe what the conf is for
    backend_id
    type_id

    model_id: model the conf applies to
    res_id: id of the record it belongs to
        model_id an res_id will be set automatically in the specific form/ctx.
        The idea is that we can reuse configurations for other records (eg: apply the same conf to several customers)
        We'll have a m2m from consumer records rather than an o2m.

    target: Selection field to make easier and more explicit the use of the conf for a specific event (eg: on_post_account_move, on_so_confirm, etc)

    snippet_validate_todo: validate the state, used to collect records todo
        receives: operation, edi_action, vals, old_vals
        no need for triggers conf nor "if", "callable", etc, anymore
        the snippet can return
            todo = True/False
            snippet_do_vars = {}  # set of variables to pass over to the next snippet
            event_only
            tracked_fields
            .. in fact, any other variable that might be needed
            Then if we use `snippet_do` instead of assuming what has to be done... 
            all these vars become useless.

        NOTE: this makes sense only in the ctx of `on_create` and `on_write` because otherwise there's no need to collect records todo beforehand.
        You can simply use `snippet_do` otherwise.
        We could maybe encourage the use of specific targets instead in a help text.

    snippet_do: 
        Do something specific. 
        In the ctx of *_auto it can be used by `_edi_auto_handle` to execute what you want.
        Receives full EDIAutoInfo like obj (specific keys to be removed, add snippet_do_vars as attr)

Specific modules will then be able to define:

  • the hooks (listeners) and register new target options
  • specific fields for configuration (eg: edi_account_configuration_ids) so that we can show some conf only to specific groups

string="EDI Exchange type for outgoing invoices",
comodel_name="edi.exchange.type",
help="If defined, this EDI Exchange Type will be used to export invoices "
"of that customer.",
)
1 change: 1 addition & 0 deletions edi_account_oca/views/res_partner.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
<field name="arch" type="xml">
<group name="accounting_entries" position="after">
<group name="edi_configuration" string="EDI Configuration" />
<field name="edi_account_move_out_exchange_type_id" />
</group>
</field>
</record>
Expand Down
Loading