Problem/Motivation

Lots of locations refer to a route and reconstruct it, with node special cases.
Recent improvements on navigation and selection duplicated even more code.

Proposed resolution

Don't building the routes with repeating custom code when building the navigation.
We have a diffRoute on the generic revision controller that is dedicated for this. Make it static or convert it to a trait and reuse it. :-)

API changes

Change in diffRoute.

CommentFileSizeAuthor
#10 use_diffroute_instead-2799923-10.patch5.51 KBtoncic
#9 interdiff-2799923-4-9.txt2.03 KBtoncic
#9 use_diffroute_instead-2799923-9.patch5.47 KBtoncic
#6 interdiff-2799923-2-4.txt3.05 KBtoncic
#4 interdiff-2799923-2-4.txt0 bytestoncic
#4 use_diffroute_instead-2799923-4.patch5.44 KBtoncic
#2 use_diffroute_instead-2799923-2.patch4.59 KBtoncic

Comments

miro_dietiker created an issue. See original summary.

toncic’s picture

toncic commented
Assigned: Unassigned » toncic
Status: Active » Needs review
StatusFileSize
new use_diffroute_instead-2799923-2.patch4.59 KB

I switched to use diffRoute instead of some duplicated code.

Status: Needs review » Needs work

The last submitted patch, 2: use_diffroute_instead-2799923-2.patch, failed testing.

toncic’s picture

toncic commented
Status: Needs work » Needs review
StatusFileSize
new use_diffroute_instead-2799923-4.patch5.44 KB
new interdiff-2799923-2-4.txt0 bytes

Trying to fix test failing.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented
Status: Needs review » Needs work 
+++ b/src/Controller/PluginRevisionController.php
@@ -277,7 +277,8 @@ class PluginRevisionController extends ControllerBase {
+  public static function  diffRoute(EntityInterface $entity, $left_revision_id, $right_revision_id, $layout = NULL, $layout_options = NULL) {

Extra space between function and diffRoute

+++ b/src/Plugin/diff/Layout/HTMLDiffLayout.php
@@ -113,22 +114,18 @@ class HTMLDiffLayout extends DiffLayoutBase {
-          ['view_mode' => $view_mode]

Don't remove the layout_options here.

toncic’s picture

toncic commented
StatusFileSize
new interdiff-2799923-2-4.txt3.05 KB

Sorry for empty interdiff.

The last submitted patch, 4: use_diffroute_instead-2799923-4.patch, failed testing.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented 
+++ b/src/DiffLayoutBase.php
@@ -144,21 +145,24 @@ abstract class DiffLayoutBase extends PluginBase implements DiffLayoutInterface,
+        'raw'
...
+         'strip_tags'

This should be an array, keep the one that was before.

toncic’s picture

toncic commented
Status: Needs work » Needs review
StatusFileSize
new use_diffroute_instead-2799923-9.patch5.47 KB
new interdiff-2799923-4-9.txt2.03 KB

This should works.

toncic’s picture

toncic commented
StatusFileSize
new use_diffroute_instead-2799923-10.patch5.51 KB

Rebase patch.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Status: Needs review » Fixed

Committing this.

Dunno if it's worth making it a trait. It's about path management that is highly related with the Controller.

Status: Fixed » Closed (fixed)

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