Conversation
plugin.json
Outdated
| "hint": "Group type that you want to enrich with hubspot companies", | ||
| "name": "Company Group Type", | ||
| "type": "string", | ||
| "default": null, |
There was a problem hiding this comment.
if type is string let's give this a string default
| "default": null, | |
| "default": "", |
plugin.json
Outdated
| "hint": "Group type that you want to enrich with hubspot deals", | ||
| "name": "Deal Group Type", | ||
| "type": "string", | ||
| "default": null, |
index.js
Outdated
| global.hubspotAuth | ||
| }&properties=${properties.join(',')}` | ||
| } | ||
| const authResponse = await fetchWithRetry(requestUrl) |
There was a problem hiding this comment.
nit: authResponse implies to me we're just testing auth, not fetching important data
index.js
Outdated
| } | ||
| if (res && res['results']) { | ||
| for (hubspotDeal of res['results']) { | ||
| exists = await storage.get(hubspotDeal['id'], false) |
There was a problem hiding this comment.
| exists = await storage.get(hubspotDeal['id'], false) | |
| const exists = await storage.get(hubspotDeal['id'], false) |
index.js
Outdated
| if (res && res['results']) { | ||
| for (hubspotDeal of res['results']) { | ||
| exists = await storage.get(hubspotDeal['id'], false) | ||
| if (!exists) { |
There was a problem hiding this comment.
let's flip this if around:
if (exists) {} else {}much easier to read that way
index.js
Outdated
| } | ||
| } | ||
| let nextDealBatch | ||
| res['paging'] && res['paging']['next'] |
There was a problem hiding this comment.
let's just use an if here
| return [] | ||
| } | ||
| let requestUrl = await storage.get(NEXT_COMPANY_BATCH_KEY) | ||
| const properties = [ |
There was a problem hiding this comment.
this can live at the top-level instead of inside the function
There was a problem hiding this comment.
This is just because before doing anything I am checking a condition that whether should I run this function or not.
There was a problem hiding this comment.
right but there's no need to re-initialize this every time the function is called
Co-authored-by: Yakko Majuri <38760734+yakkomajuri@users.noreply.github.com>
Co-authored-by: Yakko Majuri <38760734+yakkomajuri@users.noreply.github.com>
| } | ||
|
|
||
| export async function setupPlugin({ config, global }) { | ||
| async function setupPlugin({ config, global }) { |
There was a problem hiding this comment.
why are we removing export? this should be here. all plugin functions should be exported
There was a problem hiding this comment.
Actually, I have exported it below at the end along with the other exports.
| const { resetMeta } = require('@posthog/plugin-scaffold/test/utils') | ||
| const { fetchAllCompanies, fetchAllDeals } = require('./index') | ||
|
|
||
| test('test skip fetching companies if company groupType not set', async () => { |
There was a problem hiding this comment.
these definitely don't test very much
yakkomajuri
left a comment
yakkomajuri
left a comment
There was a problem hiding this comment.
@marcushyett-ph this is shaping up - I wonder if someone should test this e2e? i.e. connect to Hubspot and try this out
2 participants
Hey guys, let me know if you want me to make any changes.