joomla / joomla-cms

Home of the Joomla! Content Management System
https://www.joomla.org
GNU General Public License v2.0
4.77k stars 3.65k forks source link

[5.3][SEF] Remove parent_key from url in NoMenuRules #44460

Open heelc29 opened 1 week ago

heelc29 commented 1 week ago

Summary of Changes

the new preprocessrule (#43992) adds the parent key during build process but if no menu item exist for this component there is no redirect to sef url

This reverts commit #4f2a09ef42676f91647ebecdef27bf1e73117e17 and restore same behavior like in J5.2

Testing Instructions

Actual result BEFORE applying this Pull Request

no redirect to sef url

Expected result AFTER applying this Pull Request

redirect to NoMenuRules sef url: /index.php/component/contact/contact/{contact.alias}

Link to documentations

Please select:

Hackwar commented 1 week ago

Unfortunately this isn't that easy. Simply removing this at this point is a break in b/c for the routing. Yes, the query parameter there has been wrong and it has been wrong for years, also already in Joomla 3, but simply removing it means lots of errors in the google search console, so we should carefully consider how to handle this. I've been thinking about this for a long time already and don't have a satisfying solution for this. The problem is that we also need the information about the category for URLs without IDs. The way the PR is now, it can't be merged unfortunately.

richard67 commented 1 week ago

@Hackwar Maybe I misunderstand something, or I lack some knowledge, but how can this PR be a b/c break or make problems in Google search if this PR reverts a change which was made in 5.3-dev to what we have in 5.2?

richard67 commented 3 days ago

@heelc29 I allowed myself to update your branch to the latest changes in 5.3-dev and have resolved conflicts.

Now the system tests are failing at the last test:

1) Test in frontend that the contact site router
       can process contact with legacy routing:
     AssertionError:
       expected 'http://localhost/cmysql/index.php/component/contact/contact/38-test-contact-router-test-contact-router'
       to match /\/index.php\/component\/contact\/contact\/38-test-contact-router$/
      at __webpack_modules__</</overrideChaiAsserts/ (http://localhost/__cypress/runner/cypress_runner.js:139310:25)
      at overwritingMethodWrapper (http://localhost/__cypress/runner/cypress_runner.js:79011:33)
      at  (webpack://joomla/./tests/System/integration/site/components/com_contact/Router.cy.js:172:44)

It seems the "test-contact-router" is appended 2 times in the URL.