Skip to content

Adds examples for conversion enhancements and form data uploads#486

Merged
jradcliff merged 2 commits intomainfrom
convexamples_v8_1
Sep 22, 2021
Merged

Adds examples for conversion enhancements and form data uploads#486
jradcliff merged 2 commits intomainfrom
convexamples_v8_1

Conversation

@jradcliff
Copy link
Copy Markdown
Member

Also adds an --orderId parameter to UploadOfflineConversion
since order ID is required for enhancements.

Also adds an `--orderId` parameter to UploadOfflineConversion
since order ID is required for enhancements.
@jradcliff jradcliff requested review from aohren and devchas September 22, 2021 16:21
@jradcliff
Copy link
Copy Markdown
Member Author

This PR moves the example changes made in tg/1173607 (reviewed by Adam and Nick) into main.

// User identifiers for conversion enhancements MUST use first party data.
.setUserIdentifierSource(UserIdentifierSource.FIRST_PARTY)
.setAddressInfo(
OfflineUserAddressInfo.newBuilder()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It doesn't look like the OfflineUserAddressInfo builder is ever actually built. Is this correct? Does it work?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yep, this works. Setters are usually overloaded to take either the type of the field or its corresponding builder type.
I was using this feature for the sake of brevity, but do you think this will be confusing for users? If so, I can add explicit build() calls in the relevant spots.


if (restatementValue != null) {
// Creates a builder to construct the restated conversion value.
RestatementValue.Builder valueBuilder = enhancementBuilder.getRestatementValueBuilder();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It doesn't look like valueBuilder builder is ever actually built. Is this correct? Does it work?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This works as well since the valueBuilder is created from the enhancementBuilder, and the latter will be built when it's set during request construction below.

conversionUploadServiceClient.uploadConversionAdjustments(
UploadConversionAdjustmentsRequest.newBuilder()
.setCustomerId(Long.toString(customerId))
.addConversionAdjustments(enhancementBuilder)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It doesn't look like enhancementBuilder builder was ever actually built. Is this correct? Does it work?

@jradcliff jradcliff requested a review from devchas September 22, 2021 19:18
@jradcliff jradcliff merged commit 60b99f1 into main Sep 22, 2021
@jradcliff jradcliff deleted the convexamples_v8_1 branch September 22, 2021 21:23
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