Skip to content

Conversation

@joaquincasal
Copy link
Collaborator

Purpose

The ResourceLink field type is not supported in the Braze app. However, users were able to select them. This PR disables those fields.

Approach

The FieldFactory wasn't aware of the existance of the ResourceLink type, so it created a BasicField. Now it creates a ExternalResourceField, which is disabled for both versions of the app.

Testing steps

Try to use both versions of the app with an entry that has a ResourceLink field.

@joaquincasal joaquincasal requested a review from a team as a code owner August 5, 2025 18:50
@netlify
Copy link

netlify bot commented Aug 5, 2025

Deploy Preview for ecommerce-app-base-components canceled.

Name Link
🔨 Latest commit f290d7b
🔍 Latest deploy log https://app.netlify.com/projects/ecommerce-app-base-components/deploys/689252e06e012600088de606

@joaquincasal joaquincasal changed the title Braze: Disable resource link fields Braze: Disable resource link fields [INTEG-2981] Aug 5, 2025
Copy link
Collaborator

@JuliRossi JuliRossi left a comment

Choose a reason for hiding this comment

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

LGTM! Left a non-blocking comment

Copy link
Collaborator

Choose a reason for hiding this comment

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

💭 I feel like this class and richText have a lot in common. Perhaps there is a way to avoid duplication. On the other hand, I like that it is explicit on how we handle ExternalResourceField. So I leave it up to you if you want to remove the duplicates

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The thing is that rich text is supported in one of the versions, that's why I kept them separate

@joaquincasal joaquincasal merged commit 435ef22 into master Aug 6, 2025
17 of 18 checks passed
@joaquincasal joaquincasal deleted the braze-resource-link branch August 6, 2025 17:14
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.

4 participants