Removed dead code from admin and ember-admin - #29686
Merged
Merged
Conversation
no ref - traced every module for live importers before deleting: the empty src/index.tsx, the orphaned automations welcome-email chain, the posts-analytics feature-flag hook/HOC pair duplicating the canonical src/hooks/use-feature-flag, and five unused analytics components and hooks referenced by nothing or only by each other - use-edit-links was a property-renaming wrapper around useBulkEditLinks already flagged for removal in its own header, so its single caller uses the framework hook directly now - mock-analytics-apps became a no-op when the analytics apps moved into the React admin, so the helper and its imports and call sites go too - the tagsX and commentModeration labs flags have no remaining consumers in ember-admin, so their feature service declarations are dropped
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
WalkthroughObsolete Admin analytics hooks, components, utilities, and tests are removed. Newsletter analytics link editing now uses the bulk-edit mutation directly. Ember feature declarations replace two flags with three new flags. Ember acceptance tests stop configuring no-op analytics mocks, while setup authentication flows are centralized in helpers. Sender-detail implementation notes are updated to reference current counterpart modules. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run @tryghost/admin:test:acceptance |
✅ Succeeded | 5m 45s | View ↗ |
nx run-many -t test:unit -p @tryghost/admin-x-s... |
✅ Succeeded | 2m 32s | View ↗ |
nx run ghost-monorepo:lint:boundaries |
✅ Succeeded | 22s | View ↗ |
nx run-many -t lint -p @tryghost/admin-x-settin... |
✅ Succeeded | 1m 7s | View ↗ |
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | 1s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 21s | View ↗ |
nx run ghost-admin:test |
✅ Succeeded | 3s | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-07-30 14:39:35 UTC
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #29686 +/- ##
=======================================
Coverage 75.42% 75.42%
=======================================
Files 1608 1609 +1
Lines 141408 141448 +40
Branches 17471 17486 +15
=======================================
+ Hits 106654 106692 +38
+ Misses 33683 33682 -1
- Partials 1071 1074 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/admin/src/posts/analytics/Newsletter/newsletter.tsx (1)
150-159: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSend the trimmed URL to the mutation.
trimmedUrlis used for validation, but the request sends the originaleditedUrl. Leading or trailing whitespace can therefore be persisted in the link target.Proposed fix
void editLinks({ originalUrl: link.link.originalTo, - editedUrl: editedUrl, + editedUrl: trimmedUrl, postId: postId🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/admin/src/posts/analytics/Newsletter/newsletter.tsx` around lines 150 - 159, Update the editLinks call in the link-edit handler to pass trimmedUrl as editedUrl instead of the untrimmed editedUrl, while preserving the existing validation and early-return behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/admin/src/posts/analytics/Newsletter/newsletter.tsx`:
- Around line 150-159: Update the editLinks call in the link-edit handler to
pass trimmedUrl as editedUrl instead of the untrimmed editedUrl, while
preserving the existing validation and early-return behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 60e98068-9daa-4c67-b40c-7f6110876beb
📒 Files selected for processing (29)
apps/admin/src/analytics/hooks/use-labs-flag.tsapps/admin/src/analytics/views/Stats/components/post-menu.tsxapps/admin/src/analytics/views/Stats/components/section-header.tsxapps/admin/src/automations/hooks/use-welcome-email-sender-details.tsapps/admin/src/automations/utils/newsletter-emails.tsapps/admin/src/automations/utils/welcome-email-sender-details.tsapps/admin/src/index.tsxapps/admin/src/posts/analytics/Newsletter/newsletter.tsxapps/admin/src/posts/analytics/components/sidebar.tsxapps/admin/src/posts/analytics/hooks/use-edit-links.test.tsxapps/admin/src/posts/analytics/hooks/use-edit-links.tsapps/admin/src/posts/analytics/hooks/use-feature-flag.test.tsxapps/admin/src/posts/analytics/hooks/use-feature-flag.tsxapps/admin/src/posts/analytics/hooks/with-feature-flag.test.tsxapps/admin/src/posts/analytics/hooks/with-feature-flag.tsxapps/admin/src/posts/analytics/modals/share-modal.tsxapps/admin/src/whats-new/components/index.tsapps/ember-admin/app/services/feature.jsapps/ember-admin/tests/acceptance/authentication-test.jsapps/ember-admin/tests/acceptance/dashboard-test.jsapps/ember-admin/tests/acceptance/editor-test.jsapps/ember-admin/tests/acceptance/editor/post-settings-menu-test.jsapps/ember-admin/tests/acceptance/editor/publish-flow-test.jsapps/ember-admin/tests/acceptance/editor/visibility-test.jsapps/ember-admin/tests/acceptance/search-test.jsapps/ember-admin/tests/acceptance/setup-test.jsapps/ember-admin/tests/acceptance/signin-test.jsapps/ember-admin/tests/acceptance/signup-test.jsapps/ember-admin/tests/helpers/mock-analytics-apps.js
💤 Files with no reviewable changes (25)
- apps/admin/src/posts/analytics/components/sidebar.tsx
- apps/admin/src/posts/analytics/modals/share-modal.tsx
- apps/admin/src/posts/analytics/hooks/with-feature-flag.test.tsx
- apps/admin/src/automations/hooks/use-welcome-email-sender-details.ts
- apps/admin/src/posts/analytics/hooks/use-feature-flag.tsx
- apps/admin/src/posts/analytics/hooks/use-feature-flag.test.tsx
- apps/admin/src/posts/analytics/hooks/use-edit-links.ts
- apps/admin/src/whats-new/components/index.ts
- apps/admin/src/automations/utils/welcome-email-sender-details.ts
- apps/admin/src/posts/analytics/hooks/with-feature-flag.tsx
- apps/admin/src/posts/analytics/hooks/use-edit-links.test.tsx
- apps/admin/src/analytics/views/Stats/components/post-menu.tsx
- apps/ember-admin/tests/acceptance/signup-test.js
- apps/admin/src/automations/utils/newsletter-emails.ts
- apps/ember-admin/tests/acceptance/editor/visibility-test.js
- apps/admin/src/analytics/hooks/use-labs-flag.ts
- apps/ember-admin/tests/helpers/mock-analytics-apps.js
- apps/ember-admin/tests/acceptance/editor/publish-flow-test.js
- apps/ember-admin/app/services/feature.js
- apps/ember-admin/tests/acceptance/authentication-test.js
- apps/ember-admin/tests/acceptance/editor-test.js
- apps/ember-admin/tests/acceptance/dashboard-test.js
- apps/ember-admin/tests/acceptance/search-test.js
- apps/ember-admin/tests/acceptance/editor/post-settings-menu-test.js
- apps/ember-admin/tests/acceptance/signin-test.js
ref #29686 The dead-code cleanup left counterpart comments pointing to obsolete paths. Keeping the notes aligned preserves the cross-app maintenance contract.
9larsons
enabled auto-merge (squash)
July 30, 2026 14:33
This was referenced Jul 30, 2026
cmraible
added a commit
that referenced
this pull request
Aug 20, 2026
no refs ## Evidence that the component is unused `PostAnalyticsView` is not exported by the private Admin package, and repository-wide searches for both its symbol and module path find no importer. Admin-only Knip reported `apps/admin/src/posts/analytics/components/post-analytics-view.tsx` as an unused file before this removal and stopped reporting it afterward. The Admin build uses explicit routes and entry points; no dynamic import or glob references this path. ## History and intentionality Documented facts: - ad52284 introduced the component as the loading/empty/content wrapper in the first React post-analytics skeleton. #22978 explains that the work began a better breakdown of post statistics, initially focused on web analytics for one post. - 54d993b replaced that static wrapper in the main page with real post and traffic data. #22986 says this made the previously static skeleton usable for beta users. - 26de9ed later reused the component only as the loading screen for a duplicated `withFeatureFlag` wrapper. #23270 documents that the wrapper prevented the Newsletter tab from redirecting before the alpha setting had loaded, and explicitly called the duplication temporary. - #23029 documents the rollout model: `trafficAnalytics` selected beta users, while `trafficAnalyticsAlpha` selected features still under development for that beta. - f0a33b8 intentionally moved Newsletter analytics out of alpha. Its diff replaced the protected route with the direct `Newsletter` route and made the sidebar links unconditional; #23294 states that purpose directly. Production therefore stopped reaching the wrapper, and consequently `PostAnalyticsView`, on May 12, 2025. Given the documented flag semantics, this was an intentional promotion to the analytics beta rather than an accidental bypass. - fb0790b later restored the alpha flag for other design work but did not restore Newsletter's route guard, corroborating that Newsletter's promotion was deliberate. - 566f8a9 removed the now-unreachable wrapper and its tests. #29686 identifies those files as duplicated dead code with no live importers. That deletion removed the component's last source or test importer on July 30, 2026. Inference: leaving `PostAnalyticsView` behind in that last cleanup was an incomplete leaf cleanup. No historical source explicitly describes the omission, but the dead-code PR's stated intent, the current zero-reference result, and Knip's independent unused-file report support that conclusion. ## Removal Deleted the 33-line orphaned component. It had no dedicated tests, export, dependency, configuration, or documentation to remove. Its `EmptyStatView` and `PostAnalyticsContent` dependencies remain used by live analytics screens. ## Verification - `pnpm exec knip --workspace @tryghost/admin --include files --reporter compact --no-progress` (the removed file is absent; four pre-existing acceptance setup/config entries remain) - `pnpm --dir apps/admin lint` - `pnpm --dir apps/admin typecheck` - `pnpm --dir apps/admin test:unit` (135 files, 1,604 tests) - `pnpm nx run @tryghost/admin:build` - `pnpm nx run-many -t test --exclude @tryghost/e2e --exclude ghost-admin --exclude @tryghost/koenig-lexical` (35 projects, including 8,000 Ghost tests) `pnpm check` completed repository-wide lint and documentation checks, but its aggregate test phase could not run Koenig Lexical's browser suite in this environment: Chromium was denied Mach port registration and the configured Firefox executable is absent. The broader test run above excludes only that environment-blocked package and passed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Deletes modules in
apps/adminandapps/ember-adminthat have no live importers, plus two labs-flag declarations in the Ember feature service that nothing consumes.apps/admin/src/index.tsx— zero-byte file; the Vite entry issrc/main.tsxand no config or import references it.automations/hooks/use-welcome-email-sender-details.ts,automations/utils/welcome-email-sender-details.ts, andautomations/utils/newsletter-emails.tsonly imported each other; nothing else imports any of them. The live copy atautomations/components/email-modal/newsletter-emails.tsis untouched.posts/analytics/hooks/use-feature-flag.tsxandwith-feature-flag.tsx(plus their tests) duplicated the canonicalsrc/hooks/use-feature-flag.ts; every liveuseFeatureFlagimport already resolves to the canonical hook.analytics/hooks/use-labs-flag.ts,analytics/views/Stats/components/post-menu.tsx,analytics/views/Stats/components/section-header.tsx(already empty),posts/analytics/components/sidebar.tsx,posts/analytics/modals/share-modal.tsx, and the unused barrelwhats-new/components/index.ts. Zero references to each: the onboardingShareModalcomes from@tryghost/shade/patterns, and all whats-new imports go directly to the component files rather than the barrel.posts/analytics/hooks/use-edit-links.ts(plus test) — a property-renaming wrapper arounduseBulkEditLinkswhose own header comment says to remove it; its single caller (posts/analytics/Newsletter/newsletter.tsx) now callsuseBulkEditLinksfrom@tryghost/admin-x-framework/api/linksdirectly.apps/ember-admin/tests/helpers/mock-analytics-apps.js— reduced to two no-op functions when the analytics apps moved into the React admin; removed along with its imports and call sites in the 10 acceptance test files, droppingbeforeEach/afterEachhooks that would otherwise be empty.tagsXandcommentModerationdeclarations inapp/services/feature.jshave no consumers anywhere inapps/ember-admin; flag registration inghost/corelabs is untouched.Every deletion was verified with repo-wide greps for the module path/basename and exported symbols before removal.
Verification
cd apps/admin && pnpm lint— passcd apps/admin && pnpm test:unit— 97 files, 1074 tests, passcd apps/admin && pnpm typecheck— passpnpm nx run ghost-admin:test— pass