Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim created an issue. See original summary.

Brolad’s picture

Agree with author, it's much better

joachim’s picture

Status: Needs review » Needs work

Thanks for the patch!

However, this needs a bit more work:

+++ b/src/FlagType/FlagTypeBase.php
@@ -134,13 +134,15 @@ abstract class FlagTypeBase extends PluginBase implements FlagTypePluginInterfac
+          '%flag_entity' => $flag->getFlaggableEntityTypeId(),

getFlaggableEntityTypeId() get you an entity ID which is a machine name, not a human-readable label.

You need to pass that entity ID to one of the entity type managers (I forget which) and get the plural label.

For example, instead of 'node' you want 'Content'; instead of 'taxonomy_term', 'Taxonomy terms'.

Also, nitpick, but use %flag_entity_type as your placeholder, rather than %flag_entity.

joachim’s picture

Status: Needs work » Postponed
joachim’s picture

Status: Postponed » Active
Brolad’s picture

Status: Active » Needs review
FileSize
1.12 KB

Status: Needs review » Needs work

The last submitted patch, 6: flag-2815097-detail_perm_labels-6.patch, failed testing. View results

Brolad’s picture

Status: Needs work » Needs review
FileSize
1.05 KB

Status: Needs review » Needs work

The last submitted patch, 8: flag-2815097-detail_perm_labels-8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

martin107’s picture

I think the test failures here are a sign of broken tests - so that should be fixed first

As a minor review nit-pick

\Drupal::entityTypeManager()

should be replaced by injecting the appropriate service into the constructor.