Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update owndomain handling to accept FQDN #1149

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

admdly
Copy link
Contributor

@admdly admdly commented Apr 22, 2023

Update owndomain handling to accept a FQDN as one field, rather than separate SLD and TLD, and to instead detect the TLD.

@admdly admdly self-assigned this Apr 22, 2023
@admdly admdly added the bug Something isn't working label Apr 22, 2023
@@ -148,11 +148,6 @@ public function addItem(\Model_Cart $cart, \Model_Product $product, array $data)

$productServiceFromList = $productFromList->getService();

// @deprecated logic
Copy link
Member

Choose a reason for hiding this comment

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

This cannot be removed. Existing core modules still use it

@admdly admdly added this to the 0.5 milestone Apr 29, 2023
@John-S4 John-S4 removed this from the 0.5 milestone Oct 17, 2023
Update owndomain handling to accept a FQDN as one field, rather than separate SLD and TLD, and to instead detect the TLD.
@BelleNottelling
Copy link
Member

@admdly not sure if you're still working on this or not, but I believe this package will be incredibly valuable in our ability to perform proper validation: https://github.com/jeremykendall/php-domain-parser.

We can use it to extract info like the suffix, domain, subdomain, ect & I think this section from their readme highlights why it's superior to what we'd (most likely) end up building:

Consider the domain www.pref.okinawa.jp. In this domain, the public suffix portion is okinawa.jp, the registrable domain is pref.okinawa.jp, the subdomain is www and the second level domain is pref.
You can't regex that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working modules tests/CI
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

None yet

3 participants