On tab stale event, don't refresh tabs which are queryOnLoad but are not the active tab

[teosarca]

Type of issue

Bug

Steps to reproduce / Expected behavior

• open a role, e.g. https://w101.metasfresh.com:8443/window/111/1000056
• click on some tabs => remark when tab is activated, frontend asks for tab data, so those are queryOnLoad tabs.
• Check/Uncheck the "Active" flag, to provoke tab stale events
  In network tab, take a look at what happened.
• frontend PATCHED the document and as a response he got tab stale events.
• frontend queried all of them => NOK: it's pointless and inefficient because as soon as you will click on a tab the tab will be queried again.

This is a follow-up of #769

[damianprzygodzki]

@teosarca we were thinking about the solution, that in the moment of update (patch response processing or modal closing) we have no information (or access is really expensive) if tab is queried on activate (update relays on data, queryOnActivate is the case of layout), what do you say if you would send us, exactly that tabs that should be marked as stale?

[teosarca]

hmmm, let me think about it... but sounds good.

[cadavre]

Waiting for API – i.e. send only

exactly that tabs that should be marked as stale

[teosarca]

UPDATE:
I thought about this topic and I think the backend shall notify about ALL tabs that become stale.

Reason:
Imagine your current active tab has queryOnLoad.
Frontend patches the header document and let's suppose that tab becomes stale.
If the backend would not send this information (because the tab is queryOnLoad), the frontend would have no idea that it needs to refresh that tab right away (because it's active).

wdyt?

[damianprzygodzki]

In my opinion, we want to make optimization that in total cost is less efficient than before.

QueryOnActivate is part of layout, passing it to protocol GENERATING PATCH -> PROCESS RESPONSE is very antipattern.

If we want to make patch response responsible for such a things like managing window layout and data, refreshing tabs and so on, we have to rebuild our protocol, create some sort of service that is responsible for responses processing, it has to live at low level of application, and it cant be static, because we have to be connected to model.

Still i dont know that it is good idea. We have really lightweight solution now, and i want to keep it as it is for performance and pattern.

Btw. refreshing tabs should be made the same as it is for Document List updating by WS? Patch response is update (originally) for the document that was origin of that change.

[cadavre]

What I believe Teo has in mind is that only front-end knows which tab is currently opened – this way QueryOnActivate/queryOnLoad can be considered to refresh or not and decision is on front-end side. I'm not aware of implementation though, just trying to find logical explanation.

I would be +1 for utilizing WS since we have it in the system...

[teosarca]

I would be +1 for utilizing WS since we have it in the system...

(Was also considering WS, but i think that's another topic because then we need to think and implement a WS endpoint for each document.. else it would be to much noise....)

But even with the WS, the requirement stays: if u got a tab stale notification from websocket and that tab is QueryOnActivate/queryOnLoad and it's not active then don't refresh it.

[teosarca]

In my opinion, we want to make optimization that in total cost is less efficient than before.

Not so sure about that. Precisely for a case like the one that i've described in the issue summary,
e.g. https://w101.metasfresh.com:8443/window/111/1000055 ,

how much we would save in terms on time and bandwidth it's not so neglectable i would say.
take a look:

[damianprzygodzki]

But even with the WS, the requirement stays: if u got a tab stale notification from websocket and that tab is QueryOnActivate/queryOnLoad and it's not active then don't refresh it.

But there is no problem. WS are connected strictly to component. Responses from patch not.

[teosarca]

Btw. refreshing tabs should be made the same as it is for Document List updating by WS? Patch response is update (originally) for the document that was origin of that change.

Agree, but when we will have the "one WS endpoint for each document" feature implemented.
I think that's not a feature we could sneak in as a part of a fix/optimization.

[damianprzygodzki]

Do you want to rebuild important and big node in whole structure in front-end, instead of improving it by WS? I think that with these "quickfixes" we are breaking separation of concerns that we tried hard to keep.

[teosarca]

let's talk when you have time. I don't see it so dramatic.

[cadavre]

After discussions, we have two options, both would probably take same amount of time.

1. Make front-end implementation which would require refactor of tab components to achieve refreshing of currently active tab.
2. Utilize WS to inform about windows/documents/tabs that has changed in-the-background. This is currently used for gridviews and – as agreed – this is way to do in future.

Both implementations are not small, both are "not a feature we could sneak in as a part of a fix/optimization" actually. :)

Of we go with 1st option – we will make things two times, because finally we all want to use WS. :)

[cadavre]

@teosarca please provide some info about future implementation on agreed WS.

[teosarca]

API provided and documented here: metasfresh/metasfresh-webui-api-legacy#435 (comment)

[teosarca]

API ready and rolled out on w101

[metas-rc]

Results of IT

Tested on webui

• Went to a Role enty, opened it in single view
• => OK: Only the first internal tab was refreshed and displayed in the inspection
• Clicked on another tab
• => OK: Only the tab I clicked on was refreshed and displayed in inspection
• Deactivated / Activated the record
• => OK: Only the internal tab I selected was refreshed and displayed in the inspection