Problem/Motivation

symfony-cmf/routing 1.3.0 is not compatible with Symfony 3.

Proposed resolution

Update symfony-cmf/routing to 1.4.0 so that we can update to Symfony 3 later on.

Remaining tasks

Fix fails.
Review.
Commit.

User interface changes

None

API changes

Hopefully none

Data model changes

Not yet

CommentFileSizeAuthor
#19 interdiff.txt3.27 KBklausi
#19 update-cmf-2712633-19.patch6.33 KBklausi
#18 interdiff.txt1.21 KBklausi
#18 update-cmf-2712633-18.patch4.38 KBklausi
#16 interdiff.txt1.15 KBklausi
#16 update-cmf-2712633-15.patch4.27 KBklausi
#10 update-2712633-10.patch3.12 KBjibran
#7 update-2712633-7.patch3.82 KBjibran
#2 update-2712633-2.patch3.95 KBjibran

Comments

jibran created an issue. See original summary.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Active » Needs review
StatusFileSize
new update-2712633-2.patch3.95 KB

Here is the patch.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Issue summary: View changes
Related issues: +#2712637: Update stack/builder for Symfony 3 compatibility
jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Parent issue: » #2712647: Update Symfony components to ~3.2

Status: Needs review » Needs work

The last submitted patch, 2: update-2712633-2.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented
Component: base system » routing system

Oh wow, I would have not expected that something like this could actually break our routing. When we figure out what caused this change we should certainly create an issue on github, as other projects might run into that.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs work » Needs review
StatusFileSize
new update-2712633-7.patch3.82 KB

Reroll after #2712643: Update behat/* to 1.7.1 and fabpot/goutte to 3.1.2.

composer update symfony-cmf/routing
You are running composer with xdebug enabled. This has a major impact on runtime performance. See https://getcomposer.org/xdebug
Loading composer repositories with package information
Updating dependencies (including require-dev)
  - Removing symfony-cmf/routing (1.3.0)
  - Installing symfony-cmf/routing (1.4.0)
    Loading from cache

> Drupal\Core\Composer\Composer::vendorTestCodeCleanup
Writing lock file
Generating autoload files
> Drupal\Core\Composer\Composer::preAutoloadDump
> Drupal\Core\Composer\Composer::ensureHtaccess

Status: Needs review » Needs work

The last submitted patch, 7: update-2712633-7.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented

Mh, I tried to reproduce it, but it works on both d8.dev and localhost/d8

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs work » Needs review
StatusFileSize
new update-2712633-10.patch3.12 KB

Maybe instead of test we should fix what really is broken.

Status: Needs review » Needs work

The last submitted patch, 10: update-2712633-10.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented

@jibran
Have you been able to reproduce the failure locally?

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

Nope.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Assigned: Unassigned » klausi

ChainRouter in Symfony CMF has a new method rebuildRequest() which is used in the doMatch() method. On a DELETE request to a not existing node DefaultExceptionHtmlSubscriber is invoked. It tries to route the system 404 page which is restricted to GET and POST requests. With Symfony CMF 1.4 the request used internally to route system 404 is now built from the global request context and inherits the DELETE method. DELETE is not allowed for the 404 page ==> kaboom.

Proposed solution: manipulate the request context in DefaultExceptionHtmlSubscriber and set the method to GET. Let me try that.

dawehner’s picture

dawehner
Primary language German
commented

Urgs.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Assigned: klausi » Unassigned
Status: Needs work » Needs review
StatusFileSize
new update-cmf-2712633-15.patch4.27 KB
new interdiff.txt1.15 KB

Here we go.

Status: Needs review » Needs work

The last submitted patch, 16: update-cmf-2712633-15.patch, failed testing.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs work » Needs review
StatusFileSize
new update-cmf-2712633-18.patch4.38 KB
new interdiff.txt1.21 KB

Aha, the request context can sometimes be null. Whatever.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
StatusFileSize
new update-cmf-2712633-19.patch6.33 KB
new interdiff.txt3.27 KB

Or better fix the unit test by properly mocking the request context.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Reviewed & tested by the community

Seems good to me. Thanks @klausi.

dawehner’s picture

dawehner
Primary language German
commented

Nice work!

  • catch committed 5451d56 on 8.2.x 
    Issue #2712633 by klausi, jibran, dawehner: Update symfony-cmf/routing...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.2.x, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +8.2.0 release notes