Send timezone offset as string instead of integer

[ludoboludo]

PR Summary:

This fixes an issue when sending timezone offset as negative integer. This change ensures the offset is sent as string which avoid the cart add issue for gift cards with recipient.

Why are these changes introduced?

The cart serialization of line item properties with negative integer does not work. Using a string instead works.

The latest version of recipient form sends a new line item property __shopify_offset which can be a negative number (based on the customer timezone). When its negative, request to add gift card to the cart will fail (cart serialization code doesnt accept negative number)

What approach did you take?

Casting.

Visual impact on existing themes

Hide whitespace when checking the files changed

No visual impact.

Checklist

• Added PR summary for release notes
• Requested review from UX (Only for changes that are affecting the experience or perceivable visual details)
• Created a ticket for the help.shopify.com documentation team about updates to theme settings. (Internal-only task)
• Followed theme code principles
• Linted with Theme Check
• Tested on mobile
• Tested on multiple browsers
• Tested for accessibility

[ludoboludo]

Original comment from @fredma :

The backend already casts the property to int if its a string, so this is safe.

[sofiamatulis]

@ludoboludo I think the original change was only this line, but the whole file seems to be changed 🤔

https://github.com/Shopify/dawn/pull/2633/files#diff-b155c49124449ce256c03e05ec95dc95b232976c969766f023e30278c1833210L14

[ludoboludo]

Yeah formatting happened. I don't think Fred was using the prettier settings we do. So if you hide the whitespace while reviewing there should not be any change affecting the rest of the code:

[sofiamatulis]

I still see a lot of other lines being affected. Is that expected?

[ludoboludo]

Can you share a screenshot of what you're seeing ?
From my side it's just different formatting, spacing, semi colons, parentheses, etc. And that change with .toString()

[ludoboludo]

It wasn't formatted on main though I think. It's code that's been added by folks from another team and I don't think they were using prettier the way we do. 🤷

We could confirm by having you go or other devs into that file from /main and save.

[sofiamatulis]

I dont see all the changes when I save it 🤔 but maybe i have different settings?

(how customElements .define line went to a different line) : https://screenshot.click/12-31-y8gnz-tgjux.mp4

[kmeleta]

I dont see all the changes when I save it 🤔 but maybe i have different settings? (how customElements .define line went to a different line

When I prettify this file (outside this PR) I get the same type of formatting seen here. I'm not 100% but I think prettier in its default state tends to break complex arguments like an anon function, or in this case a class definition, into their own lines even if the line was within our defined print width of 120. So this is expected as far as I can tell.

Sofia, we can pair to see if/why your editor isn't using the same formatting as ours. Being a new addition, we've definitely had some cases of it not taking effect and instead using your existing editor/workspace preferences. Quitting vscode usually fixes it though. Either way, I wouldn't treat the formatting as a blocker.

[ludoboludo]

The formatting in settings.json should be like this:

Source

[sofiamatulis]

Ah! Mystery solved 😅 thanks Ludo! Definitely not a blocker - agreed

[LucasLacerdaUX]

@ludoboludo Code looks good. How do we test this?

[fredma]

@LucasLacerdaUX

How do we test this?

I recorded a video if that helps.

Just to clarify, the casting in itself does not make a difference since the payload is built using FormData which already convert values to string. However, the change here really serves a documenting role and makes it explicit that the offset should only be sent as string. Some customized themes may send the payload as JSON which allows integer type.