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.I'm so glad you blogged about this module in February, because I might never have known about it otherwise, and I need it! :)
In my particular case, we're publishing content in English first, and translations sometime later. After the English /node/1 is published, I need /fr/node/1 to redirect to the English content. But eventually I'll want it to serve the newly translated French content. That means a 301 isn't the right HTTP status code for me; I need a 302 (or a 303 or 307 or whatever). Would it make sense to add a sitewide config option for the status code? I'll be glad to write it, but only if you think it's a good idea.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | interdiff.txt | 985 bytes | jhodgdon |
| #6 | interdiff-2880975-3-6.diff | 550 bytes | ksenzee |
| #6 | partial_multi-config-2880975-6.patch | 9.39 KB | ksenzee |
Comments
Comment #2
jhodgdonGreat idea! Patch welcome for sure.
Comment #3
ksenzeeI tested this for a new install and it works fine. I didn't test the upgrade path, but it should theoretically work.
Comment #4
jhodgdonI read through the patch, and it looks good! Has help and everything. :)
One thing I think may be missing: I think it would be good to have the settings page shown on admin/config, and I don't think it will be with the present patch -- unless things have changed recently, it would need a partial_multi.links.menu.yml file to create that link.
Another question is the permission on the route... Looking at standard system module permissions:
https://api.drupal.org/api/drupal/core%21modules%21system%21system.permi...
I think it should be 'access administration pages' -- I don't see 'administer administration pages' permission... does that exist?
Anyway, I'll give it a whirl, including the upgrade path, sometime soon-ish, probably not for about 2 weeks (sorry for the delay).
Comment #5
ksenzeeI believe you are right on all counts. I'll see if I have time this afternoon to update the patch.
And no rush. Enjoy the summery weather. :)
Comment #6
ksenzeeHere's a new patch and an interdiff.
Comment #7
jhodgdonThe interdiff looks good! I'll test/commit the patch around June 5th most likely. Maybe even write an automated test for the module... always a good idea once it becomes a Real Maintained Module rather than something you blogged about. :)
Comment #8
jhodgdonQuick interdiff for docs and t() in the settings form... then I'll write a test, get it to pass, and commit...
Comment #10
jhodgdonOK. I created a test. It failed saying:
Drupal\Core\Config\Schema\SchemaIncompleteException: No schema for partial_multi.settings
So I had to add a config/schema/partial_multi.schema.yml file (rather trivial but still required).
Then... I hate phpunit in Drupal -- never can get it to run right, and it stands in my way of just making and running a test....finally went back to making it a Simpletest WebTestBase instead, because I can run the test and not have trouble just running it. I don't care if the dang class is deprecated, I just want to be able to make a quick test and run it, is that too much to ask???
Anyway, sorry, ranting. Finally got the test to pass...
One other change: I think 'administer site configuration' is probably a better permission to use for the admin page. That is what you need for various pages in the system module, like admin/config/development/performance etc.
So, with those changes and the interdiff above, committed. Thanks again!
Comment #11
ksenzeeThat all sounds great (except for phpunit, which, ugh). Thank you!