OCA / web

Odoo web client UI related addons
GNU Affero General Public License v3.0
912 stars 1.86k forks source link

[16.0] web_refresher: add ability to auto-refresh and make it configurable from view action #2748

Closed colpari closed 6 months ago

colpari commented 6 months ago

Hi there, this is a proposal for merging the auto-refresh feature as we use it. It reads the view action in onMounted() to be configurable from the view.

If this is not the proper way to implement it, we're happy to enhance it :)

pedrobaeza commented 6 months ago

Do you mean to not enable it if specified in the act_window?

colpari commented 6 months ago

@pedrobaeza : it's disabled by default but by putting the key 'web_refresher_autorefresh' into the action context it will auto-enable on load with the value of web_refresher_autorefresh as interval in seconds, if that is a number. 30 Seconds otherwise

We're open to implement more options and also to maybe backport it to 15.0 and 14.0 with some guidance.

We're new to OCA repositories - shall we do something about the failing linting checks? From the messages we're not sure what it expects. Apply the diff in the output?

pedrobaeza commented 6 months ago

You shouldn't do that in the current module, as you are modifying the previous behavior, where all views have the refresh button.

I don't get what is the motivation for not showing always the button? It's under demand, so no performance problem, and the icon gets very few space. Can you explain?

In any case, you should add this as an extra module for those wanting to opt-in in this way to act.

colpari commented 6 months ago

@pedrobaeza : The existing behavior of the button is unchanged and it is always shown, as before.

We added another checkbox next to the button plus optional behaviour.

image

If, and only if, you check that checkbox (or configure so in the action context), then the refresh is done automatically in certain intervals.

Indeed an additional module might be nice, but currently we don't have the time and the know-how to lay this out as an extra module. Took us hours already just to figure out how to access the action's context in the loading-phase ;-) The sparse documentation on OWL does not give us a clue how to extend an existing component's XML from another module, for example.

If somebody could provide us with pointers to documentation or examples where exactly this is done, or can formulate precise hints on what to do, we'll try to improve it.

Otherwise it's also totally ok to just ditch this pull request if nobody can support us or want's to adopt it :-)

pedrobaeza commented 6 months ago

OK, understood now the goal. Well, that auto-refresh option is something that users demand, but it comes with a very high price of resources leak: imagine 50 users with 5 tabs opened with that auto-refresh thing every minute: you are adding 250 queries (that can be heavies ones) without real value. You can add a mechanism for not refreshing while the focus is not on the tab, but apart from complicating the implementation, you still have 50 queries...

I instruct my users about refreshing on demand, and they assume the "limitation" without too many problems after they get used to.

colpari commented 6 months ago

@pedrobaeza : Yes of course that can be a waste of resources. In our opinion that decision should be left to the operators. We have a bunch of odoo's running. Some are so big (data and users) that i never would offer this feature there.

But on other instances we have small teams that really need a near-real-time overview to react to events without having to refresh manually. We use it for 1 or two views in the whole installation.

What we would like best is if we could measure the time it takes to reload the view. Any idea how to do this? Then one could set a sensible lower bound on the refresh-interval. Say for example minimum refresh interval is 1000 x reload-time. Then you can be assured that if you have 10 users and the are all on auto-refresh, you never get more than 1% sysload from it.

pedrobaeza commented 6 months ago

On the "Network" tab of your browse you can see the requests done and the time it takes, so you can measure manually checking it. Let's include the feature opt-in. Maybe the lonely check is a bit rude. Can you add an icon next to it to represent the feature, and also document how to put it on the README?

colpari commented 6 months ago

Well of course we can measure time of one single request in the browser but what we meant was measuring every refresh request to automatically scale down auto-refresh if the server get's too busy or an unexpected amount of people activates the feature.

We were hoping for some help from more experienced odoo frontend developers here but getting a second opinion trough the discussion here was already very helpful, thanks! :)

We decided to close this PR for now and see if we can find the time to polish it ourselves.

We'll maybe come back later with a version in a separate module which can also auto-adapt to loading times to ensure a configurable upper bound on server load generated by this feature.

pedrobaeza commented 6 months ago

Ok, hope to see you around in more occasions :)