Problem/Motivation

After content translation removal we need to remove the path for the given translation language.
This happens actually in core/modules/content_translation/src/Form/ContentTranslationDeleteForm, but should happen in path module because if is not there the deletion might not happen, for example using a drush command.

Proposed resolution

Remove the path module check and move that functionality into path.module

Remaining tasks

Contributor tasks needed
Task Novice task? Contributor instructions Complete?
Create a patch Novice Instructions
Update the issue summary noting if allowed during the beta Instructions
Add automated tests Instructions
Manually test the patch Novice Instructions

User interface changes

None.

API changes

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because if a translation gets deleted not using the form submit we get an orphan alias
Issue priority Major because is not he expected result at all.
Unfrozen changes Unfrozen because it only moves code to the correct place.

None.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fran seva’s picture

Issue summary: View changes
fran seva’s picture

Issue summary: View changes
fran seva’s picture

Issue summary: View changes
keopx’s picture

Assigned: Unassigned » keopx
fran seva’s picture

Assigned: keopx » Unassigned
D Szkiba’s picture

I work on this on drupal dev days.

bogdan.racz’s picture

Assigned: Unassigned » bogdan.racz
bogdan.racz’s picture

Assigned: bogdan.racz » Unassigned
D Szkiba’s picture

Assigned: Unassigned » D Szkiba
rodrigoaguilera’s picture

Issue summary: View changes
Issue tags: +Needs issue summary update

Since this is a normal task is at risk for not being allowed to not be committed in the beta it needs a beta evaluation.

D Szkiba’s picture

Moved the function to path.module. However, this patch still contains the deprecated function getInternalPath so it's not finished yet.

rodrigoaguilera’s picture

Status: Active » Needs review
rodrigoaguilera’s picture

Category: Task » Bug report
Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs tests

Added more motivation for the change.
While writing it I realized that this is probably a bug since there might be situations where the path is not deleted when the translation is deleted. Any case that doesn't use the form.

We can also write tests for it deleting the translation using code instead of the form.

The code looks ok, only tests letft to do

rodrigoaguilera’s picture

Issue summary: View changes
D Szkiba’s picture

This patch contains only the test.

D Szkiba’s picture

Status: Needs work » Needs review
FileSize
2.74 KB

There is still an error in the test, I'm just posting this to be able to test it with simpletest.

fran seva’s picture

Status: Needs review » Needs work
D Szkiba’s picture

Status: Needs work » Needs review
FileSize
2.75 KB

Thats (hopefully) the final version. "$this" was missing before assertFalse, thx fran for helping!

D Szkiba’s picture

D Szkiba’s picture

FileSize
915 bytes

The last submitted patch, 16: path_deletion_should_be-2470952-16.patch, failed testing.

fran seva’s picture

Issue tags: -Needs tests

The code is good for me.

fran seva’s picture

Issue summary: View changes
fran seva’s picture

Assigned: D Szkiba » Unassigned
rodrigoaguilera’s picture

Issue summary: View changes
Priority: Normal » Major
Status: Needs review » Reviewed & tested by the community

Code looks great, thanks for working on it.

alexpott’s picture

Title: Path deletion should be removed in path module after content translation removal » Path deletion should be removed in path module after content translation removal
Status: Reviewed & tested by the community » Fixed

Committed 612581d and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.

diff --git a/core/modules/path/path.module b/core/modules/path/path.module
index 707481f..494c338 100644
--- a/core/modules/path/path.module
+++ b/core/modules/path/path.module
@@ -80,7 +80,7 @@ function path_entity_base_field_info(EntityTypeInterface $entity_type) {
 }
 
 /**
- * Implements hook_entity_translation_delete()
+ * Implements hook_entity_translation_delete().
  */
 function path_entity_translation_delete(EntityInterface $translation) {
   $path = $translation->urlInfo()->getInternalPath();

Added full stop for coding standards.

  • alexpott committed 612581d on 8.0.x
    Issue #2470952 by D Szkiba: Path deletion should be removed in path...
Gábor Hojtsy’s picture

Issue tags: -sprint

Thanks!

Status: Fixed » Closed (fixed)

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