nystudio107 / craft-retour

Retour allows you to intelligently redirect legacy URLs, so that you don't lose SEO value when rebuilding & restructuring a website
https://nystudio107.com/plugins/retour
Other
38 stars 26 forks source link

Short links with multi site #277

Closed rellafella closed 1 year ago

rellafella commented 1 year ago

Describe the bug

When short link field is used in our Craft 3 multi-site setup we can't seem to use the shortlinks field across multiple sites.

If we create a shortlink in our default site, then visit /admin/retour/shortlinks then as expected we will see the shortlink appear in the "All sites" and the "Default site" view when using the site selector drop down.

The issue starts when we then go to an entry in another site and create a short link there. Once we save the entry and navigate back to /admin/retour/shortlinks now the "All sites" table is empty and the "site2" table is empty. The original shortlink from the default site still shows if you select "default" in the site selector dropdown.

The default short link still continues to work, but the shortlink created for site2 does not.

I have tried this in a fesh Craft 3 installation (and even tried upgrading the example project to Craft 4 but that did not fix the issue)

To reproduce

Steps to reproduce the behaviour:

  1. Create a shortlink field and add it to an entry type that is shared across multiple sites.
  2. Create a shortlink in entries across multiple sites
  3. Test shortlinks and check the shortlinks dashboard at /admin/retour/shortlinks

Expected behaviour

Shortlinks should work across multiple sites

Versions

khalwat commented 1 year ago

How are your sites set up in terms of localized site URLs?

In other words, do you have:

myfirstsite.com mysecondsite.com

or

en.mysite.com de.mysite.com

or do you use a sub-path, like:

mysite.com/en/ mysite.com/de/

?

khalwat commented 1 year ago

Also what propagation method do you have set on the Short Link field?

rellafella commented 1 year ago

Good questions.

Site url setup is by directory.

mydefaultsite.com mydefaultsite.com/site2

By propagation method, I take it you're referring to translation method? That's set to "translate for each site"

khalwat commented 1 year ago

I'm unable to reproduce this. Here's what I did:

  1. multisite using directory setup
  2. section that's set to "translate for each site"
  3. shortlink field that's set to "translate for each site"
  4. create a new entry with some value in the shortlink field and save it.
  5. In Shortlinks, under "all sites", the link appears twice (as it was translated to both sites, creating two copies).
  6. Values are updated independently when editing, the links respecting the site selector.

Can you give me some further information that may help?

rellafella commented 1 year ago

In step 4 did you try populating that short link field in multiple sites? or only the default site? For me the problem seems to be specifically when using the shortlink field in sites that aren't the default site.

Here are some screenshots to help describe what is going on a bit better. I would be happy to provide a repo and a seed db if you're open to having a look that way?

Section settings:

CleanShot 2023-03-14 at 14 11 32@2x

Shortlink field settings:

CleanShot 2023-03-14 at 14 12 02@2x

Site settings:

CleanShot 2023-03-14 at 14 13 16@2x

Results (note that the default site is the only one that is displaying the shortlink in retour): shortlinks

khalwat commented 1 year ago

Addressed in https://github.com/nystudio107/craft-retour/commit/1c89b9c15d49c3db2f0682f1668ffb396df32b41 & https://github.com/nystudio107/craft-retour/commit/675ce49d3cd6236c7220b3eb01c3f5511aee4a3d

Craft CMS 3:

You can try it now by setting your semver in your composer.json to look like this:

    "nystudio107/craft-retour": "dev-develop as 3.2.10",

Then do a composer clear-cache && composer update

…..

Craft CMS 4:

You can try it now by setting your semver in your composer.json to look like this:

    "nystudio107/craft-retour": "dev-develop-v4 as 4.1.12",

Then do a composer clear-cache && composer update

rellafella commented 1 year ago

This looks to have solved the issue of displaying the shortlinks in the CP but the shortlink itself doesn't seem to be working.

✅ Shortlink in main site: about expected resolution: https://craft-test.test/about-us ❌ Shortlink in site2: contact expected resolution: https://craft-test.test/site2/contact-us

The shortlink in a subsite seems to require that the site's base dir is prefixed however this is the result I get ❌ Shortlink in site2: site2/contact expected resolution: https://craft-test.test/site2/contact-us actual resolution: https://craft-test.test/contact-us

Trialling with the site base dir prefixed has also alerted me to another issue that once the entry is saved and the shortlink is changed, the shortlink still appears in the /admin/retour/shortlinks page and cannot be deleted? I presume this is because it still exists within a revision?

khalwat commented 1 year ago

@rellafella so I think the thing to do here is that if the site is multi-site, and the redirect is localized per-site, the Short Links that Retour creates should match the Full URL.

This way it'll work for both de.example.com and example.com/de.

So we just need to look to see whether the field is localized, and if so, set the redirectSrcMatch to fullurl, and ensure that the redirectDestUrl we generate is a full URL instead of a path.

khalwat commented 1 year ago

Addressed.

Craft CMS 3:

You can try it now by setting your semver in your composer.json to look like this:

    "nystudio107/craft-retour": "dev-develop as 3.2.10”,

Then do a composer clear-cache && composer update

…..

Craft CMS 4:

You can try it now by setting your semver in your composer.json to look like this:

    "nystudio107/craft-retour": "dev-develop-v4 as 4.1.12”,

Then do a composer clear-cache && composer update

rellafella commented 1 year ago

Thanks @khalwat that is now cleaning out the list as that shortlink is updated or removed from the entry.

The full url fix for multisite redirects also works so thats good, I guess the only gotcha here is debugging stuff locally since in production the full url will be different.

khalwat commented 1 year ago

So this means it is working as intended for you @rellafella ?