🐛 Fixed degraded database performance when using the Post Analytics screen #22031
Conversation
This reverts commit 9082a9f.
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
ghost/admin/app/components/posts/analytics.jsOops! Something went wrong! :( ESLint: 8.44.0 Error: Failed to load parser '@babel/eslint-parser' declared in 'ghost/admin/.eslintrc.js': Cannot find module '@babel/eslint-parser'
WalkthroughThe pull request modifies the post analytics component in Ghost's admin interface by removing automatic data refresh functionality and introducing a manual refresh button. The changes eliminate the periodic auto-refresh mechanism and replace it with a user-triggered refresh option. The modification shifts the control of data refreshing from an automatic background process to an explicit user action. Changes
Sequence DiagramsequenceDiagram
participant User
participant PostAnalyticsComponent
participant DataService
User->>PostAnalyticsComponent: Click "Refresh" button
PostAnalyticsComponent->>DataService: Fetch post analytics
DataService-->>PostAnalyticsComponent: Return updated analytics data
PostAnalyticsComponent-->>User: Display refreshed data
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
cmraible
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ghost/admin/app/components/posts/analytics.hbs (1)
38-45: LGTM! Manual refresh button implementation looks good.The GhTaskButton implementation provides a clear way for users to manually refresh analytics data, replacing the auto-refresh functionality. The button includes appropriate visual feedback and maintains consistent styling.
Consider adding a tooltip to inform users about the manual refresh functionality, especially since this is a change in behavior from auto-refresh:
<GhTaskButton @buttonText="Refresh" @task={{this.fetchPostTask}} @showIcon={{true}} @idleIcon="reload" @successText="Refreshed" + @title="Click to refresh analytics data" @class="gh-btn gh-btn-icon refresh" @successClass="gh-btn gh-btn-icon refresh" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ghost/admin/app/components/posts/analytics.hbs(1 hunks)ghost/admin/app/components/posts/analytics.js(1 hunks)
🔇 Additional comments (1)
ghost/admin/app/components/posts/analytics.js (1)
6-6: LGTM! Import cleanup looks good.The import statement has been correctly updated to remove unused imports while maintaining required ones.
| @@ -3,7 +3,7 @@ import DeletePostModal from '../modals/delete-post'; | |||
| import PostSuccessModal from '../modal-post-success'; | |||
| import anime from 'animejs/lib/anime.es.js'; | |||
| import {action} from '@ember/object'; | |||
| import {didCancel, task, timeout} from 'ember-concurrency'; | |||
| import {didCancel, task} from 'ember-concurrency'; | |||
There was a problem hiding this comment.
💡 Codebase verification
Error handling needs improvement for manual refresh
The current implementation only handles task cancellation errors silently and lacks proper error handling for API requests. Since users will now manually trigger refreshes, failed requests should be properly communicated.
- Add proper error handling in
fetchPostTaskto show API errors to users - Add error handling for store queries in
fetchPostCountTask - Add error notifications for failed API requests in
_fetchReferrersStats
🔗 Analysis chain
Verify error handling for manual refresh.
Since we're moving from auto-refresh to manual refresh, ensure that error states are properly handled and communicated to users.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for error handling in fetchPostTask
ast-grep --pattern 'fetchPostTask() {
$$$
try {
$$$
} catch {
$$$
}
$$$
}'
# Check for error notifications
rg -A 5 'notifications\.showAPIError|notifications\.error' ghost/admin/app/components/posts/analytics.js
Length of output: 201
Script:
#!/bin/bash
# Find the task implementation
ast-grep --pattern 'fetchPostTask = task(function* () {
$$$
}'
# Look for alternative error handling patterns
rg -A 10 'catch\(' ghost/admin/app/components/posts/analytics.js
# Check for notification service injection
rg '@service notifications' ghost/admin/app/components/posts/analytics.js
# Look for any task error handling
rg -A 10 'task\(' ghost/admin/app/components/posts/analytics.js
Length of output: 303
Script:
#!/bin/bash
# Look for yield statements and task implementation
rg -A 10 'yield' ghost/admin/app/components/posts/analytics.js
# Check for notifications usage
rg 'this\.notifications' ghost/admin/app/components/posts/analytics.js
# Look for error-related code
rg -i 'error|fail|catch' ghost/admin/app/components/posts/analytics.js
Length of output: 3195
cmraible
changed the title
cmraible
changed the title
cmraible
changed the title
cmraible
changed the title
cmraible
changed the title
cmraible
changed the title
cmraible
changed the title
cmraible
deleted the
chris-onc-717-support-escalation-re-dashboard-unresponsive
branch
…creen (#22031) ref https://linear.app/ghost/issue/ONC-717/support-escalation-re-dashboard-unresponsive This reverts commit 9082a9f, which introduced an automatic refresh interval on the Post Analytics screen in Admin. This change led to an increase in the number of requests to the `/ghost/api/admin/members/events/` endpoint, which is a particularly database intensive endpoint. Ultimately this led to significantly higher load on the database which degraded performance for sites with a large `email_recipients` table.
…creen (#22031) ref https://linear.app/ghost/issue/ONC-717/support-escalation-re-dashboard-unresponsive This reverts commit 9082a9f, which introduced an automatic refresh interval on the Post Analytics screen in Admin. This change led to an increase in the number of requests to the `/ghost/api/admin/members/events/` endpoint, which is a particularly database intensive endpoint. Ultimately this led to significantly higher load on the database which degraded performance for sites with a large `email_recipients` table.
ref https://linear.app/ghost/issue/ONC-717/support-escalation-re-dashboard-unresponsive
This reverts commit 9082a9f, which introduced an automatic refresh interval on the Post Analytics screen in Admin. This change led to an increase in the number of requests to the
/ghost/api/admin/members/events/endpoint, which is a particularly database intensive endpoint. Ultimately this led to significantly higher load on the database which degraded performance for sites with a largeemail_recipientstable.