perf: Dont update list view data if list view not active #20396

ankush · 2023-03-20T10:33:16Z

Steps to reproduce:

visit a list view that's quite active
move to some other page
list view data keeps getting refreshed in background

Fix: Only refresh when user is back on list view

codecov · 2023-03-20T10:55:01Z

Codecov Report

Merging #20396 (025d514) into develop (44d4010) will increase coverage by 0.14%.
The diff coverage is 66.66%.

❗ Current head 025d514 differs from pull request most recent head 54d2d2c. Consider uploading reports for the commit 54d2d2c to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #20396      +/-   ##
===========================================
+ Coverage    63.82%   63.96%   +0.14%     
===========================================
  Files          758      757       -1     
  Lines        68620    68635      +15     
  Branches      6188     6190       +2     
===========================================
+ Hits         43796    43904     +108     
+ Misses       21296    21230      -66     
+ Partials      3528     3501      -27

Flag	Coverage Δ
server-ui	`31.69% <ø> (-0.03%)`	⬇️
ui-tests	`52.05% <66.66%> (+0.39%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

ankush · 2023-03-20T12:22:51Z

frappe/public/js/frappe/list/list_view.js

+		if (!cur_list || route[0] != "List" || cur_list.doctype != route[1]) {
+			// wait till user is back on list view before refreshing
+			return;
+		}


This might break when/if user comes back but no new event is received, perhaps we can just trigger this function again when list view is rendered to process pending refreshes.

it doesn't cause on every render it refreshes.

gavindsouza · 2023-03-20T13:38:58Z

I think we should un-sub/re-sub from/to updates on route change instead 🤔

We don't need the updates via Socket when we're not on the view as a refresh call (reportview.get) is triggered unconditionally on ListView render.

There's no APIs for unsubscribing at this point (it's just refreshing desk xD)

surajshetty3416 · 2023-03-20T15:21:34Z

There's no APIs for unsubscribing at this point (it's just refreshing desk xD)

You can do frappe.realtime.off("event-name", <function>) if you want to remove realtime listener.

Steps to reproduce: 1. visit a list view that's quite active 2. move to some other page 3. list view data keeps getting refreshed in background Fix: Only refresh when user is back on list view

ankush · 2023-03-21T08:41:52Z

will fix unsubscribing later... merging this for now to reduce self inflicted DDOS 😔

Steps to reproduce: 1. visit a list view that's quite active 2. move to some other page 3. list view data keeps getting refreshed in background Fix: Only refresh when user is back on list view (cherry picked from commit 7fc6ae6)

…0420) Steps to reproduce: 1. visit a list view that's quite active 2. move to some other page 3. list view data keeps getting refreshed in background Fix: Only refresh when user is back on list view (cherry picked from commit 7fc6ae6) Co-authored-by: Ankush Menat <ankush@frappe.io>

…0419) Steps to reproduce: 1. visit a list view that's quite active 2. move to some other page 3. list view data keeps getting refreshed in background Fix: Only refresh when user is back on list view (cherry picked from commit 7fc6ae6) Co-authored-by: Ankush Menat <ankush@frappe.io>

# [14.29.0](v14.28.2...v14.29.0) (2023-03-21) ### Bug Fixes * allow 5 column layout in doctype form ([7eb3bb4](7eb3bb4)) * Allow card actions (adding/dragging) if access to reference doctype ([bc5b362](bc5b362)) * Check perms on Kanban Column actions ([efa74bb](efa74bb)) * Check Reference Doctype perms & control indicator change ([c2fe3b0](c2fe3b0)) * clear contacts cache (backport [#20397](#20397)) ([#20406](#20406)) ([1905241](1905241)) * **Database:** clear background jobs and realtime logs on rollback ([#20236](#20236)) ([#20417](#20417)) ([750059b](750059b)) * Dispatch `update_order` always. Handle perm-wise action in backend ([e5c5cdb](e5c5cdb)) * handle image extraction while editing comment ([00c5f32](00c5f32)) * Hide Menu if empty & render columns without state change ([a9b5f04](a9b5f04)) * hide My Workspace sidebar section if empty in edit mode ([b51dd18](b51dd18)) * Hide Sort Selector in Kanban ([13be928](13be928)) * Kanban Board Menu Items Accessibility via perms ([4ffc7a5](4ffc7a5)) * **meta:** get_permitted_fields with no field-columns (backport [#20401](#20401)) ([#20405](#20405)) ([644c6c8](644c6c8)) * **patch:** move desk prop patch to post model sync ([#20361](#20361)) ([#20362](#20362)) ([661820a](661820a)) * **print:** Language set in document should have higher precedence (backport [#20336](#20336)) ([#20338](#20338)) ([a27178d](a27178d)) * server method to return evaluated dict of perms for a document ([d1446b8](d1446b8)) * Set link title in PDF ([716db05](716db05)) * sidebar becomes unhidden while removing skeleton ([71ac099](71ac099)) * skip InReadOnlyMode in error snapshots ([#20358](#20358)) ([#20359](#20359)) ([eb14fe1](eb14fe1)) * treat Phone as Data on list view ([06395dd](06395dd)) * **workspace:** Setup Dynamic Link if value exists ([b393d5b](b393d5b)) ### Features * clear integration log request logs ([#20373](#20373)) ([#20374](#20374)) ([4c48f55](4c48f55)) * configurable rounding methods (v14 port) ([#20330](#20330)) ([0d60385](0d60385)), closes [#20299](#20299) * **minor:** log datetime in worker log ([#20414](#20414)) ([#20418](#20418)) ([bfda74d](bfda74d)) ### Performance Improvements * Dont update list view data if list view not active ([#20396](#20396)) ([#20420](#20420)) ([3a6e307](3a6e307)) * rebuild website search index in background (backport [#17974](#17974)) ([#20335](#20335)) ([c1b0210](c1b0210))

## [13.51.2](v13.51.1...v13.51.2) (2023-03-21) ### Bug Fixes * **Database:** clear background jobs and realtime logs on rollback ([#20236](#20236)) ([#20416](#20416)) ([f4f399e](f4f399e)) * **meta:** get_permitted_fields with no field-columns (backport [#20401](#20401)) ([#20404](#20404)) ([d2be9d7](d2be9d7)) * **print:** Language set in document should have higher precedence (backport [#20336](#20336)) ([#20337](#20337)) ([2a84075](2a84075)) ### Performance Improvements * Dont update list view data if list view not active ([#20396](#20396)) ([#20419](#20419)) ([85cd2d2](85cd2d2))

ankush requested review from a team and shariquerik and removed request for a team March 20, 2023 10:33

ankush added backport version-13-hotfix backport version-14-hotfix backport to version 14 frappe-support labels Mar 20, 2023

ankush requested a review from gavindsouza March 20, 2023 11:37

ankush commented Mar 20, 2023

View reviewed changes

perf: Dont update list view data if list view not active

54d2d2c

Steps to reproduce: 1. visit a list view that's quite active 2. move to some other page 3. list view data keeps getting refreshed in background Fix: Only refresh when user is back on list view

ankush force-pushed the list_update_bg branch from 025d514 to 54d2d2c Compare March 21, 2023 06:39

ankush merged commit 7fc6ae6 into frappe:develop Mar 21, 2023

ankush deleted the list_update_bg branch March 21, 2023 08:41

This was referenced Mar 21, 2023

perf: Dont update list view data if list view not active (backport #20396) #20419

Merged

perf: Dont update list view data if list view not active (backport #20396) #20420

Merged

ankush mentioned this pull request Mar 21, 2023

perf: unsubscribe from list_update events #20423

Merged

github-actions bot locked as resolved and limited conversation to collaborators Apr 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf: Dont update list view data if list view not active #20396

perf: Dont update list view data if list view not active #20396

ankush commented Mar 20, 2023

codecov bot commented Mar 20, 2023 •

edited

Loading

ankush Mar 20, 2023

ankush Mar 21, 2023

gavindsouza commented Mar 20, 2023

surajshetty3416 commented Mar 20, 2023

ankush commented Mar 21, 2023 •

edited

Loading

perf: Dont update list view data if list view not active #20396

perf: Dont update list view data if list view not active #20396

Conversation

ankush commented Mar 20, 2023

codecov bot commented Mar 20, 2023 • edited Loading

Codecov Report

ankush Mar 20, 2023

Choose a reason for hiding this comment

ankush Mar 21, 2023

Choose a reason for hiding this comment

gavindsouza commented Mar 20, 2023

surajshetty3416 commented Mar 20, 2023

ankush commented Mar 21, 2023 • edited Loading

codecov bot commented Mar 20, 2023 •

edited

Loading

ankush commented Mar 21, 2023 •

edited

Loading