- Issue created by @Jon Pugh
- Status changed to Needs review
9 months ago 4:07pm 11 May 2024 - πΊπΈUnited States mglaman WI, USA
Seems reasonable. Canonical feels too developer
Laravel has this as well, `url`. It's used for CLI URL generation.
'url' => env('APP_URL', 'http://localhost'),
I'm sure other projects also have this. Would be nice for Drush when using the default site.
- πΊπΈUnited States Jon Pugh Newburgh, NY
@matt: is the APP_URL for the current site? Or as a reference for the live site?
This is to store the live site so users can see it in status reports etc.
I think the screenshot is confusing. The default value is whatever site you are on, hence "localhost" because I was using PHP CLI server, but normally you would put in a separate domain.
- πΊπΈUnited States mglaman WI, USA
@matt: is the APP_URL for the current site? Or as a reference for the live site?
It's the current site. So example.com or uat.example.com.
You're meaning it should always be the production URL?
- πΊπΈUnited States Jon Pugh Newburgh, NY
Matt: I'm thinking Main URL is a good human label, but like canonical for the property name.
Open to whatever.
- πΊπΈUnited States Jon Pugh Newburgh, NY
Like
SiteService::isCanonical() as a universal way to see if a site is "live" or not.
- πΊπΈUnited States Jon Pugh Newburgh, NY
Update: After thinking a few days I think "Main URL" is the best human-readable name for this property.
"Canonical URL" is still good for the backend, I think. "Main URL" is a little vague when used in code.
Thoughts?
- πΊπΈUnited States Jon Pugh Newburgh, NY
Development on this feature has moved to β¨ Add "system.site" service to access information about the site. Active . Keeping this issue for discussing the Main URL field specifically.
- πΊπΈUnited States smustgrave
Talking to a committer and this needs product manager sign off.
- πΊπΈUnited States Jon Pugh Newburgh, NY
Matt: Yes, this is specifically for the prod URL. Can be used both for showing admin users in the status report page, and for detecting if a site is "live" or not.
- πΊπΈUnited States mglaman WI, USA
Okay, so it is not the current site's URL but intended to reflect whatever is the live/production URL for the codebase.
- πΊπΈUnited States Jon Pugh Newburgh, NY
I coincidentally installed metatags on my site yesterday, and it has a field "Canonical URL".
Did a little digging. Seems like this could be useful for printing these tags.
https://developers.google.com/search/docs/crawling-indexing/canonicaliza...
- πΊπΈUnited States Jon Pugh Newburgh, NY
@mglaman yep. I thought "Basic site info" would be a good place for it because email is there too. It feels like that form is the core identity of the site.
Also, if Drupal core had this feature, it could set DRUPAL_ENV to prod only if the URL matches. It would be a very simple way to detect environment on any system. No more if/then/else clutter in settings.php.
A comment by Ryan Price on linkedin brought up "trusted hosts"... Maybe we need a "trusted prod hosts"?
- πΊπΈUnited States liberatr Portland, OR
If you want the site to report "this is my canonical public URL", it should be validated against trusted host patterns.
- Status changed to Needs work
8 months ago 12:38am 20 June 2024 - πΊπΈUnited States xjm
I think this issue might be a wontfix:
- Having a new required field on every single Drupal site, especially one that does not affect the production user experience, seems like it is fixing an edgecase at the expense of the evaluator and site owner experience for all sites.
- Adding additional form elements always inevitably causes a usability regression, so we should make sure the usability benefit of adding the element outweighs that regression.
- It is also relevant for the product manager role because it affects the evaluator experience of Drupal. Adding an additional thing that must be configured (at site install maybe even?) is a non-trivial increase in the amount of configuration necessary to get up and running.
At a minimum, I think the field should be optional rather than required, and have a sensible default value if the user doesn't provide one. Furthermore, though, this could be addressed with a contrib module and might not even be appropriate for core.
If folks do still feel this belongs in core, then at a minimum, the field should be made optional. Then, that could be submitted for usability review (tagged "Needs usability review") once that change is made. Furthermore, even if the usability team gives the addition a +1 (benefit outweighing negative UX impact), this would still require product manager signoff because it affects the evaluator experience of Drupal.
Also, there is not actually a merge request here ATM, so we would need that too before proceed.
My personal recommendation is actually to mark this wontfix and support it via contrib, but it's outside the scope of my role as a release manager to make a final decision on that myself.