Skip to content

[17.0][ADD]edi_notification_related_record_oca: configurable per-action related-record notifications - #317

Open
GuillemCForgeFlow wants to merge 2 commits into
OCA:17.0from
ForgeFlow:17.0-imp-reorg_exch_type_ui_toggle_notify
Open

GuillemCForgeFlow wants to merge 2 commits into
OCA:17.0from
ForgeFlow:17.0-imp-reorg_exch_type_ui_toggle_notify

Conversation

@GuillemCForgeFlow

@GuillemCForgeFlow GuillemCForgeFlow commented Jul 7, 2026

Copy link
Copy Markdown

Makes the chatter notes edi_oca posts on an exchange record's related records
configurable per exchange type and per action.

Why: every exchange logs a note on the business record when its data is generated and
again when it is sent — two near-redundant entries for a non-technical reader, and the ones
that matter end up buried. They stay useful while setting a flow up, so rather than
dropping them, which events are surfaced is left to configuration.

What

  1. [IMP] edi_oca_notify_related_record now receives the action the note comes
    from (generate/send/process/receive, or None for acknowledgements). It carries
    no behaviour in core; it is there so an override can decide per action.
  2. [ADD] edi_notification_related_record_oca — adds
    notify_related_record_on_{generate,send,process,receive} on edi.exchange.type,
    gated in a single override, with a Related Record Notifications page added to the
    form via inherit_id (no existing element is moved). All four default to True, so
    install changes nothing and no migration is needed. Unticking one suppresses the note
    only: the record still advances, keeps its state and errors, and still triggers events.

On the process toggle: deliberately not filtered by direction. exchange_process is
input-only, but an output record's "processed" note comes from the check component
resolved in edi_backend._exchange_output_check_state (e.g. edi_storage_oca polling a
remote done/error file), which also calls _notify_done/_notify_error.
Acknowledgements are not bound to an action and have no toggle (readme/ROADMAP.md).

Tests: the default note, each of generate/send/process disabled, that disabling one
action leaves the others posting, and that an action with no toggle still notifies.

cc @ForgeFlow

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hi @etobella, @simahawk,
some modules you are maintaining are being modified, check this out!

Comment thread edi_oca/models/edi_exchange_type.py Outdated
Comment thread edi_oca/models/edi_exchange_type.py Outdated
Comment on lines -42 to -65
<group name="config">
<field name="exchange_filename_pattern" />
<field name="exchange_filename_sequence_id" />
<field name="exchange_file_ext" />
<field name="exchange_file_auto_generate" />
<field name="ack_type_id" />
<field name="ack_for_type_ids" widget="many2many_tags" />
<field name="partner_ids" widget="many2many_tags" />
<field name="job_channel_id" />
<field name="quick_exec" />
<field name="encoding" />
<field
name="encoding_out_error_handler"
invisible="direction == 'input'"
/>
<field
name="encoding_in_error_handler"
invisible="direction == 'output'"
/>
<field
name="allow_empty_files_on_receive"
invisible="direction == 'output'"
/>
</group>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please don't do this: this architecture might be inherited and customized by third-party modules, and this change might break the view's inheritance mechanism (even though I personally agree a page in the notebook yields to a better UX).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I see, thanks for pointing out. what I've done now is still keep the name="config" attribute on the main group in the view. I understand there's still a chance that will can break current installations. do you think we should perhaps note this to be done in the next migration?

@GuillemCForgeFlow
GuillemCForgeFlow force-pushed the 17.0-imp-reorg_exch_type_ui_toggle_notify branch from 47cb50b to 0ce38b4 Compare July 10, 2026 07:36
@GuillemCForgeFlow

Copy link
Copy Markdown
Author

@simahawk any thoughts on this?

@simahawk simahawk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@GuillemCForgeFlow Thanks for your contrib!
I don't think this should stay core.
I would add a new module or add this to edi_notification_oca.

The related-record note is posted with no indication of which exchange
action it comes from, so a module wanting to filter those notes per
action has no way to tell them apart. Pass the originating action down,
leaving behaviour unchanged: edi_oca itself still notifies on every one.
Every exchange logged a note on the business record when its data was
generated and again when it was sent, two entries a non-technical reader
cannot act on and that bury the chatter entries which matter. Make each
of the four notes configurable per exchange type rather than dropping
them, since they are useful while a flow is being set up. All toggles
default to enabled, so installing the module changes no existing database.
@GuillemCForgeFlow
GuillemCForgeFlow force-pushed the 17.0-imp-reorg_exch_type_ui_toggle_notify branch from 0ce38b4 to 09bb480 Compare September 17, 2026 08:48
@OCA-git-bot OCA-git-bot added the mod:edi_notification_related_record_oca Module edi_notification_related_record_oca label Sep 17, 2026
@GuillemCForgeFlow GuillemCForgeFlow changed the title [17.0]edi_oca: configurable per-stage related-record notifications + form cleanup [17.0][ADD]edi_notification_related_record_oca: configurable per-action related-record notifications Sep 17, 2026
@GuillemCForgeFlow

Copy link
Copy Markdown
Author

@GuillemCForgeFlow Thanks for your contrib! I don't think this should stay core. I would add a new module or add this to edi_notification_oca.

@simahawk I've created a new module to handle the initial proposal, it lives now under a new one and not edi_notification_oca as I've seen those features unrelated to my changes. Please feel free to share your thoughts on this 🙂

@simahawk

Copy link
Copy Markdown
Contributor

@GuillemCForgeFlow thanks for your proposal.

AFAIU your final goal is to - for instance - avoid having a message "Exchange record generated" followed right away by a "Exchange record sent". Correct?

I'd like to summarize the use cases that we have here and in the current implementation of edi_notification.

UC

  1. be able to notify for failure certain groups and users (already in edi_notif)
  2. be able to turn on/off message posting for specific actions (what we have here)

Do you see any other use case?

@GuillemCForgeFlow

Copy link
Copy Markdown
Author

@GuillemCForgeFlow thanks for your proposal.

AFAIU your final goal is to - for instance - avoid having a message "Exchange record generated" followed right away by a "Exchange record sent". Correct?

I'd like to summarize the use cases that we have here and in the current implementation of edi_notification.

UC

  1. be able to notify for failure certain groups and users (already in edi_notif)
  2. be able to turn on/off message posting for specific actions (what we have here)

Do you see any other use case?

No, exactly as you described. the final goal is what drove me to have this new module implemented so that we can let the user manage the settings and have more granularity on what to show.

do you think that makes sense?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mod:edi_notification_related_record_oca Module edi_notification_related_record_oca mod:edi_oca Module edi_oca series:17.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants