Problem/Motivation

Followup of #2350837: Convert most usages of EntityInterface::getSystemPath() to use routes

Proposed resolution

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#19 Home___Site-Install.png770.1 KBLewisNyman
#11 2372899-fix-broken-head.patch1.54 KBamateescu
#5 interdiff-3-5.txt535 bytesmpdonadio
#5 pagecachetagstestbase-2372899-5.patch28.61 KBmpdonadio
#3 page-cache-tags-tests-url-2372899-3.patch28.61 KBBerdir
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

znerol’s picture

znerol CreditAttribution: znerol commented
Title: PageCacheTestBase should use url objects » PageCacheTagsTestBase should use url objects
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Title: PageCacheTagsTestBase should use url objects » PageCacheTagsTestBase should use Url objects
Status: Active » Postponed
Issue tags: +Novice

I think it makes sense to postpone this on #2350837: Convert most usages of EntityInterface::getSystemPath() to use routes.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Postponed » Needs review
Issue tags: -Novice
FileSize
page-cache-tags-tests-url-2372899-3.patch28.61 KB

First patch, I don't really know why tour tests are not working?

Status: Needs review » Needs work

The last submitted patch, 3: page-cache-tags-tests-url-2372899-3.patch, failed testing.

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
CreditAttribution: mpdonadio commented
Status: Needs work » Needs review
FileSize
pagecachetagstestbase-2372899-5.patch28.61 KB
interdiff-3-5.txt535 bytes

Simple fix for the route name in the test.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

Ouch. I did update that, but I probably did that in the wrong project :(

Berdir--.

amateescu’s picture

amateescu CreditAttribution: amateescu commented
Status: Needs review » Reviewed & tested by the community

The patch looks good but we will need a beta evaluation before commit (only changes test code).

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Test code is not frozen in beta. Committed 86f62c1 and pushed to 8.0.x. Thanks!

  • alexpott committed 86f62c1 on 8.0.x 
    Issue #2372899 by mpdonadio, Berdir: PageCacheTagsTestBase should use...
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

MHHH https://qa.drupal.org/pifr/test/953568

amateescu’s picture

amateescu CreditAttribution: amateescu commented
Title: PageCacheTagsTestBase should use Url objects » HEAD BROKEN: PageCacheTagsTestBase should use Url objects
Priority: Normal » Critical
Status: Fixed » Needs review
FileSize
2372899-fix-broken-head.patch1.54 KB

This should do it.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Status: Needs review » Reviewed & tested by the community

if bot agrees

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

+1

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.0.x. Thanks!

amateescu’s picture

amateescu CreditAttribution: amateescu commented
Title: HEAD BROKEN: PageCacheTagsTestBase should use Url objects » PageCacheTagsTestBase should use Url objects
Priority: Critical » Normal

  • webchick committed a6d172f on 8.0.x 
    Issue #2372899 follow-up by amateescu: HEAD BROKEN: Fix names of things

Status: Fixed » Needs work

The last submitted patch, 11: 2372899-fix-broken-head.patch, failed testing.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs work » Fixed
LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
FileSize
Home___Site-Install.png770.1 KB

I'm not sure if this is the right issue but I noticed the people icon in the toolbar is broken. The HTML class has changed somehow

amateescu’s picture

amateescu CreditAttribution: amateescu commented

@LewisNyman, we're fixing that in #2410441: 'People' icon missing from toolbar :)

Status: Fixed » Closed (fixed)

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