Skip to content

feat/traffic-analytics-v1.1#741

Open
ethanaturner wants to merge 8 commits intostagingfrom
feat/traffic-analytics-v1.1
Open

feat/traffic-analytics-v1.1#741
ethanaturner wants to merge 8 commits intostagingfrom
feat/traffic-analytics-v1.1

Conversation

@ethanaturner
Copy link
Copy Markdown
Member

  • Handle separated subdomains/sites in analytics service
  • Automate segment provisioning/syncing
  • UX improvement when book segment not processed yet
  • Exit server process after catching SIGHUP/SIGINT/SIGTERM to prevent hanging or force-kill

@ethanaturner ethanaturner changed the base branch from master to staging March 22, 2026 18:24
@jakeaturner jakeaturner force-pushed the feat/traffic-analytics-v1.1 branch from 9ec612a to d6f9540 Compare April 2, 2026 22:20
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds multi-site/subdomain support to the traffic analytics integration, introduces automated Matomo segment provisioning/syncing, improves the analytics UI when no data exists yet, and ensures the server exits cleanly on shutdown signals to avoid hanging.

Changes:

  • Introduce a new TrafficAnalytics service (with SSM-backed subdomain→site mapping) and remove the old single-site service.
  • Add an internal API endpoint to bulk-sync segments across all libraries/sites.
  • Add a client-side empty-state message for analytics, plus a helper to extract subdomains from book URLs.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
server/util/helpers.js Adds extractSubdomain(url) helper for deriving library subdomain from a URL.
server/types/TrafficAnalytics.ts Centralizes Traffic Analytics-related TypeScript types used by the service.
server/server.ts Exits the process after graceful shutdown to prevent signal-handling hangs.
server/api/traffic-analytics.ts Adds a new API handler to trigger bulk segment syncing.
server/api/services/traffic-analytics/traffic-analytics-ssm-client.ts Adds SSM client singleton to retrieve and cache subdomain→site ID mapping.
server/api/services/traffic-analytics/traffic-analytics-service.ts New multi-site traffic analytics + segment provisioning/syncing implementation.
server/api/services/traffic-analytics-service.ts Removes the old single-site TrafficAnalytics service implementation.
server/api/projects.js Updates Projects API to use the new TrafficAnalytics service path.
server/api.js Registers a new POST route for /traffic-analytics/sync-segments.
client/src/screens/conductor/Projects/Analytics/index.tsx Adds a “no data found” placeholder state once all queries return empty.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +75 to +84
const booksToCreate = (await Book.aggregate([
{
$match: {
$expr: { $not: '$trafficAnalyticsConfigured' },
library: {
$in: subdomainMatches,
},
},
}
])) as BookInterface[];
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

This aggregation $match uses $expr: { $not: '$trafficAnalyticsConfigured' }, which is not valid aggregation syntax for $not (it expects an expression array) and can cause the pipeline to error or mis-filter books. Use a standard field match (e.g., trafficAnalyticsConfigured: { $ne: true }) or a valid $expr form (e.g., { $expr: { $not: ["$trafficAnalyticsConfigured"] } }).

Copilot uses AI. Check for mistakes.
@jakeaturner
Copy link
Copy Markdown
Collaborator

@ethanaturner fixed a few things here but there is some comments from Copilot that I am not familiar enough with the analytics architecture to address

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants