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

Add locale check logic for the redirect #71

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

bighoho123
Copy link

Since Retour has already got locale fields in the database.
It makes sense to check for locale when check for redirect.

The locale fields is displayed on the list of redirects and can be edited as well.

image

image

@khalwat
Copy link
Contributor

khalwat commented Aug 26, 2017

I didn't see this until now; it looks good! Can you do the PR against the develop branch though please?

@khalwat
Copy link
Contributor

khalwat commented Aug 30, 2017

@bighoho123 Would love to roll this in, can the PR be against develop?

@bighoho123 bighoho123 changed the base branch from master to develop August 30, 2017 23:11
@khalwat
Copy link
Contributor

khalwat commented Aug 30, 2017

Thank you sir :)

@bighoho123
Copy link
Author

@khalwat Sorry it took a bit long. I didn't check my email until yesterday. All good now

@khalwat
Copy link
Contributor

khalwat commented Aug 30, 2017

@bighoho123 No worries at all, I very much appreciate the PR. I may have to merge it in the next release, just because it looks like there may be some minor cleanup needed in the case of a site not being localized, and the test matrix I need to set up to ensure it does the right thing.

@bighoho123
Copy link
Author

@khalwat
It's an awesome project so the pleasure is mine.

FYI, I did put some logic to check if craft is being localized or not by using shouldMatchLocale() in RetourService.

@khalwat
Copy link
Contributor

khalwat commented Sep 1, 2017

@bighoho123 yeah I just need to do a bunch of sanity checking on it is all; there are many sites that use Retour, often with hundreds of redirects set up already.

I just want to do some testing to make sure it isn't going to change the behavior they are experiencing on their site currently.

@tekstrand
Copy link

We'll be looking forward to the release of this

@shoored
Copy link

shoored commented May 2, 2018

You must be crazy busy since the release of Craft 3, any idea when you will get a change to merge this one in?

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

4 participants