Skip to content

Removed unused tinybird filters#12

Open
tomerqodo wants to merge 6 commits intocoderabbit_full_base_removed_unused_tinybird_filters_pr12from
coderabbit_full_head_removed_unused_tinybird_filters_pr12
Open

Removed unused tinybird filters#12
tomerqodo wants to merge 6 commits intocoderabbit_full_base_removed_unused_tinybird_filters_pr12from
coderabbit_full_head_removed_unused_tinybird_filters_pr12

Conversation

@tomerqodo
Copy link
Copy Markdown

@tomerqodo tomerqodo commented Jan 29, 2026

Benchmark PR from agentic-review-benchmarks#12

Summary by CodeRabbit

  • Changes
    • Removed device, browser, and operating system filtering options from analytics endpoints
    • Removed dedicated analytics endpoints for displaying top browsers, devices, and operating systems
    • Simplified analytics queries to focus on location, pathname, source, and other metrics
    • Updated test coverage to reflect the simplified filtering capabilities

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 29, 2026

📝 Walkthrough

Walkthrough

This pull request removes device, browser, and operating system filtering capabilities from multiple Tinybird data pipeline endpoints and their corresponding test suites. The TinybirdService also removes three pipe references and modifies JWT token handling logic.

Changes

Cohort / File(s) Summary
Query Filter Removal
ghost/core/.../tinybird/endpoints/api_kpis.pipe, api_top_locations.pipe, api_top_pages.pipe
ghost/core/.../tinybird/pipes/filtered_sessions.pipe
Removed optional filter conditions for device, browser, and os from SQL WHERE clauses across multiple query endpoints. Remaining filters (location, pathname, post_uuid, source, member_status) remain intact.
Test Case Cleanup
ghost/core/.../tinybird/tests/api_kpis.yaml, api_top_locations.yaml, api_top_pages.yaml, api_top_sources.yaml, api_top_utm_campaigns.yaml, api_top_utm_contents.yaml, api_top_utm_mediums.yaml, api_top_utm_sources.yaml, api_top_utm_terms.yaml
Removed test scenarios for "Filtered by browser - Chrome", "Filtered by device - desktop", and "Filtered by OS - Windows" across all test files. Updated "Test with multiple filters combined" scenarios to use source and pathname parameters instead of device and browser filters, with corresponding expected results adjustments.
Service Configuration & Token Logic
ghost/core/.../services/tinybird/TinybirdService.js
Removed pipe references for api_top_browsers, api_top_devices, and api_top_os. Modified token generation to assign entire tokenData object instead of just tokenData.token. Changed JWT verification from jwt.verify() to jwt.decode() in token expiration check.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 No more devices, browsers, OS in sight,
The filters are gone, the pipes run light,
Tests simplified, the code grows lean,
A streamlined data flow, clean and keen,
Simplicity reigns with each deleted line! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Removed unused tinybird filters' accurately and concisely describes the main objective of the changeset: removing device, browser, and OS filters from multiple Tinybird pipe files and their corresponding tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
ghost/core/core/server/services/tinybird/TinybirdService.js (1)

96-104: Critical bug: _serverToken is assigned the entire tokenData object instead of the token string.

This change breaks the token handling:

  1. _generateToken() returns {token, exp} object
  2. this._serverToken = tokenData stores the entire object
  3. Line 102 returns token: this._serverToken which is now an object, not a string
  4. Line 96 passes the object to _isJWTExpired(), but jwt.decode() expects a string and will fail

The return value becomes {token: {token: "...", exp: ...}, exp: ...} instead of {token: "...", exp: ...}.

🐛 Proposed fix
 if (!this._serverToken || this._isJWTExpired(this._serverToken)) {
     const tokenData = this._generateToken({name, expiresInMinutes});
-    this._serverToken = tokenData;
+    this._serverToken = tokenData.token;
     this._serverTokenExp = tokenData.exp;
 }

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.

2 participants