Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Hi,
Thanks timhilliard for the wonderful module.
We have a requirement in one of our projects and this module meets our requirement. Project being built in Drupal 8, we need a Drupal 8 version of the module.
So we thought of porting the module to Drupal 8 and therefore would like to help with upgrading this module.
Comment | File | Size | Author |
---|---|---|---|
#18 | domain_301_redirect-2775825-18.patch | 40.7 KB | malcolm_p |
| |||
#17 | domain_301_redirect-2775825-17.patch | 16.44 KB | badjava |
Comments
Comment #2
neetu morwani CreditAttribution: neetu morwani as a volunteer and at Acquia commentedWe are carrying out the work here - https://github.com/neetumorwani/domain_301_redirect
We welcome all who would like to help us in porting the project.
Thanks.
Comment #3
naveenvalechaThe module is not much big. @Neetu, could you post the patch here so that you'll also get the review by community on it like we did here in case of follow module https://www.drupal.org/node/2022181
Thanks!
Comment #4
naveenvalechaMoving to right queue
Comment #5
naveenvalechaunassigning, as there was not update on the issue from a month so I picked this up.
I have created couple of followup issues and given the credit to the users as well of the PRs which looks good to me. Some of PR does not looks good to me. So I left the issues open to review the code on the d.o.
https://www.drupal.org/node/2805871
https://www.drupal.org/node/2805887
Comment #6
naveenvalechaComment #7
badjava CreditAttribution: badjava at Metasun for Pfizer, Inc. commentedI am trying to kickstart this module and navigating through the various issues and the GitHub repo has proven to be a bit of a challenge. I have combined everything into one patch so that at least the basic redirection works (and someone wanting to test this can easily just apply it to the 8.x-1.x branch).
This patch includes:
and starts:
I have marked everything on the settings admin page that doesn't work with a *not implemented*. I will try to port the rest of the module in the near future so assigning to myself.
Comment #8
badjava CreditAttribution: badjava at Metasun for Pfizer, Inc. commentedI've changed this to an MVP port so that we can hopefully get this committed and some testing done, specifically with the event subscriber that handles the redirection.
This patch provides basic configuration as well as include/exclude pages (see screenshot). Everything else has been removed and I will update the specific issues to address those issues.
Comment #9
badjava CreditAttribution: badjava at Metasun for Pfizer, Inc. commentedComment #10
naveenvalechaNice work @badjava
Add these constants to the EventSubscriber class itself.
This is the variable of config factory interface.
Change it to
\Drupal\Core\Config\ConfigFactoryInterface
Change it to "Constructs a DomainRedirectEventSubscriber object"
Comment #11
badjava CreditAttribution: badjava at Metasun for Pfizer, Inc. commentedThanks for the review @naveenvalecha. If I move the constants, then they won't be available to
src/Form/Domain301RedirectConfigForm.php
as they are used in the applicability field? Perhaps this can be done in #2805887: Add the Domain301RedirectManager service for helper functions and included as part of the service as it would need to be injected into the form anyways.I made the other changes.
Comment #12
naveenvalecha@badjava
Would you like to sum up all the code available in the GitHub and do the required changes? I think committing it in one go would be nice.
//Naveen
Comment #13
badjava CreditAttribution: badjava at Metasun for Pfizer, Inc. commented@naveenvalecha Can we get this patch committed first? Then the dev branch will actually do something and we can get some real world testing on the event subscriber that handles the redirection.
It's going to take some time for the rest of the port to happen and I think it would be very beneficial for this module to provide the basic functionality, today. It will also allow the remainder of the work to be broken up into smaller, more manageable chunks and not have to apply multiple patches to test it.
Comment #15
naveenvalecha@badjava,
I wouldn't mind doing that :) Thanks for your contribution. I've given you the commit credit for the nice work.
Committed #11 to 8.x-1.x branch. Leaving this issue open and marking its N/W for the rest of the functionality as per #12
//Naveen
Comment #16
badjava CreditAttribution: badjava at Metasun for Pfizer, Inc. commentedExcellent. I will go through the other issues and update the plan here.
Comment #17
badjava CreditAttribution: badjava at Metasun for Pfizer, Inc. commentedPosting the remainder of the patch here for now so that if anyone wants to pick this up, they can.
These things are ported but need testing:
- cron
- redirect manager including checkDomain and checkRedirectLoop functionality
- controller with csrf_token added to route (for checkDomain)
Comment #18
malcolm_p CreditAttribution: malcolm_p commentedThe attached patch builds off of #17 by adding the following:
* Redirect http & https schemes
* Return a cacheable redirect response
* Updates to the redirect check & loop
* Tests for: redirect, redirect cacheability, redirect check & cron
* Not yet tests for: domain 301 access bypass, redirect loop check.
Comment #19
pookmish CreditAttribution: pookmish commentedWhat is the delay in getting the above fixes added. We've been using the dev version of this module for many months and it's proved to withstand over a dozen sites at this time. Can we move this along and get at least an alpha or beta release out.
Comment #20
malcolm_p CreditAttribution: malcolm_p commentedWe are also using this module in production with this patch and it would be great to see it get a stable release. I'm wondering if it's being actively maintained currently?
Comment #22
naveenvalechaI have pushed the code to the 8.x-1.x branch. It will included in the next release.
Thanks everyone for their contributions!
Comment #23
naveenvalechaI have cut the first alpha release https://www.drupal.org/project/domain_301_redirect/releases/8.x-1.0-alpha0