Skip to content

Fix double-logo bug: only replace placeholder with logo image - #167

Open
sidharth-pardeshi wants to merge 5 commits into
Affirm:masterfrom
sidharth-pardeshi:fix/double-logo-replacement
Open

Fix double-logo bug: only replace placeholder with logo image#167
sidharth-pardeshi wants to merge 5 commits into
Affirm:masterfrom
sidharth-pardeshi:fix/double-logo-replacement

Conversation

@sidharth-pardeshi

Copy link
Copy Markdown
Collaborator

Summary

Fix bug where ALL instances of "Affirm" in promo messages were replaced with logo images. Only the {affirm_logo} placeholder positions should become logos; plain-text "Affirm" (e.g., in "Affirm is a form of credit") stays as text.

Root Cause

  1. PromoRequest.java replaced {affirm_logo} with literal "affirm", losing the distinction between placeholder and real text
  2. AffirmUtils.java used \baffirm\b regex pattern which matched EVERY "affirm" and replaced it with a logo ImageSpan

Changes

  • AffirmUtils.java: Changed LOGO_PATTERN to match {affirm_logo} instead of \baffirm\b
  • PromoRequest.java: Stopped converting placeholder to bare "affirm" before spannable rendering; placeholder now flows through intact
  • AffirmUtilsTest.java: Added 4 regression tests

Test plan

  • Template with both {affirm_logo} and plain "Affirm" -> only 1 logo
  • Template with only plain "Affirm" -> 0 logos
  • Template with multiple placeholders -> all replaced
  • TEXT display type -> no logos inserted

sidharth-pardeshi and others added 2 commits April 27, 2026 09:41
… image

When promo messages contain multiple instances of "Affirm" (e.g., "...with
Affirm. Affirm is a form of credit..."), ALL instances were being replaced
with logo images. Only the {affirm_logo} placeholder should become a logo;
plain text "Affirm" should remain as text.

Root cause: PromoRequest replaced the {affirm_logo} placeholder with plain
text "affirm", then AffirmUtils used a case-insensitive regex (\baffirm\b)
to replace every occurrence with a logo ImageSpan.

Fix: Keep the {affirm_logo} placeholder intact through the pipeline and
change LOGO_PATTERN to match the placeholder literal instead of the word
"affirm". The plain-text description still replaces the placeholder with
"Affirm" for accessibility/content description use.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Affirm-Skill: att-test
Affirm-Skill: buildkite-debug
Affirm-Skill: capture-context
Affirm-Skill: cmt-writer
Affirm-Skill: commit-and-push
Affirm-Skill: gather-best-practices
Affirm-Skill: mcp-debug
Affirm-Skill: save-output
Affirm-Skill: tickets-from-plan
Affirm-Skill: write-implementation-plan
…le inflation

The new createSpannable tests were failing with Resources$NotFoundException
because Robolectric cannot reliably inflate vector XML drawables. Replace
RuntimeEnvironment.getApplication() with a mock Context that provides a
mock Drawable via Resources.getDrawable(), so the tests verify spannable
logic without depending on Android resource inflation.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Affirm-Skill: att-test
Affirm-Skill: buildkite-debug
Affirm-Skill: capture-context
Affirm-Skill: cmt-writer
Affirm-Skill: commit-and-push
Affirm-Skill: gather-best-practices
Affirm-Skill: generateReleaseUnitTestStubRFile
Affirm-Skill: mcp-debug
Affirm-Skill: save-output
Affirm-Skill: testDebugUnitTest
Affirm-Skill: tickets-from-plan
Affirm-Skill: write-implementation-plan
Comment thread affirm/src/main/java/com/affirm/android/PromoRequest.java
sidharth-pardeshi and others added 2 commits April 27, 2026 13:46
When display type is TEXT (no logo), the {affirm_logo} placeholder was
leaking through as raw text. Now getSpannable replaces the placeholder
with "Affirm" when logoDrawable is null. Also adds test assertion to
verify the placeholder is not visible in TEXT display output.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Affirm-Skill: att-test
Affirm-Skill: buildkite-debug
Affirm-Skill: capture-context
Affirm-Skill: cmt-writer
Affirm-Skill: commit-and-push
Affirm-Skill: gather-best-practices
Affirm-Skill: generateReleaseUnitTestStubRFile
Affirm-Skill: mcp-debug
Affirm-Skill: save-output
Affirm-Skill: testDebugUnitTest
Affirm-Skill: tickets-from-plan
Affirm-Skill: write-implementation-plan
Match the Kotlin sample by using Y8CQXFF044903JC0 instead of
3HCWTVU5BYWZB9RK which is no longer responding.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Affirm-Skill: att-test
Affirm-Skill: buildkite-debug
Affirm-Skill: capture-context
Affirm-Skill: cmt-writer
Affirm-Skill: commit-and-push
Affirm-Skill: gather-best-practices
Affirm-Skill: generateReleaseUnitTestStubRFile
Affirm-Skill: mcp-debug
Affirm-Skill: save-output
Affirm-Skill: testDebugUnitTest
Affirm-Skill: tickets-from-plan
Affirm-Skill: write-implementation-plan

private static final Pattern LOGO_PATTERN =
Pattern.compile("\\baffirm\\b", Pattern.CASE_INSENSITIVE);
Pattern.compile("\\{affirm_logo\\}");

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.

This works as is, but using Pattern.quote would make it easier to maintain.

Pattern.compile(Pattern.quote(LOGO_PLACEHOLDER))

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done in 0026a93. Switched to Pattern.compile(Pattern.quote(LOGO_PLACEHOLDER)) and added the static import for LOGO_PLACEHOLDER from AffirmConstants.

Addresses review feedback: keeps the regex in sync with the
LOGO_PLACEHOLDER constant so any future change to the placeholder
string flows through automatically.

Affirm-Skill: att-test
Affirm-Skill: buildkite-debug
Affirm-Skill: capture-context
Affirm-Skill: cmt-writer
Affirm-Skill: commit-and-push
Affirm-Skill: gather-best-practices
Affirm-Skill: mcp-debug
Affirm-Skill: save-output
Affirm-Skill: tickets-from-plan
Affirm-Skill: write-implementation-plan
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