-
Notifications
You must be signed in to change notification settings - Fork 324
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NAS-132124 / 24.10.3 / WidgetResourcesService should only fetch data once per load and reload when needed #11522
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## stable/electriceel #11522 +/- ##
=====================================================
Coverage ? 80.03%
=====================================================
Files ? 1574
Lines ? 51545
Branches ? 5948
=====================================================
Hits ? 41256
Misses ? 10289
Partials ? 0 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
attributes: { | ||
client_id: '332449661223-vlssrel0bhuasutipj1fg2f6in378h1i.apps.googleusercontent.com', | ||
client_secret: 'LIbFkoKL693tA_RvVAMLCKF_', | ||
token: '{"access_token": "ya29.a0AXeO80TX4NMZhEBr4ngVY2P2_MUDJ44d4Xp9Ji6pEhGYXhemyg2lFT3Trx1sicb01oudoV2i-LEnDLq9pRyaev7S0YOBKk8tV9AnGyQMWMMVTbb9T1IT5Kbc2qCgfJvkruu0U5X2avkmuYhZsBapLah3hBRPCLu58dAPNd8oaCgYKAdkSARESFQHGX2MikOZoQgFoQyQ5c07k_y2SSw0175", "token_type": "Bearer", "refresh_token": "1//01aDLaKfIzvsxCgYIARAAGAESNwF-L9Irlc7iPbMQkboB1xwRxVcI6kMneM_LXpLDmqwgKJYAenQWwqtf0U6Fx0vuzqvFuaUl54Y", "expiry": "2025-02-09T12:56:01.111180+00:00"}', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this used somewhere?
@@ -27,6 +31,7 @@ import { waitForSystemInfo } from 'app/store/system-info/system-info.selectors'; | |||
* 3. Use `toLoadingState` to provide widget with loading status. | |||
* 4. Use subscriptions when possible. | |||
*/ | |||
@UntilDestroy() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unnecessary since the service is a singleton.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some target branch other than stable/electriceel
needs to be selected.
Changes:
Previously, widget-resource-service was making calls to certain endpoints once per session and then ever updating the data no matter how much time passed and even if the user navigated away. I've changed that so the calls are remade whenever the user navigates to the dashboard page. Furthermore, I've optimized the calls so that multiple widgets asking for the same data only trigger one call. Instead of a new call for every widget.
Testing:
Code review. Also, check network logs to ensure that whenever you navigate to the dashboard page via either page refresh, or via navigating away to a different page and then coming back to the dashboard page, interface.query call is made only once and data is updated as per the latest call response. Also, check that every time the user navigates away from the dashboard page or refreshes the tab, the data for backup tasks is fetched and shown updated data on the UI.