feat: format legacy plugins to new format #28017

MarconLP · 2025-01-29T09:55:26Z

Changes

export LegacyPlugin type at the bottom
replace console with logger.log
await meta.fetch instead of normal
make sure all tests work

status for all plugins: https://docs.google.com/spreadsheets/d/1K7Y5ZaefxP_mGl_q2ubjbFj6NybQT1DB0Q53CsEAMek/edit?usp=sharing

Does this work well for both Cloud and self-hosted?

How did you test this code?

…nto feat/cyclotron-plugins

# Conflicts: # plugin-server/src/cdp/consumers/__snapshots__/cdp-cyclotron-plugins-worker.test.ts.snap # plugin-server/src/cdp/consumers/cdp-cyclotron-plugins-worker.consumer.ts # plugin-server/src/cdp/consumers/cdp-cyclotron-plugins-worker.test.ts # plugin-server/src/cdp/consumers/cdp-cyclotron-worker.consumer.ts # plugin-server/src/cdp/legacy-plugins/customerio/index.ts # plugin-server/src/cdp/legacy-plugins/intercom/index.ts # plugin-server/src/cdp/legacy-plugins/types.ts # plugin-server/src/main/pluginsServer.ts # posthog/cdp/migrations.py # posthog/models/plugin.py

MarconLP · 2025-01-30T12:33:44Z

tests are succeeding locally after creating a test_cyclotron database

greptile-apps

PR Summary

This PR significantly restructures legacy plugins in the PostHog plugin server, focusing on standardizing plugin formats and improving type safety.

Removes multiple legacy plugins (currency-normalization, downsampling, taxonomy, timestamp-parser, etc.) while converting remaining plugins to TypeScript with proper type definitions
Updates remaining plugins (Hubspot, Braze, GCS, etc.) to use LegacyPlugin interface instead of Plugin type, with standardized logging via meta.logger
Replaces global fetch with meta.fetch across plugins and updates test files to use jest-fetch-mock
Consolidates plugin exports in src/cdp/legacy-plugins/index.ts with consistent plugin registration pattern
Adds comprehensive test coverage for converted plugins while removing tests for deprecated ones

_{116 file(s) reviewed, 43 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2025-01-30T12:34:57Z

plugin-server/src/cdp/legacy-plugins/currency-normalization/index.js

logic: Removing currency normalization functionality without a clear replacement could break existing integrations that depend on currency conversion. Ensure this functionality is either migrated elsewhere or that this is an intentional deprecation.

greptile-apps · 2025-01-30T12:35:46Z

plugin-server/src/cdp/legacy-plugins/downsampling/src/index.ts

logic: This file is being completely removed. Ensure there is a replacement implementation in the new plugin format that maintains the same sampling functionality and backwards compatibility.

greptile-apps · 2025-01-30T12:37:46Z

plugin-server/src/cdp/legacy-plugins/hubspot/index.ts

@@ -178,6 +179,7 @@ function getEmailFromEvent(event) {

 export const hubspotPlugin: LegacyPlugin = {
    id: 'hubspot',
-    onEvent,
+    metadata: metadata as any,


style: casting metadata as any bypasses type checking - consider defining proper type

greptile-apps · 2025-01-30T12:37:47Z

plugin-server/src/cdp/legacy-plugins/index.ts

 export const PLUGINS_BY_ID = {
    [customerioPlugin.id]: customerioPlugin,
    [intercomPlugin.id]: intercomPlugin,
+    [rudderstackPlugin.id]: rudderstackPlugin,
+    [hubspotPlugin.id]: hubspotPlugin,
+    [engagePlugin.id]: engagePlugin,
+    [avoPlugin.id]: avoPlugin,
+    [patternsPlugin.id]: patternsPlugin,
+    [brazePlugin.id]: brazePlugin,
+    [pubsubPlugin.id]: pubsubPlugin,
+    [sendgridPlugin.id]: sendgridPlugin,
+    [gcsPlugin.id]: gcsPlugin,
+    [salesforcePlugin.id]: salesforcePlugin,
+    [laudspeakerPlugin.id]: laudspeakerPlugin,
 }


style: Consider alphabetizing the plugin entries in PLUGINS_BY_ID for better maintainability and easier lookup

greptile-apps · 2025-01-30T12:37:47Z

plugin-server/src/cdp/legacy-plugins/index.ts

 import { customerioPlugin } from './customerio'
+import { hubspotPlugin } from './hubspot'
 import { intercomPlugin } from './intercom'
+import { avoPlugin } from './posthog-avo'
+import { brazePlugin } from './posthog-braze-app'
+import { engagePlugin } from './posthog-engage-so'
+import { gcsPlugin } from './posthog-gcs'
+import { laudspeakerPlugin } from './posthog-laudspeaker-app'
+import { patternsPlugin } from './posthog-patterns-app'
+import { pubsubPlugin } from './pubsub'
+import { rudderstackPlugin } from './rudderstack-posthog'
+import { salesforcePlugin } from './salesforce/src'
+import { sendgridPlugin } from './sendgrid'


style: Consider grouping imports by internal/external dependencies and alphabetizing within groups

greptile-apps · 2025-01-30T12:56:37Z

plugin-server/src/cdp/legacy-plugins/salesforce/src/index.ts

+    metadata: metadata as any,
+    setupPlugin: setupPlugin as any,
+    onEvent,


style: unsafe type assertions with 'as any' could be replaced with proper types

greptile-apps · 2025-01-30T12:56:38Z

plugin-server/src/cdp/legacy-plugins/salesforce/src/index.ts

        throw new Error(`Got bad response getting the token ${response.status}`)
    }
    const body = await response.json()
-    cache.set(CACHE_TOKEN, body.access_token, CACHE_TTL)
+    void cache.set(CACHE_TOKEN, body.access_token, CACHE_TTL)


style: void operator is unnecessary for Promise returned by cache.set

greptile-apps · 2025-01-30T12:56:54Z

plugin-server/src/cdp/legacy-plugins/salesforce/src/sendEventToSalesforce.test.ts

+                config,
+                global,
+                logger: { error: jest.fn(), debug: jest.fn() },
+                fetch: mockFetch as unknown,


style: mockFetch is already typed as jest.Mock, casting to unknown here is unnecessary and could hide type errors

greptile-apps · 2025-01-30T12:57:33Z

plugin-server/src/cdp/legacy-plugins/sendgrid/index.ts

+    metadata: metadata as any,
+    setupPlugin: setupPlugin as any,


style: Type assertions to 'any' bypass TypeScript's type checking. Consider defining proper types for metadata and setupPlugin

greptile-apps · 2025-01-30T12:57:33Z

plugin-server/src/cdp/legacy-plugins/sendgrid/index.ts

 function isEmail(email) {
-    const re = /^(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/
+    const re =
+        /^(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/
    return re.test(String(email).toLowerCase())
 }


style: Add type annotation for email parameter to maintain TypeScript consistency

plugin-server/src/cdp/legacy-plugins/_destinations/posthog-engage-so/test/index.test.js

+const { getMeta, resetMeta } = require('@posthog/plugin-scaffold/test/utils')
+const { engagePlugin } = require('../index')
+const config = {
+    publicKey: 'ENGAGE_PUBLIC_KEY',


plugin-server/src/cdp/legacy-plugins/_destinations/posthog-engage-so/test/index.test.js

+const { engagePlugin } = require('../index')
+const config = {
+    publicKey: 'ENGAGE_PUBLIC_KEY',
+    secret: 'ENGAGE_SEECRET',


plugin-server/src/cdp/legacy-plugins/_destinations/posthog-laudspeaker-app/index.ts

+}
+
+function result(target: any, path: any, value: any) {
+    target[path] = value


To fix the problem, we need to ensure that the set function does not allow the assignment of properties that can lead to prototype pollution, such as __proto__ and constructor. We can achieve this by adding a check to skip these properties during the assignment process.

Modify the set function to include a check that skips the __proto__ and constructor properties.

Ensure that the result function is only called if the property is not one of the dangerous properties.

plugin-server/src/cdp/legacy-plugins/_destinations/rudderstack-posthog/index.ts

+}
+
+function result(target: any, path: any, value: any) {
+    target[path] = value


To fix the problem, we need to ensure that the set function does not allow the assignment of properties like __proto__ or constructor that can lead to prototype pollution. We can achieve this by adding a check to skip these properties during the assignment process.

Modify the set function to include a check that skips the assignment if the property name is __proto__ or constructor.

This change should be made in the file plugin-server/src/cdp/legacy-plugins/_destinations/rudderstack-posthog/index.ts within the set function.

plugin-server/src/cdp/legacy-plugins/_destinations/salesforce/src/eventSinkMapping.test.ts

+                    global,
+                    logger: { error: jest.fn(), debug: jest.fn() },
+                } as unknown as SalesforceMeta,
+                'token'


plugin-server/src/cdp/legacy-plugins/_destinations/salesforce/src/sendEventToSalesforce.test.ts

+
+        expect(mockFetch).toHaveBeenCalledWith('https://test.salesforce.com/test', {
+            body: '{"$current_url":"https://home/io"}',
+            headers: { Authorization: 'Bearer token', 'Content-Type': 'application/json' },


plugin-server/src/cdp/legacy-plugins/_destinations/salesforce/src/sendEventToSalesforce.test.ts

+                properties: { $current_url: 'https://home/io' },
+            } as unknown as ProcessedPluginEvent,
+            { config, global, logger: { error: jest.fn(), debug: jest.fn() } } as unknown as SalesforceMeta,
+            'token'


plugin-server/src/cdp/legacy-plugins/_destinations/sendgrid/__tests__/index.js

+    expect(fetch).toHaveBeenCalledWith('https://api.sendgrid.com/v3/marketing/field_definitions', {
+        method: 'GET',
+        headers: {
+            Authorization: 'Bearer SENDGRID_API_KEY',


plugin-server/src/cdp/legacy-plugins/_destinations/sendgrid/__tests__/index.js

+    expect(fetch).toHaveBeenCalledWith('https://api.sendgrid.com/v3/marketing/contacts', {
+        method: 'PUT',
+        headers: {
+            Authorization: 'Bearer SENDGRID_API_KEY',


plugin-server/src/cdp/legacy-plugins/_destinations/sendgrid/__tests__/index.js

+    expect(fetch).toHaveBeenCalledWith('https://api.sendgrid.com/v3/marketing/contacts', {
+        method: 'PUT',
+        headers: {
+            Authorization: 'Bearer SENDGRID_API_KEY',


benjackwhite and others added 30 commits January 23, 2025 16:13

added initial work

7e02fc8

Fix up new plugins

99e5b72

Fixes

52792bf

Fixes

dfb9e0d

Fixes

6c2575a

Fix

f7db6e5

Fixes

835d8fb

Fixes

dcc8b2f

Fix

18ef79b

Fixes

9317b33

Fix

e47eb3c

Added check for metrics production

365c1b8

fixed frontend

20a304c

Fixes

693bf59

Fix up

4e8267d

Fix

8d2f00b

Update query snapshots

e3a1ca9

Update UI snapshots for chromium (1)

9694f07

Fix storage

e33dd64

Update query snapshots

b7781cf

Merge branch 'feat/cyclotron-plugins' of github.com:PostHog/posthog i…

9895d15

…nto feat/cyclotron-plugins

Fixes

9001d92

Fixes

1bbda52

Fixes

d573b20

Update UI snapshots for chromium (1)

d054739

USe own types

2ab4312

Added all production plugins

c2c1d55

Remove files

71ca8a5

Fix

d28c1a7

Fix

cbeb38c

benjackwhite and others added 15 commits January 28, 2025 09:44

Merge branch 'feat/cyclotron-plugins' into feat/cyclotron-plugins-all

98e4f5c

fix

be04729

Merge branch 'feat/cyclotron-plugins' into feat/cyclotron-plugins-all

6890e64

format legacy plugins to new format

3c6c2a7

add laudspeaker plugin

e9a178a

add plugin.json

5430465

fix tests

5af915b

run prettier

0c541a8

fix braze tests

8723441

fix pace tests

ce78979

run prettier

cf6ec96

ignore ts errors

c2607a6

remove transformations

b2e92e6

fix lint errors

b756893

MarconLP marked this pull request as ready for review January 30, 2025 12:33

MarconLP requested review from benjackwhite and removed request for benjackwhite January 30, 2025 12:33

update test snap

0f92c48

greptile-apps bot reviewed Jan 30, 2025

View reviewed changes

MarconLP added 2 commits January 30, 2025 14:29

fix typescript errors

27e12ab

fix additional tests and add replicator plugin

095ea95

MarconLP requested a review from benjackwhite January 31, 2025 10:11

benjackwhite added 2 commits January 31, 2025 13:04

Merge branch 'master' into feat/cyclotron-plugins-all

e5a8082

Merge

734cead

benjackwhite changed the base branch from feat/cyclotron-plugins-all to master January 31, 2025 12:08

benjackwhite added 2 commits January 31, 2025 13:11

Fixes

36e8087

Fixes

776e99a

github-advanced-security bot found potential problems Jan 31, 2025

View reviewed changes

@@ -91,2 +91,6 @@
+                    if (prop === '__proto__' || prop === 'constructor') {
+                        continue
+                    }
                     if (!isObject(target[prop])) {

@@ -90,2 +90,6 @@
+                    if (prop === '__proto__' || prop === 'constructor') {
+                        break
+                    }
                     if (!isObject(target[prop])) {

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: format legacy plugins to new format #28017

feat: format legacy plugins to new format #28017

MarconLP commented Jan 29, 2025 •

edited

Loading

MarconLP commented Jan 30, 2025

greptile-apps bot left a comment

greptile-apps bot Jan 30, 2025

greptile-apps bot Jan 30, 2025

greptile-apps bot Jan 30, 2025

greptile-apps bot Jan 30, 2025

greptile-apps bot Jan 30, 2025

greptile-apps bot Jan 30, 2025

greptile-apps bot Jan 30, 2025

greptile-apps bot Jan 30, 2025

greptile-apps bot Jan 30, 2025

greptile-apps bot Jan 30, 2025

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

feat: format legacy plugins to new format #28017

Are you sure you want to change the base?

feat: format legacy plugins to new format #28017

Conversation

MarconLP commented Jan 29, 2025 • edited Loading

Changes

Does this work well for both Cloud and self-hosted?

How did you test this code?

MarconLP commented Jan 30, 2025

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Jan 30, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 30, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 30, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 30, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 30, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 30, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 30, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 30, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 30, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 30, 2025

Choose a reason for hiding this comment

MarconLP commented Jan 29, 2025 •

edited

Loading