Remove URL setting #5000
Merged
Remove URL setting #5000
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We were using `Setting["url"]` to verify the content belonged to the application URL, but we can use `root_url` instead. Note that means we need to include the port when filling in forms in the tests, since in tests URL helpers like `polymorphic_url` don't include the port, but a port is automatically added when actually making the request.
We've been using the `url` Setting for a long time, but since then we've added a few references to `root_url` to this file, so we're now adding consistency. We're also removing a now unnecessary condition.
In the dev seeds, we were using `Setting["url"]/proposals`, but we can use `proposals_url` instead, similar to what we do in the rest of the application. We can do a similar thing in the sitemap. This way the sitemap will also work on installations who haven't manually set the "url" setting. Since we aren't using `Setting["url"]` anywhere anymore, we're removing it. This setting was mainly redundant, since we already had the `server_name` in the secrets. Furthermore, `server_name` is automatically configured when running the installer, while `Setting["url"]` had to be manually set in the admin section the application was installed. Note we're using `ActionMailer::Base` setting to generate URLs. Sounds a bit strange, but it's a standard way Rails provides to generate URLs outside the context of a request.
taitus
approved these changes
Oct 4, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
References
Backgrounds
Until now, CONSUL installations had to specify their domain in two places: the
server_namekey in thesecrets.ymlfile (which is done automatically by the installer) and theurlconfiguration option in the administration settings.This was redundant and, since the
urloption was only used when generating the sitemap, the dev_seeds, a<meta>tag and to validate related content, it was perfectly possible to leave it at the default value ofexample.comand never realize some things weren't working properly.Furthermore, validating the related content and generating the mentioned
<meta>tag is done in the context of a request, so we already have the domain in the request itself and don't need a configuration option.Objectives
Release Notes
We've removed the
urlconfiguration option in the administration settings. If you're usingSetting["url"]in your custom code, replace it with eitherroot_url(if the code is executed during a request) or withroot_url(ActionMailer::Base.default_url_options)(if the code is executed during a rake task or a delayed job).