Skip to content
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

IBX-5369: Fixed admin notifications request being queued too often #740

Merged
merged 4 commits into from
Mar 24, 2023

Conversation

Steveb-p
Copy link
Contributor

@Steveb-p Steveb-p commented Mar 21, 2023

Question Answer
Tickets https://issues.ibexa.co/browse/IBX-5369
Bug fix? yes (kinda)
New feature? no
BC breaks? no
Tests pass? yes
Doc needed? no
License GPL-2.0

This PR replaces setInterval function call with setTimeout.

This is done to prevent notification requests being queued regardless of the state of the previous request. For development environments, especially with multiple tabs open, this can cause local server to become overwhelmed by long-running requests, especially when clearing cache or performing other operations which require application container to be recompiled, like changing configuration.

Checklist:

  • Coding standards ($ composer fix-cs)
  • Ready for Code Review

@Steveb-p Steveb-p changed the base branch from main to 4.4 March 21, 2023 14:49
@github-advanced-security
Copy link

You have successfully added a new SonarCloud configuration ``. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab.

@@ -169,6 +169,10 @@

modal.querySelectorAll(SELECTOR_MODAL_RESULTS).forEach((link) => link.addEventListener('click', handleModalResultsClick, false));

getNotificationsStatus();
global.setInterval(getNotificationsStatus, INTERVAL);
const loop = function loop() {
Copy link
Contributor

@lucasOsti lucasOsti Mar 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the variable name loop is too generic

@tischsoic tischsoic changed the title Fixed admin notifications request being queued too often IBX-5369: Fixed admin notifications request being queued too often Mar 22, 2023
@tischsoic tischsoic force-pushed the fixed-notification-requests-queue branch from 0f3952f to c1d9f08 Compare March 22, 2023 12:52
@sonarcloud
Copy link

sonarcloud bot commented Mar 22, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jwibexa jwibexa self-assigned this Mar 23, 2023
Copy link
Contributor

@jwibexa jwibexa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and validated on 4.4 experience FF Chrome

@dew326 dew326 merged commit ba7a27f into 4.4 Mar 24, 2023
@dew326 dew326 deleted the fixed-notification-requests-queue branch March 24, 2023 11:15
@tischsoic
Copy link
Contributor

Merged up:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants