When saving the Privacy and Imprint URIs as relative URIs (e.g. '/privacy'), they are automatically converted to a system URI (e.g. '/node/25/').
The problem is that the system URI is saved to config, which could be an issue if a privacy page is represented by different nodes across environments (e.g. '/node/136/' instead of '/node/25/').
I propose the following: When checking if the relative URI is resolvable, save relative URIs in config as entered by the user. But do not convert them to the system URI.
Thank you.
Remaining steps (see #13):
0. Explain the change. What happened before and what happens now? How are the values handled and saved?
1. Add tests to ensure this works as expected for all possible cases of input, like for example:
- Drupal paths: /xyz (existing and not existing)
- Drupal paths:
- External URLs: https://xyz.com/123
- Other (non-existing) protocols: example:// and crap like "test123" (without leading slash)
- Did I forget something?
If we already have tests for this, please ensure all these cases are tested for their expected result
2. Ensure we have an update path here for existing values. What is expected to happen for existing values? What happens if a user re-saved the form with the old values?
3. Check for possible side effects where these values are used and write tests for these
4. Ensure this works like it's expected in Drupal and for example known from menu items etc.
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
markdcComment #5
gilmordAgree, dependency on content URI is hell on multi-environment set-ups.
Removed alias transformations in the MR, the same approach we can see in the core "Default front page" in the "Basic site settings" configuration form.
Comment #6
gilmordChecking why the tests have failed
Comment #7
gilmordLooks like it comes from facebook_pixel submodule, I do not think it was affected by the changes
Comment #9
gilmordCreated and empty MR to trigger test (the only option for not project maintainers)
I want to check if it fails on 1.0.x or this facebook_pixel failed tests are really related to the changes in this issue.
Comment #10
gilmordClean MR with no changes on 1.0.x branch also failed the same tests, I can assume this is a problem in the 1.0.x and not in this task.
Comment #11
markdc#4 works. Thanks so much!
Comment #13
anybodyThank you for your work on this @gilmord!
Indeed, the failing tests are unrelated and are caused by: #3250126: Submodule dependencies are ignored if composer.json present in parent module (only in issue fork test runs!)
For this to be merged, we need to take the following steps:
0. Explain the change. What happened before and what happens now? How are the values handled and saved?
1. Add tests to ensure this works as expected for all possible cases of input, like for example:
- Drupal paths: /xyz (existing and not existing)
- Drupal paths:
- External URLs: https://xyz.com/123
- Other (non-existing) protocols: example:// and crap like "test123" (without leading slash)
- Did I forget something?
If we already have tests for this, please ensure all these cases are tested for their expected result
2. Ensure we have an update path here for existing values. What is expected to happen for existing values? What happens if a user re-saved the form with the old values?
3. Check for possible side effects where these values are used and write tests for these
4. Ensure this works like it's expected in Drupal and for example known from menu items etc.
I think we still have a way to go here, but this is also a good chance to make the module better tested and for standardization!
Comment #14
anybodyComment #15
anybodyComment #16
anybodyWe should make this switch with 2.x
Comment #17
anybodyComment #18
anybodyHey ho, let's go! 2.x branch was recently created. Please help to implement this or if you're not a developer, please sponsor some development hours.
Comment #22
_shyCreated a new branch and PR for 2.x version.