Conversation
etobella
force-pushed
the
19.0-mig-mail
branch
from
c598e9c to
9b4f48e
Compare
etobella
force-pushed
the
19.0-mig-mail
branch
from
9b4f48e to
a88067b
Compare
simahawk
left a comment
simahawk
left a comment
There was a problem hiding this comment.
Interesting approach! Thanks!
|
|
||
| class EdiExchangeType(models.Model): | ||
| _name = "edi.exchange.type" | ||
| _inherit = ["edi.exchange.type", "mail.alias.mixin"] |
There was a problem hiding this comment.
Thinking out loud: instead of polluting the exchange type model, wouldn't be interesting to have a specific record for this? Eg: "EDI mail gateway" or something similar.
Easier to maintain and to configure for the end user probably. WDYT?
There was a problem hiding this comment.
Or maybe using edi.backend. At the end, that is the goal of backends, isn't it?
etobella
force-pushed
the
19.0-mig-mail
branch
from
c4338c9 to
c52a31e
Compare
Remove unnecessary sudo
etobella
force-pushed
the
19.0-mig-mail
branch
from
c52a31e to
12991dd
Compare
etobella
force-pushed
the
19.0-mig-mail
branch
from
8b6c81b to
82e92fb
Compare
|
@simahawk After your comments, I do agree that the best approach would be to set everything on the backend and do the same thing we do with Storage. Also, I improved REadmes and information inside the backend to make it easier to handle (actually, it would be nice to include similar information in other modules to make it easier for users. Thanks for your comments, IMO they improved the solution. 
|
| <label | ||
| for="alias_name" | ||
| class="fw-bold o_form_label" | ||
| string="Create tasks by sending an email to" |
There was a problem hiding this comment.
| string="Create tasks by sending an email to" | |
| string="Create EDI records by sending an email to" |
| return values | ||
|
|
||
| def _mail_exchange_type_pending_input_domain(self): | ||
| """Domain for retrieving input exchange types for emails.""" |
There was a problem hiding this comment.
I would add some more conventional constraint here.
What if we add specific type of backend Email and we rely on the fact that exc types refer to that type.
I would make it more clear from the configuration point of view.
| not custom_values.get("type_id") | ||
| and not backend.full_mail_exchange_type_id | ||
| ): | ||
| types = self.env["edi.exchange.type"].search( |
There was a problem hiding this comment.
if we rely on the fact that we can have multiple files in an email and multiple types configured, we should support multiple creation of records.
Hence, here we should collect all new record values and call super on each set of values at the end.
Also, each record should be related to a single parent record (conventionally I would use the 1st one).
| custom_values["type_id"] = exchange_type.id | ||
| break | ||
| else: | ||
| if backend.full_mail_exchange_type_id: |
There was a problem hiding this comment.
If I get it right you want the possibility to just store the full content of the email. Correct?
If that's the case, I would move this configuration on the type itself.
Something like:
mail_record_policy = fields.Selection(
[
("full", "Only archive the full mail message"),
("pattern", "Match single filename pattern"),
]
)
This field will be visible only for the backend type email and will command how this record creation will work.
As for the full json storage: IMO attachments should not be incapsulated into the json data.
We should store them as attachments of the exchange record.
WDYT?
| content = json.dumps(new_message_dict) | ||
| filename = "email_message.json" | ||
| if not custom_values or "type_id" not in custom_values: | ||
| _logger.warning( |
There was a problem hiding this comment.
is there a need for this logging if you raise an error?
2 participants
Depends on #215