Validate product and collection handles to be URL safe

[thetutlage]

This PR validates the Product and the Collection handles to be URL safe using a Regex. Consult the following screenshot to see what is allowed

---

Update

Ended up moving the normalization and validation both to the service. Also tested manually with the admin panel and following are the results.

A title with @ in it generates correct handle

The backend returns error when handle it manually updated to contain invalid characters.

However, in this case frontend isn't showing errors. Something should be handled in a separate issue.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 83be029

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[vercel]

@thetutlage is attempting to deploy a commit to the medusajs Team on Vercel.

A member of the Team first needs to authorize it.

[thetutlage]

This is my first PR. So please feel free to familiarise me to the process and let me know if something needs to changed or done in a different way.

Also, I wasn't table to find any tests for the validators (maybe I looked at wrong places). So please guide me on how/where to write tests

[riqwan]

💪🏻

[srindom]

Good work with the validators - I'd expect these checks to happen further down the stack too.

E.g., calling productModuleService.create({ handle: "not allowed-cuz'-bad chars" }) should throw.

Also, there is some logic somewhere that constructs a handle from the title, not sure if it's in the admin code or somewhere else. Currently, this code doesn't remove disallowed characters. E.g. title: "Summer '24" will result in handle: "summer-'24". This logic should be modified too.

[thetutlage]

Good work with the validators - I'd expect these checks to happen further down the stack too.

Yeah, I had this conversation on the Slack too. I did not find any sort of schema validations happening in the services, so didn't feel comfortable to put it there.

But, if the goal is to keep formatting validations at the service layer. I can surely move the logic there as a manual validation. Or should we use Zod within services as-well?

[thetutlage]

Also, there is some logic somewhere that constructs a handle from the title, not sure if it's in the admin code or somewhere else. Currently, this code doesn't remove disallowed characters. E.g. title: "Summer '24" will result in handle: "summer-'24". This logic should be modified too.

Yeah, makes sense. The creation of handle should perform the normalization

[thetutlage]

I ended up changing the implementation

• Create common helpers to generate and validate handles.
• Remove Zod validations and handle it at the service.

Let me know if the implementation looks right. If yes, I will repeat it for collections and other entities as well.

[thetutlage]

Anyone knows what is causing this error in integration tests?

[adrien2p]

Anyone knows what is causing this error in integration tests?

I believe this might be due to the upert with replace, @sradevski what do you think? I believe the reason is that an obj ref might be assigned at different level creating a circular ref on jsonifying it

[thetutlage]

I got the error fixed after this commit ae4d9a7

Seems like removing the handle generation from the model hook breaks something and the error stack isn't able to point at it.

I am not sure if I should remove the model hook or not. Because we have the same logic in service also and I remove the logic from service, then some other parts of the application break.

[thetutlage]

@riqwan I shared the reasoning about the CI failing earlier in this comment. #7310 (comment)

[adrien2p]

Just need to fix the pipeline 💪 good work

[jackjyq]

I am wondering if we need the handle to be URL-safe, for example:

We got the product URL https://shopee.tw/恐龙-i.59046599.23945502411 which contains Chinese characters, we could use 恐龙-i.59046599.23945502411 as handle and let the frontend to encode the URL to https://shopee.tw/%E6%81%90%E9%BE%99-i.59046599.23945502411.

In this PR, the handle removes all non-alphanumeric characters, so we can not use 恐龙-i.59046599.23945502411.

I suggest to remove this line replace(/[^a-z0-9A-Z-_]/g, "")

[sradevski]

@jackjyq thanks for bringing it up. I agree, we went with a very restrictive check for URL-safety. Although we do want to handle this in the BE, we probably want to revisit the Regex we use for the check.

I'll open a ticket internally to track this and tackle it before the public v2 release.