[18.0][IMP] sign_oca: adding events for bus synchronization #76

kobros-tech · 2025-01-13T13:33:13Z

In this new feature, web client user can observe each sign request update in the status bar without needing to reload the page or even clicking on the pen icon.

Observe that event is sent only to scoped users who should sign on the new created sign request. and not for everybody.

OCA-git-bot · 2025-01-13T13:33:18Z

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

kobros-tech · 2025-01-13T13:41:10Z

@dnplkndll
@victoralmau
@ovnicraft
@pedrobaeza
@etobella

Hi guys, I need you to review this new feature, with which user can observe each sign request update in the status bar without needing to reload the page.

Observe that event is sent only to scoped users and not for everybody.

pedrobaeza · 2025-01-13T13:56:48Z

You should explain the new feature on the commit message and PR description, and if it makes sense in this module or in a separate one.

kobros-tech · 2025-01-13T14:02:25Z

You should explain the new feature on the commit message and PR description, and if it makes sense in this module or in a separate one.

@pedrobaeza
no worries this PR is not the new feature that I plan to add in a separate module.

The new one is to show sign requests to customers in the portal and to enable them to sign there.

For this PR feature, it is syncing changes instantly to backend users who are asked to sign and not everybody to save our servers from unnecessarily load.

pedrobaeza · 2025-01-13T14:10:11Z

OK, that explanation is the one it should go in the commit message. Adding 2 new contributors seems too much for this change, isn't it? Then, there are weird changes in .po files that are not coming from your improvement and should be avoided.

kobros-tech · 2025-01-13T14:16:17Z

OK, that explanation is the one it should go in the commit message. Adding 2 new contributors seems too much for this change, isn't it? Then, there are weird changes in .po files that are not coming from your improvement and should be avoided.

thanks, let me improve it all

kobros-tech · 2025-01-13T14:35:03Z

@pedrobaeza

I have just added the 3 improvements that you requested from me, I can see the code in the PR is more clean and descriptive.

victoralmau · 2025-01-14T15:30:13Z

Sorry, if I understood the enhancement correctly it should not be necessary to reload the page to update systray, but testing it does not work as expected.

Maybe I am forgetting something?

kobros-tech · 2025-01-14T15:58:54Z

Sorry, if I understood the enhancement correctly it should not be necessary to reload the page to update systray, but testing it does not work as expected.

Maybe I am forgetting something?

@victoralmau

all right I will debug this.

could you try to create sign request from button in the template views?

pedrobaeza · 2025-01-14T16:01:02Z

One thing is clear: a total reload of the single-page app shouldn't be done in any case.

kobros-tech · 2025-01-14T16:04:02Z

@victoralmau
I need you to try this on chrome and on firefox

with me chrome was not listening to bus events, don't know why

kobros-tech · 2025-01-14T16:04:41Z

One thing is clear: a total reload of the single-page app shouldn't be done in any case.

scenario is not to reload the page at all.

dnplkndll · 2025-01-14T16:15:56Z

One thing is clear: a total reload of the single-page app shouldn't be done in any case.

are there any other projects working with this concept of pushing the events to the clients over polling for changes? I was hoping to prove it out then move to a more generic approach as you would then be able to push render update events on much more expensive areas to re-render. or areas with a lot of subscribers

kobros-tech · 2025-01-14T16:16:38Z

@pedrobaeza

Can we figure out whether or no OCA boat is using workers > 0, if they are 0 nobody can use OCA boat for reviewing this PR and will be reviewed locally with --workers=1

pedrobaeza · 2025-01-14T16:18:37Z

@sbidoul should know if runboat can be run with multiple workers activating the bus.

dnplkndll · 2025-01-14T16:22:31Z

@pedrobaeza

Can we figure out whether or no OCA boat is using workers > 0, of they are 0 nobody can use OCA boat for review this PR and will be reviewed locally with --workers=1

https://github.com/sbidoul/runboat/blob/main/src/runboat/kubefiles/runboat-start.sh#L39

sbidoul · 2025-01-14T16:22:43Z

runboat runs in multithread mode (the default, --workers=0). Why is that a problem for this PR?

pedrobaeza · 2025-01-14T16:28:15Z

I think the bus doesn't work with multi-threaded mode.

kobros-tech · 2025-01-14T16:32:49Z

I think the bus doesn't work with multi-threaded mode.

workers should be 1 or more to be effective with this PR or any similar case.

sbidoul · 2025-01-14T16:37:53Z

I don't know what is special in this PR, but I tried chatting between two sessions on the same runboat database and it works, so I would assume the bus is functional on runboat.

I would say that if we have features that requires multi worker, this would need a big warning and/or something that prevent installing the module in multithread mode.

dnplkndll · 2025-01-14T16:43:05Z

since: odoo/odoo#191222
you would see a warning if not working right?

victoralmau · 2025-01-14T16:58:10Z

Perhaps the way it is used in mail.activity can be useful https://github.com/odoo/odoo/blob/81bf1bab9bed9aa0780601d69a1c89942d92b657/addons/mail/models/mail_activity.py#L308

kobros-tech · 2025-01-14T17:16:00Z

Perhaps the way it is used in mail.activity can be useful https://github.com/odoo/odoo/blob/81bf1bab9bed9aa0780601d69a1c89942d92b657/addons/mail/models/mail_activity.py#L308

In fact method used in the link is based on the method I used here, no lower level than what I provided.

I guess chatter is using bus as both methods should use bus module.

nilshamerlinck · 2025-03-23T13:07:42Z

sign_oca/models/sign_oca_request.py

+        partner_list_ids = partner_recs.mapped("id")
+        for partner_id in partner_list_ids:
+            self.env["bus.bus"]._sendone(
+                "broadcast",
+                f"sign_oca_request_updates_{partner_id}",
+                {"message": "Sign OCA Requests Model updated"},
+            )


Suggested change

partner_list_ids = partner_recs.mapped("id")

for partner_id in partner_list_ids:

self.env["bus.bus"]._sendone(

"broadcast",

f"sign_oca_request_updates_{partner_id}",

{"message": "Sign OCA Requests Model updated"},

)

partner_recs._bus_send(

"sign.oca.request/insert",

{}

)

better not send to broadcast channel or all users will receive a notification

you can use higher-level method _bus_send() method on partners (see here)

maybe sign.oca.request itself could implement bus.listener.mixin to cover more events type?

What I am doing here is a uniqe notification sent to it related partner and not everybody.
Only partners related to the created request are in scope.

If you find something else in review, refer to me, please?

by sending your notification to the broadcast channel, all clients will receive it

if you're not convinced, you can inspect messages received by the websocket worker here: chrome://inspect/#workers

of course you are filtering client-side, but server-side would be better (by using the already existing partner channel)

ok, it sounds interesting to check

nilshamerlinck · 2025-03-23T13:08:27Z

sign_oca/models/sign_oca_request.py

+            partner_recs = record.signer_ids.mapped("partner_id")
+            self.notify_changes(partner_recs)


Suggested change

partner_recs = record.signer_ids.mapped("partner_id")

self.notify_changes(partner_recs)

partner_recs = records.signer_ids.mapped("partner_id")

self.notify_changes(partner_recs)

@ nilshamerlinck
thanks, but there will be no change in code functionality or syntax.

if you create 2 sign.oca.request that involve same signers, isn't it better to send only one notification to each signer?

creating two requests does not make sense.
only one is needed to be fully signed but better if some partner is not there we send him an email again.

we made a PR on that.

@nilshamerlinck
#89

Currently it is possible to create multiple requests at the same time, for example, through maintenance_sign_oca (although it can be done without this module).

victoralmau

Functionally testing this one does not work. Example of another module that does something similar and works correctly: https://github.com/OCA/server-ux/blob/59205647d92f9c6c6ee955f1ae23d5f7b8b0409d/base_tier_validation/models/tier_validation.py#L784 + https://github.com/OCA/server-ux/blob/59205647d92f9c6c6ee955f1ae23d5f7b8b0409d/base_tier_validation/static/src/js/services/tier_review_service.esm.js#L14

kobros-tech · 2025-03-25T17:06:16Z

@victoralmau
The talk here is about sending notification to users of new docs to be signed if they are opening the system, this update can only work with bus port working like --workers=2 when running odoo-bin.

It is not related to creating requests, but it can not be reviewed using OCA boat so better to have a solution of sending notifications without workers app.

@nilshamerlinck gave me suggestion but I need a suggestion to use same port 8069 instead of 8073 of workers, by the way only important notificatios are sent via 8069 but ordinary ones use 8073 port.

victoralmau · 2025-03-26T07:16:38Z

@victoralmau The talk here is about sending notification to users of new docs to be signed if they are opening the system, this update can only work with bus port working like --workers=2 when running odoo-bin.

It is not related to creating requests, but it can not be reviewed using OCA boat so better to have a solution of sending notifications without workers app.

Sorry, but I don't understand the reason for having to configure what you indicate, in the previous examples (https://github.com/OCA/server-ux/blob/59205647d92f9c6c6ee955f1ae23d5f7b8b0409d/base_tier_validation/models/tier_validation.py#L784 + https://github.com/OCA/server-ux/blob/59205647d92f9c6c6ee955f1ae23d5f7b8b0409d/base_tier_validation/static/src/js/services/tier_review_service.esm.js#L14) it is not necessary to configure anything and it works correctly in oca boat for example.

kobros-tech · 2025-06-01T15:13:23Z

@victoralmau The talk here is about sending notification to users of new docs to be signed if they are opening the system, this update can only work with bus port working like --workers=2 when running odoo-bin.
It is not related to creating requests, but it can not be reviewed using OCA boat so better to have a solution of sending notifications without workers app.

Sorry, but I don't understand the reason for having to configure what you indicate, in the previous examples (https://github.com/OCA/server-ux/blob/59205647d92f9c6c6ee955f1ae23d5f7b8b0409d/base_tier_validation/models/tier_validation.py#L784 + https://github.com/OCA/server-ux/blob/59205647d92f9c6c6ee955f1ae23d5f7b8b0409d/base_tier_validation/static/src/js/services/tier_review_service.esm.js#L14) it is not necessary to configure anything and it works correctly in oca boat for example.

this also looks interesting, and I must check

kobros-tech · 2025-06-16T08:28:18Z

@nilshamerlinck
@victoralmau

your suggestions were useful to reduce code, it is ok to use one single common channel to send notification as the update will apply on the partners who should sign because large workers need large processors.

I can run the module when workers=1 but not less although I use the same method that you suggest it did not apply with workers=0

victoralmau

IMO it does not work as it should, the icon in the upper right corner with the total number of signatures pending is not updated (it is necessary to reload the page for it to work properly).

kobros-tech · 2025-06-23T11:12:02Z

IMO it does not work as it should, the icon in the upper right corner with the total number of signatures pending is not updated (it is necessary to reload the page for it to work properly).

Did you test it on run boat? It will not work on run boat unless there is a value for workers > 0.

I applied your suggestion although it needs a value for workers similar to the low level method I used before.

victoralmau · 2025-07-24T12:34:40Z

Yes, it may work in runboat. You can check base_tier_validation https://github.com/OCA/server-ux/tree/18.0/base_tier_validation (I initially indicated that module as an example where it is implemented and works correctly). although to check it functionally, you will have to do so with sale_tier_validation https://github.com/OCA/sale-workflow/tree/18.0/sale_tier_validation

dnplkndll · 2025-08-29T01:51:39Z

@kobros-tech did this feature with event update over refresh prove valuable? can is be used in other place or in a more general way?

kobros-tech · 2025-08-29T02:10:00Z

@kobros-tech did this feature with event update over refresh prove valuable? can is be used in other place or in a more general way?

if you plan to use same feature we can think to make a base module, but it needs smart implementation.

not sure but all what I have done I was not sure about before I start.

github-actions · 2025-12-28T12:39:37Z

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

kobros-tech force-pushed the 18.0-imp-sign_oca-1 branch from 29164fe to c4d315d Compare January 13, 2025 14:25

[18.0][IMP] sign_oca: adding events for bus synchronization

06f1a05

kobros-tech force-pushed the 18.0-imp-sign_oca-1 branch from c4d315d to 06f1a05 Compare January 13, 2025 14:28

nilshamerlinck suggested changes Mar 23, 2025

View reviewed changes

victoralmau suggested changes Mar 25, 2025

View reviewed changes

[IMP] sign_oca: use single channel

52df9f3

victoralmau suggested changes Jun 20, 2025

View reviewed changes

github-actions bot added the stale label Dec 28, 2025

		partner_recs = record.signer_ids.mapped("partner_id")
		self.notify_changes(partner_recs)

Uh oh!

[18.0][IMP] sign_oca: adding events for bus synchronization #76

Are you sure you want to change the base?

[18.0][IMP] sign_oca: adding events for bus synchronization #76

Conversation

kobros-tech commented Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

OCA-git-bot commented Jan 13, 2025

Uh oh!

kobros-tech commented Jan 13, 2025

Uh oh!

pedrobaeza commented Jan 13, 2025

Uh oh!

kobros-tech commented Jan 13, 2025

Uh oh!

pedrobaeza commented Jan 13, 2025

Uh oh!

kobros-tech commented Jan 13, 2025

Uh oh!

kobros-tech commented Jan 13, 2025

Uh oh!

victoralmau commented Jan 14, 2025

Uh oh!

kobros-tech commented Jan 14, 2025

Uh oh!

pedrobaeza commented Jan 14, 2025

Uh oh!

kobros-tech commented Jan 14, 2025

Uh oh!

kobros-tech commented Jan 14, 2025

Uh oh!

dnplkndll commented Jan 14, 2025

Uh oh!

kobros-tech commented Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pedrobaeza commented Jan 14, 2025

Uh oh!

dnplkndll commented Jan 14, 2025

Uh oh!

sbidoul commented Jan 14, 2025

Uh oh!

pedrobaeza commented Jan 14, 2025

Uh oh!

kobros-tech commented Jan 14, 2025

Uh oh!

sbidoul commented Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dnplkndll commented Jan 14, 2025

Uh oh!

victoralmau commented Jan 14, 2025

Uh oh!

kobros-tech commented Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kobros-tech Mar 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

kobros-tech commented Jan 13, 2025 •

edited

Loading

kobros-tech commented Jan 14, 2025 •

edited

Loading

sbidoul commented Jan 14, 2025 •

edited

Loading

kobros-tech commented Jan 14, 2025 •

edited

Loading

kobros-tech Mar 23, 2025 •

edited

Loading