Add new route implementation #7726
Conversation
766aa63 to
fcfe31e
Compare
2fd552e to
a72f63e
Compare
| @@ -36,7 +36,7 @@ | |||
| "doctrine/cache": "^1.12.1 || ^2.0", | |||
| "doctrine/collections": "^1.6 || ^2.0", | |||
| "doctrine/data-fixtures": "^1.4", | |||
| "doctrine/dbal": "^3.3", | |||
| "doctrine/dbal": "^3.6", | |||
There was a problem hiding this comment.
Deprecated
Connection::PARAM_*_ARRAYconstants: Use the corresponding constants onArrayParameterTypeinstead.
So I also replaced it in AccessControlRepository
The 3.6 was released 2 years ago: https://github.com/doctrine/dbal/releases/3.6.0 so we are fine to upgrade.
| @@ -87,7 +88,7 @@ public function findIdsWithGrantedPermissions( | |||
| ->andWhere('accessControl.role IN (' . $systemRoleQueryBuilder->getDQL() . ')') | |||
| ->setParameter('system', $system) | |||
| ->setParameter('entityClass', $entityClass) | |||
| ->setParameter('entityIds', $entityIds, Connection::PARAM_STR_ARRAY) | |||
| ->setParameter('entityIds', $entityIds, ArrayParameterType::STRING) | |||
There was a problem hiding this comment.
DoctrineDBAL:
Deprecated
Connection::PARAM_*_ARRAYconstants: Use the corresponding constants onArrayParameterTypeinstead.
cea29b3 to
ee76ab4
Compare
0b60c03 to
4986b13
Compare
|
|
||
| <field name="site" type="string" length="16" nullable="true"/> | ||
| <field name="locale" type="string" length="11"/> | ||
| <field name="slug" type="string"/> |
There was a problem hiding this comment.
To support or versions of mysql site + locale + slug can not be longer together as 191 chars. Which means in this case 191 - 16 -11 = 164 chars for the slug.
There would be one workaround to support longer versions of URL by using a additional slugHash field based on widely supported SHA1 algorithm. That way we can support longer Urls 164.
@benr77 already requested that the current title of 64 is to small for his usecases. If we increase that to 128 or 191. A longer parent title and a child page or 3 levels of navigation could already hit the 191 limit and we would require trim the url correctly. But that is more a theoretical issue. Which may already existed before.
There was a problem hiding this comment.
Interesting the workaround with a Hash is also documented in MySQL docs itself but as a alternative for composite indexes so n our case it would include the site, locale, slug: https://dev.mysql.com/doc/refman/8.4/en/multiple-column-indexes.html
84c2072 to
6df21e0
Compare
2b38bd9 to
0f49c5a
Compare
packages/route/src/Infrastructure/Doctrine/EventListener/RouteChangedUpdater.php
Outdated
Show resolved
Hide resolved
1d7772d to
cf6a3f6
Compare
cf6a3f6 to
5773a20
Compare
aff9fbd to
ef19b66
Compare
packages/route/src/Infrastructure/Doctrine/EventListener/RouteChangedUpdater.php
Outdated
Show resolved
Hide resolved
packages/route/src/Infrastructure/Doctrine/Repository/RouteRepository.php
Outdated
Show resolved
Hide resolved
Co-authored-by: Prokyonn <daniel.mathis@sector8.eu>
|
8.4 error is unrelated to the PR. Looks like some issues in one of our dependencies maybe only a dev anomaly but we should test what is causing it symfony or the 2fa bundle. |
What's in this PR?
Add new route implementation.
Why?
I did some experimenting to simplify the route handling in https://github.com/alexander-schranz/route-prototype. Instead of that the developer need manually care about updating things we will work with doctrine listeners which will update child urls automatically to avoid issues like:
sulu_navigation_root_tree#7580Also we will not longer have a difference between unpublished or published URLs. Currently in Article Bundle I could create 2 articles with the url
/testand/testand it did only error when try to publish one. This should not longer be the case as we already reserving and create a unique URL when saving the Entity and does not end in publishing errors specially when try using in combination with the AutomationBundle.ToDo
/testand Site B/testand article parent Site A/test/articleshould not be updated when Site B/testwas changedToDos in own Pull Requests