Skip to content

Fetch app alert data at intervals#5437

Merged
Cloud11PL merged 7 commits intomainfrom
PROD-102-fetch-data-at-intervals
Feb 27, 2025
Merged

Fetch app alert data at intervals#5437
Cloud11PL merged 7 commits intomainfrom
PROD-102-fetch-data-at-intervals

Conversation

@Cloud11PL
Copy link
Contributor

@Cloud11PL Cloud11PL commented Feb 25, 2025

Scope of the change

  • Fetches the webhook delivery info at regular intervals so that this heavy query is not overused

@changeset-bot
Copy link

changeset-bot bot commented Feb 25, 2025

🦋 Changeset detected

Latest commit: cbc7ff1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
saleor-dashboard Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codecov
Copy link

codecov bot commented Feb 25, 2025

Codecov Report

Attention: Patch coverage is 95.23810% with 2 lines in your changes missing coverage. Please review.

Project coverage is 62.16%. Comparing base (4e2b5fc) to head (cbc7ff1).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ps/components/AppAlerts/useAppsFailedDeliveries.ts 88.88% 1 Missing ⚠️
src/hooks/useIntervalActionWithState.ts 96.42% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5437      +/-   ##
==========================================
+ Coverage   62.11%   62.16%   +0.04%     
==========================================
  Files        1268     1269       +1     
  Lines       22008    22036      +28     
  Branches     4365     4370       +5     
==========================================
+ Hits        13671    13698      +27     
- Misses       8299     8300       +1     
  Partials       38       38              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Base automatically changed from PROD-101-dot-status-cache-metadata to main February 26, 2025 08:26
@Cloud11PL Cloud11PL force-pushed the PROD-102-fetch-data-at-intervals branch from aec3ff1 to 8efd3b0 Compare February 26, 2025 09:16
@github-actions github-actions bot temporarily deployed to pr-5437 February 26, 2025 09:19 Destroyed
@github-actions github-actions bot temporarily deployed to pr-5437 February 26, 2025 10:52 Destroyed
@Cloud11PL Cloud11PL added the run pw-e2e Run e2e (basic suite from PR automation) label Feb 26, 2025
@github-actions github-actions bot temporarily deployed to pr-5437 February 26, 2025 12:02 Destroyed
@github-actions
Copy link
Contributor

github-actions bot commented Feb 26, 2025

merge-reports: Run #4666

Tests 📝 Passed ✅ Failed ❌ Skipped ⏭️ Pending ⏳ Other ❓ Flaky 🍂 Duration ⏱️
235 235 0 0 0 0 0 5m16s

🎉 All tests passed!

Github Test Reporter by CTRF 💚

🔄 This comment has been updated

@github-actions github-actions bot temporarily deployed to pr-5437 February 26, 2025 12:19 Destroyed
@Cloud11PL Cloud11PL marked this pull request as ready for review February 26, 2025 12:24
@Cloud11PL Cloud11PL requested a review from a team February 26, 2025 12:24
@Cloud11PL Cloud11PL changed the title Fetch data at 5 minutes intervals Fetch app alert data at intervals Feb 26, 2025
andrzejewsky
andrzejewsky previously approved these changes Feb 26, 2025
poulch
poulch previously approved these changes Feb 26, 2025
fetchAppsWebhooks({
variables: {
canFetchAppEvents: hasManagedAppsPermission,
canFetchAppEvents: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

question: Don't you need permission check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's checked outside of this hook

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe would be good to add short comment about this

}

return cleanup;
}, [lastInvocation, action, interval, key, setLastInvocation, skip]);
Copy link
Member

Choose a reason for hiding this comment

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

issue(?): if we put action in useRef (I assumed it was to avoid re-renders) why do we put it into dependencies array? This will cause to start new timeout

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems you're right and I'll fix it soon. It wouldn't be a big deal since new timeout will have calculated delay since last execution of the action

@Cloud11PL Cloud11PL dismissed stale reviews from poulch and andrzejewsky via b8bed1c February 26, 2025 14:51
@github-actions github-actions bot temporarily deployed to pr-5437 February 26, 2025 14:55 Destroyed
@github-actions github-actions bot temporarily deployed to pr-5437 February 26, 2025 15:57 Destroyed
@Cloud11PL Cloud11PL merged commit c2f5824 into main Feb 27, 2025
17 checks passed
@Cloud11PL Cloud11PL deleted the PROD-102-fetch-data-at-intervals branch February 27, 2025 08:41
Cloud11PL added a commit that referenced this pull request Mar 6, 2025
* fetch data at intervals

* remove unnecessary Date.now()

* fix type issue

* add skip parameter and update tests

* remove action from deps array

* add comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run pw-e2e Run e2e (basic suite from PR automation)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants