- last update
over 1 year ago Custom Commands Failed - πΊπΈUnited States smustgrave
Just postponed π Unable to save '/' root path alias Postponed on this one so triaging it some.
Cleanup the issue summary would help. Started with the template.
Will also need tests.
- π©πͺGermany Anybody Porta Westfalica
Thanks @smustgrave. So @yogeshmpawar could you perhaps reroll this as MR and try to fix the remaining tests?
- First commit to issue fork.
- last update
over 1 year ago Custom Commands Failed - @grevil opened merge request.
- π©πͺGermany Grevil
I created an issue fork on 11.x and applied patch "1255092-41.patch" by @yogeshmpawar.
- π©πͺGermany Grevil
Hm, Jenkins seems to be configured differently for 11.x? At least 1 PHPStan error, doesn't really make sense.
Changing to 10.1.x-dev
- π©πͺGermany Anybody Porta Westfalica
@Grevil: I think we should still target 10.1.x - if needed, we can switch later.
- last update
over 1 year ago Custom Commands Failed - @grevil opened merge request.
- last update
over 1 year ago Custom Commands Failed - π©πͺGermany Grevil
Alright, no chance to run the tests remotely on neither 10.1.x nor 11.x. I'll test it locally...
- last update
over 1 year ago Custom Commands Failed - Status changed to Needs review
over 1 year ago 10:18am 23 May 2023 - π©πͺGermany Anybody Porta Westfalica
The patch had a breaking change in the constructor parameters, which of course isn't allowed, at least if it's not consistently changed.
So for now I reverted that change, but kept the class property:
public function __construct(AliasManagerInterface $alias_manager) { $this->aliasManager = $alias_manager; $this->config = \Drupal::service('config.factory'); }
Unsure what's the DI strategy here in core for this case, perhaps someone else could have a look?
I guess the tests should run now.
- last update
over 1 year ago Custom Commands Failed 7:21 1:19 Running- πΊπΈUnited States smustgrave
Believe should be targeting 11.x as thatβs the latest development branch. And changes can be backported
- last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago Custom Commands Failed - Open on Drupal.org βEnvironment: PHP 8.2 & MySQL 8last update
over 1 year ago Waiting for branch to pass - π©πͺGermany Anybody Porta Westfalica
@Grevil did you read #56?
Any good reasons why you broke it again later in https://git.drupalcode.org/project/drupal/-/merge_requests/4030/diffs?co...?
I don't think it's a good idea here to provide the generic config as 2nd parameter in __construct() in this class where there's no DI.
Of course I might be wrong, but please leave a comment here then to explain the reasons. - last update
over 1 year ago Custom Commands Failed - Status changed to Needs work
over 1 year ago 8:24pm 23 May 2023 - π§πͺBelgium BramDriesen Belgium π§πͺ
Since we are adding dependencies to an existing interface, I guess we need a change record as this is a breaking change.
- π©πͺGermany Grevil
@Anybody, the class you manually implemented the "\Drupal::service" call does indeed not implement the "ContainerInjectionInterface". But that's because it is a service itself and the dependency injection happens through the service.yml (see "core/modules/path_alias/path_alias.services.yml" -> "path_alias.path_processor").
- last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago 29,306 pass, 15 fail - last update
over 1 year ago Custom Commands Failed - Status changed to Needs review
over 1 year ago 9:37am 24 May 2023 - π©πͺGermany Grevil
From the MR by @Anybody:
Are we sure the $path is passed as /<front> indeed? I didn't check that, but in the UI you typically use <front> without leading slash, so this should be double-checked! Perhaps write a test for this to be 100% sure, if not yet existing.
This is definitely important as "/" is quite untypical!
I don't have much time the next couple of weeks, so if anyone could add a test for this and do some general clean-up, go for it!
- Status changed to Needs work
over 1 year ago 3:39pm 25 May 2023 - πΊπΈUnited States smustgrave
Did not test
Issue summary needs updating
Change record
Release notes - last update
over 1 year ago Custom Commands Failed - π§πͺBelgium BramDriesen Belgium π§πͺ
Change record drafted: https://www.drupal.org/node/3362769 β
Release note added to the issue summary.Still needs work for the CCF.
- last update
over 1 year ago 29,332 pass, 19 fail - π§πͺBelgium BramDriesen Belgium π§πͺ
Will leave it ad needs work for the tests and issue summary updates.
- π¨πSwitzerland berdir Switzerland
I think this should not be in the path_alias module, which is now technically optional although most people are using it. It should be a separate event subscriber in core.
- π§π©Bangladesh DSushmita Sylhet
Solution:
1. Install and enable the "Metatag" module.
2. Configure the Metatag module by navigating to admin/config/search/metatag.
3. Define a custom token for the canonical URL by clicking on "Add custom token" in the Metatag configuration.
4. Set the custom token as [node:url:absolute] for the canonical URL on the front page.
5. Save the configuration and clear the Drupal caches.By following these steps, the Metatag module will allow you to define a custom token for the canonical URL. You can set this custom token as [node:URL: absolute] for the canonical URL on the front page. Saving the configuration and clearing the Drupal caches will ensure that the updated canonical URL settings take effect.
- π©πͺGermany Grevil
What's everyone's opinion on @berdir's comment in #66? What would be a good place for this Event Subscriber, if we would move the implementation to core?
- π©πͺGermany Anybody Porta Westfalica
Another related one: π Determine whether not only "/" but also "" should be valid input for the Link widget Active
- π«π·France andypost
Looks like duplicate but not really π setting path alias to `/` causes PHP8.1/str_starts_with/null error Needs work
- πͺπΈSpain manuel.adan π
manuel.adan β made their first commit to this issueβs fork.
- Status changed to Needs review
9 months ago 11:16am 20 April 2024 - πͺπΈSpain manuel.adan π
Another approach made to this in branch 1255092-front_page_outbound_url_path_processor, based as well in URL processor but outside of URL alias as suggested by Berdir at #66 π url() should return / when asked for the URL of the frontpage Needs work . I added outbound URL processing to the existing front page path processor.
It may fail in case of a front page path with query parameters were set, but it is also not well managed by other core parts like PathMatcher, see π Front page path with query parameters makes front path condition to fail Active . Added as related issue since support for query parameters should be added.
- Merge request !7619Resolve #1255092 "Front page outbound url path processor" β (Open) created by manuel.adan
- Status changed to Needs work
9 months ago 3:19pm 20 April 2024 - πΊπΈUnited States smustgrave
Will need test coverage and issue summary update.
Left some comments.
- π©πͺGermany Anybody Porta Westfalica
I'm asking myself if this could be fixed in contrib until it's solved in core one day.
Perhaps using OutboundPathProcessorInterface ( https://www.drupal.org/node/2238759 β )?
Any ideas? What do you think? - First commit to issue fork.
- πͺπΈSpain vidorado Pamplona (Navarra)
Sorry, three core tests are failing, and despite spending some time investigating, I havenβt been able to resolve it.
Drupal\Tests\system\Functional\System\ThemeTest::testThemeSettingsRenderCacheClear
Drupal\Tests\big_pipe\Functional\BigPipeTest::testNoJsDetection
Drupal\Tests\path_alias\Functional\UrlAlterFunctionalTest::testUrlAlter