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

New redirects always save with highest ID site #100

Merged
merged 1 commit into from Oct 7, 2019

Conversation

lmaucoin
Copy link

@lmaucoin lmaucoin commented Aug 1, 2019

Hi, love the plugin! However, I recently upgraded to Craft 3.2.7 and have found that redirects are no longer saving on the correct sites.

I did some sleuthing and found that though the $propagate parameter is set to false when saving the redirect, this only stops it from being saved to all sites if the record exists (see Arguments here).

To get around this, in the afterSave callback, all element records created for sites that aren't the relevant one are deleted. Unfortunately, this is run every time a record is saved (and it always gets saved for all sites when a new record is created), meaning that the only lingering redirect record is one on the highest site ID.

This PR shifts the record deletion to occur in the Controller's action for saving instead of in a post-save callback, so it's only executed for the record on the relevant site.

…ing this in the afterSave callback will have it run for each time the records are saved on the sites.
@janmeeus
Copy link

janmeeus commented Sep 5, 2019

Hi,
I tried your solutions and it seems to work on first save of a redirect.
However, when I try to edit and re-save an existing redirect, I get an error:

Schermafbeelding 2019-09-05 om 17 06 17

@lmaucoin
Copy link
Author

lmaucoin commented Sep 9, 2019

@janmeeus Thanks for finding that! I can have a look into this soon.

In the meantime, another fork of this repo at https://github.com/nyby/craft3-plugin-redirect by @nyby seems to address this same issue ... probably more eloquently.

@johanzandstra johanzandstra merged commit 0693184 into Dolphiq:master Oct 7, 2019
@janmeeus
Copy link

@johanzandstra Thanks for the fix. When are you planning to make a new release with these adjustments?

@EFWall
Copy link

EFWall commented Oct 24, 2019

@johanzandstra yes, also wondering when a new release with be posted to correct these issues. We are currently unable to save any redirects. Can you advise?

@tarmooo
Copy link

tarmooo commented Nov 7, 2019

Waiting for it too

@emilyfitton
Copy link

+1 for this one please. @johanzandstra

@janmeeus
Copy link

janmeeus commented Jan 6, 2020

@johanzandstra Thanks for the fix. When are you planning to make a new release with these adjustments?

@martindffrnt
Copy link

@johanzandstra I am running into the same issue, when is this being released? I see the last release was a year ago April, is this code still being maintained and when is the next release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants