Skip to content

Add currency symbol handling to product price functions - #16976

Merged
peranosborn merged 3 commits into
mainfrom
po/fix-missing-currency-symbol
Oct 20, 2026
Merged

peranosborn merged 3 commits into
mainfrom
po/fix-missing-currency-symbol

Conversation

@peranosborn

Copy link
Copy Markdown
Contributor

What does this change?

Fixes a bug where the currency symbol was missing from the lowest price when a live price was used, and correctly parses $ prices and prices with commas (e.g. £1,299) when comparing them with the live price.

Why?

Live prices store the currency symbol separately from the number, so the lowest price showed a bare 129 instead of £129 whenever a live price was used. Separately, the old price parsing never removed $ and stopped at commas. As a result, $ products never showed their live price on product cards, and prices like £1,299 were read as £1, so real price drops didn't get the strikethrough.

How has this change been tested?

Unit tests: the lowest-price tests now expect the currency symbol (e.g. £19.99 rather than 19.99), and a new test checks that a $ live price keeps its symbol ($9.99). New getProductCtaLivePrice tests cover a $ price using the live price and a price with a comma (£1,299).

Screenshots

Before After
before after

@peranosborn
peranosborn requested review from SolomonTheGuardian and ecguardian and a balanced review from Copilot October 20, 2026 11:58
@github-actions

Copy link
Copy Markdown

Hello 👋! When you're ready to run Chromatic, please apply the run_chromatic label to this PR.

You will need to reapply the label each time you want to run Chromatic.

Click here to see the Chromatic project.

@peranosborn peranosborn added run_chromatic Runs chromatic when label is applied feature Departmental tracking: work on a new feature labels Oct 20, 2026

Copilot AI left a comment

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.

🟡 Changes recommended

The thousands-separator test needs to detect the reported strikethrough regression rather than pass with the original parser.

1 open issue
What changed in this PR

Fixes currency display and price comparisons for affiliate products using live prices.

Changes:

  • Preserves currency symbols in lowest-price displays.
  • Removes dollar symbols and commas before comparing article prices.
  • Updates unit tests for currency symbols and live-price handling.
File Description
dotcom-rendering/​src/​model/​enhanceProductElement.ts Includes currency symbols in live lowest prices.
dotcom-rendering/​src/​model/​enhanceProductElement.test.ts Checks pound and dollar lowest-price displays.
dotcom-rendering/​src/​components/​AffiliateProducts/​livePriceUtils.tsx Corrects dollar and thousands-separator parsing.
dotcom-rendering/​src/​components/​AffiliateProducts/​livePriceUtils.test.tsx Adds live-price rendering tests.

Review effort: Balanced

Comment thread dotcom-rendering/src/components/AffiliateProducts/livePriceUtils.test.tsx Outdated
@github-actions github-actions Bot removed the run_chromatic Runs chromatic when label is applied label Oct 20, 2026

@rcrphillips rcrphillips left a comment

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.

LGTM

@ecguardian ecguardian left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good 👍

@peranosborn
peranosborn enabled auto-merge October 20, 2026 12:50
@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

🚀 Image pushed to AWS ECR

Image digest: sha256:da10a100123b1a66d799974c97f3202506411483113b9ff64f48ed55ef11f57a

🐛 Run the image locally

The following can be used to run the image locally:

# Refer to image using the immutable digest. Find alternatives below.
IMAGE_IDENTIFIER="@sha256:da10a100123b1a66d799974c97f3202506411483113b9ff64f48ed55ef11f57a"

# Refer to image using branch tag
# IMAGE_IDENTIFIER=":branch-po-fix-missing-currency-symbol"

# Refer to image using build tag
# IMAGE_IDENTIFIER=":build-32648"

# Set environment variables for the AWS CLI
AWS_PROFILE="<A_PROFILE_FROM_JANUS>"
AWS_DEFAULT_REGION="eu-west-1"

IMAGE_ACCOUNT_ID=$(aws ssm get-parameter --name /organisation/accounts/artifacts --query "Parameter.Value" --output text)
REGISTRY="${IMAGE_ACCOUNT_ID}.dkr.ecr.${AWS_DEFAULT_REGION}.amazonaws.com"
IMAGE="${REGISTRY}/guardian/dotcom-rendering/dotcom-rendering${IMAGE_IDENTIFIER}"

# Login to AWS ECR https://docs.aws.amazon.com/AmazonECR/latest/userguide/registry_auth.html
aws ecr get-login-password | docker login --username AWS --password-stdin $REGISTRY

# Pull the image
docker pull $IMAGE

# Run the image. You'll likely need to set additional flags. See https://docs.docker.com/reference/cli/docker/container/run.
docker run $IMAGE

From guardian/actions-ecr.

@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

@peranosborn
peranosborn disabled auto-merge October 20, 2026 13:40
@peranosborn
peranosborn enabled auto-merge October 20, 2026 13:40
@peranosborn peranosborn added the run_chromatic Runs chromatic when label is applied label Oct 20, 2026
@github-actions github-actions Bot removed the run_chromatic Runs chromatic when label is applied label Oct 20, 2026
@peranosborn
peranosborn added this pull request to the merge queue Oct 20, 2026
Merged via the queue into main with commit 673184a Oct 20, 2026
30 checks passed
@peranosborn
peranosborn deleted the po/fix-missing-currency-symbol branch October 20, 2026 14:03
@gu-prout

gu-prout Bot commented Oct 20, 2026

Copy link
Copy Markdown

Seen on PROD (merged by @peranosborn 5 minutes and 45 seconds ago) Please check your changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Departmental tracking: work on a new feature Seen-on-PROD

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants