fix(hubspot): Add marketing support to ReadUsingSearchAPI

[Cobalt0s]

Live Tests

ReadUsingSearchAPI

Contacts

Since can be passed in various ways:

Campaigns

[Cobalt0s]

• fix(hubspot): Add marketing support to ReadUsingSearchAPI #2995 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[RajatPawar]

Hey Constantin!

• The isSince/isUntil flags - I'd really prefer we don't go this route. It feels quite brittle/invisible, and will add a quiet dependency to the entire hubspot chain. The marketing path would depend on the caller having used BuildLastModifiedFilterGroup / BuildUntilTimestampFilterGroup specifically, and anyone refactoring that could miss it very easily. Or if anyone constructs an equivalent filter by hand, we'd run a backfill.

Given that this is high-pri, my preference would be to add Since time.Time and Until time.Time directly onto SearchParams. The marketing/comm/misc branches can read them directly from SearchParams. It needs a minor server change to the hubspot search reader.

Longer term, the hubspot connector could expose a method that can tell us if an object supports search or not, and the server casts the connector & uses that method to route to the correct reader.

Also, can you please add a unit test in search_test.go for the new marketing branching logic? I don't think we have a test for ReadUsingSearchAPI for non-CRM objects. Basically, something that calls ReadUsingSearchAPI with ObjectName: "marketing/emails" with Since set

Thanks!

[Cobalt0s]

Hey Constantin!

• The isSince/isUntil flags - I'd really prefer we don't go this route. It feels quite brittle/invisible, and will add a quiet dependency to the entire hubspot chain. The marketing path would depend on the caller having used BuildLastModifiedFilterGroup / BuildUntilTimestampFilterGroup specifically, and anyone refactoring that could miss it very easily. Or if anyone constructs an equivalent filter by hand, we'd run a backfill.

Given that this is high-pri, my preference would be to add Since time.Time and Until time.Time directly onto SearchParams. The marketing/comm/misc branches can read them directly from SearchParams. It needs a minor server change to the hubspot search reader.

Longer term, the hubspot connector could expose a method that can tell us if an object supports search or not, and the server casts the connector & uses that method to route to the correct reader.

Also, can you please add a unit test in search_test.go for the new marketing branching logic? I don't think we have a test for ReadUsingSearchAPI for non-CRM objects. Basically, something that calls ReadUsingSearchAPI with ObjectName: "marketing/emails" with Since set

Thanks!

Makes sense.

Also for future, we should be taking into considiration hubspot.ReadUsingSearchAPI and hubspot.Read and hubspot.Search, where the last 2 are general contracts and ReadUsingSearchAPI is special case which existed before the connectors.SearchConnector.

For now hubspot.Search will return an object is not supported, because that is the case.
But for the ReadUsingSearchAPI everything is supported. Since/Until as you stated are the primary channels to pass that info. The only problem becomes if both Filter and Since/Until is set for the CRM objects, or if the Filter was not set -- in that case Filters are auto populated params = params.applyTimestampFilters(). This means that BuildLastModifiedFilterGroup and BuildUntilTimestampFilterGroup could be removed from server as long as Since/Until are passed.

[Cobalt0s]

@RajatPawar the PR description was updated and it demonstrates what the changes do. Alternatively there are unit tests which you asked. For the contacts there 3 unit tests to demonstrate combinations of Since+Filter, Since, Filter.

PS: The change is backwards compatible and allows Since/Until for marketing/etc and acts smartly in case of CRM.