Open thienvh332 opened 2 months ago
Hi @etobella, @simahawk, some modules you are maintaining are being modified, check this out!
More information would be awesome in order to understand why you did this.
@etobella this origins from our discussion on https://github.com/OCA/edi-framework/pull/39 @thienvh332 I'm adding the specs I gave you in the description feel free to adapt / add more things. IMO we should split the purchase part when ready (let's say before switching from draft to real PR to ease the final review of both parts).
@thienvh332 as it's draft, do you want us to wait for a review?
hi @simahawk
Hello @simahawk , This PR is ready for review. Could you please take a look at it?
can you drop the pypdf fix? https://github.com/OCA/edi/pull/1035/commits/15a994eb8a60d01d05202fbd637f04df81b0cef6
Hi @simahawk , I have updated the PR but I have 2 things I need your opinion on. Could you give me your views?
I don't get what's the need for this ... there's no
action_send_email
anywhere.
send_via_email
. There we don't know what ir_action
is used on each model. So I created it as a common path for other models. They can use _edi_send_via_email
without passing ir_action
parameter.type should be mandatory
type_id
must be mandatory:
send_via_email
and send_via_edi
). If type_id
is required then we will:
edi_exchange_type
and related models in edi_oca
module.2 common confs
in another module
to avoid creating unwanted data when installing the edi_oca
module.Hi @simahawk , I have updated the PR but I have 2 things I need your opinion on. Could you give me your views?
I don't get what's the need for this ... there's no
action_send_email
anywhere.* We have a common config **`send_via_email`**. There we don't know what **`ir_action`** is used on each model. So I created it as a common path for other models. They can use **`_edi_send_via_email`** without passing **`ir_action`** parameter. * Of course, if you feel it is inappropriate I will delete it.
The idea is that everything should be explicit. You must explicitly declare which action you want to use.
type should be mandatory
* About **`type_id`** must be mandatory: * I'm confused about this. There is a requirement to create 2 common confs (**`send_via_email`** and **`send_via_edi`**). If **`type_id`** is required then we will: * Create data for **`edi_exchange_type`** and related models in **`edi_oca`** module. * Create those **`2 common confs`** in **`another module`** to avoid creating unwanted data when installing the **`edi_oca`** module.
Ok, you are right. Cannot be mandatory. Good point :)
@etobella would you have time for a review?
Here's a full real life example https://github.com/OCA/edi/pull/1067 @thienvh332 pls have a look.
TODO here:
@thienvh332 as I want to speed up this and the modules that depend on it I'm taking this over. I will:
Hi @simahawk , Ok for me, thanks for the help
@etobella would you have time for a review?
Real life examples:
Specs
In edi_oca
Add generic
edi.configuration
model with these characteristics:Fields
This model can then be used by conf consumers. Other methods:
Backend / Exc. record
Then add edi.exchange.record.action_exchange_generate_send and use that method from backend. Consumer mixin
Specific fields for configuration (eg: edi_purchase_conf_ids) will be added by glue modules on res.partner.
Default conf
NOTE:
sending emails is not really bound to edi but in real life it's handy to have a default way to send EDI docs via email. You could even generate an exchange record to keep track of that but the choice will be up to the implementers. The key is that w/ this features they can do whatever they want.Views
On edi_purchase_oca
Example of specific listener implementation:
EDIT: additional model
edi.configuration.trigger
to ease UI mgmt.