Updated: Comment 0

Problem/Motivation

We have a new local actions API, but most of core is not yet converted.

Proposed resolution

Convert them in order to find potential problems as well as remove the old system out of menu.inc

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#23 local_actions-2100073.patch31.04 KBdawehner
#18 local_actions-2100073-18.patch31.08 KBdawehner
#18 interdiff-18.txt1.63 KBdawehner
#17 interdiff-17.txt5.81 KBdawehner
#17 local_actions-2100073-17.patch35.26 KBdawehner
#14 local_action-2100073-14.patch29.89 KBdawehner
#13 local_action-2100073-13.patch29.52 KBdawehner
#13 interdiff.txt1.02 KBdawehner
#11 interdiff.txt18 KBdawehner
#11 local_actions-2100073-11.patch29.71 KBdawehner
#9 local_action-2100073-9.patch21.23 KBdawehner
#9 interdiff.txt4.31 KBdawehner
#7 local_action-2100073-7.patch16.92 KBdawehner
#7 interdiff.txt2.25 KBdawehner
#5 local_action-2100073-5.patch15.38 KBdawehner
#3 local_actions-2100073-1.patch13.47 KBdawehner
#3 local_actions-2100073-2.patch14.21 KBdawehner
#3 interdiff.txt696 bytesdawehner
#1 local_actions-2100073-1.patch13.47 KBdawehner
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Active » Needs review
Issue tags: +Needs manual testing
FileSize
local_actions-2100073-1.patch13.47 KB

There we go.

Notes:

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Issue tags: +MENU_LOCAL_ACTION

.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
interdiff.txt696 bytes
local_actions-2100073-2.patch14.21 KB
local_actions-2100073-1.patch13.47 KB

This fixes a problem on passing in the proper value of the route name.

Status: Needs review » Needs work

The last submitted patch, local_actions-2100073-2.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Needs review
FileSize
local_action-2100073-5.patch15.38 KB

Some more work.

Status: Needs review » Needs work

The last submitted patch, local_action-2100073-5.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Needs review
FileSize
interdiff.txt2.25 KB
local_action-2100073-7.patch16.92 KB

I realized that we need to get rid of the subrequest for the dialog controller in order to allow to get the title from the route definition.
This would also happen on the NormalizeView issue so we might have to wait on that one.

Status: Needs review » Needs work

The last submitted patch, local_action-2100073-7.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Needs review
FileSize
interdiff.txt4.31 KB
local_action-2100073-9.patch21.23 KB

Removed some of the tests which still dealt with the assumptions of local actions being in the {menu_links} table

Status: Needs review » Needs work

The last submitted patch, local_action-2100073-9.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Priority: Normal » Major
Status: Needs work » Needs review
FileSize
local_actions-2100073-11.patch29.71 KB
interdiff.txt18 KB

Let's rip out the rest of local actions. I personally think that the existing test is just wrong. If you request for admin/structure/content/add you expect the title of the dialog to be "Add category".

Bumping to major as this rips out the old local action system.

RoSk0’s picture

RoSk0
he/him
Primary language Russian
Location Wellington
CreditAttribution: RoSk0 commented 
+++ b/core/modules/aggregator/aggregator.module
@@ -17,6 +17,7 @@
+  debug($path);

Does this belong here?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
interdiff.txt1.02 KB
local_action-2100073-13.patch29.52 KB

Good catch!

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
local_action-2100073-14.patch29.89 KB

.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Assigned: Unassigned » tim.plunkett

This still needs manual testing, and I will do that later tonight, unless someone swoops in before me.

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented
Issue summary: View changes
Issue tags: -Needs manual testing

Code-wise this looks good.
i manual-tested though and evrything work, but one thing: the install modules action in the Extend page...all other actions are there and link where they should

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
local_actions-2100073-17.patch35.26 KB
interdiff-17.txt5.81 KB

Let's add some tests to ensure this works.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
interdiff-18.txt1.63 KB
local_actions-2100073-18.patch31.08 KB

Ignore the last tests.

Status: Needs review » Needs work

The last submitted patch, 18: local_actions-2100073-18.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Needs review

18: local_actions-2100073-18.patch queued for re-testing.

ParisLiakos’s picture

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

cool, thanks! lets get this in then.

catch’s picture

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

No longer applies.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Reviewed & tested by the community
FileSize
local_actions-2100073.patch31.04 KB

patch -p1 still worked.

catch’s picture

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

Committed/pushed to 8.x, thanks!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Issue tags: +alpha target

Since this falls under "completing the routing system," would be great to target this for alpha5 (Nov 18). Tagging.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

Oh, man. Sorry. The block still said RTBC. :\

Well. Success! ;)

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 23: local_actions-2100073.patch, failed testing.

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented
Status: Needs work » Needs review

23: local_actions-2100073.patch queued for re-testing.

ParisLiakos’s picture

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

back to rtbc, since it was never committed

catch’s picture

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

Was committed, but my push failed :(

Really pushed now though!

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Issue summary: View changes
Parent issue: » #2107533: Remove {menu_router}

Status: Fixed » Closed (fixed)

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