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

Comments

dawehner’s picture

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

There we go.

Notes:

dawehner’s picture

dawehner
Primary language German
commented
Issue tags: +MENU_LOCAL_ACTION

.

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new interdiff.txt696 bytes
new local_actions-2100073-2.patch14.21 KB
new 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
commented
Status: Needs work » Needs review
StatusFileSize
new 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
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt2.25 KB
new 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
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt4.31 KB
new 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
commented
Priority: Normal » Major
Status: Needs work » Needs review
StatusFileSize
new local_actions-2100073-11.patch29.71 KB
new 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
commented 
+++ b/core/modules/aggregator/aggregator.module
@@ -17,6 +17,7 @@
+  debug($path);

Does this belong here?

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new interdiff.txt1.02 KB
new local_action-2100073-13.patch29.52 KB

Good catch!

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new local_action-2100073-14.patch29.89 KB

.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
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 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
commented
StatusFileSize
new local_actions-2100073-17.patch35.26 KB
new interdiff-17.txt5.81 KB

Let's add some tests to ensure this works.

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new interdiff-18.txt1.63 KB
new 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
commented
Status: Needs work » Needs review

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

ParisLiakos’s picture

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
commented
Status: Reviewed & tested by the community » Needs work

No longer applies.

dawehner’s picture

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

patch -p1 still worked.

catch’s picture

catch
he/him
Primary language English
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 🇨🇦
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 🇨🇦
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 commented
Status: Needs work » Needs review

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

ParisLiakos’s picture

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
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
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.