Problem/Motivation

UUID fields don't specify a default_formatter

Proposed resolution

Add "basic_string" as "default_formatter"

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#27 interdiff-2450153-27.txt4.92 KBdamiankloip
#27 2450153-27.patch6.82 KBdamiankloip
#24 2450153-24.patch2.48 KBdamiankloip
#20 add_a_default_formatter-2450153_tests-20.patch552 bytesanksy
#11 interdiff-2450153-2-11.txt591 bytesjosephleon
#11 2450153-UUIDs-default-formatter-11.patch582 bytesjosephleon
#2 2450153-UUIDs-default-formatter-2.patch544 bytesjoshi.rohit100
#9 2450153-UUIDs-default-formatter-5.patch583 bytesjosephleon
#9 interdiff-2450153-2-5.txt513 bytesjosephleon
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Issue summary: View changes
Issue tags: +Novice

.

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
CreditAttribution: joshi.rohit100 commented
Status: Active » Needs review
FileSize
2450153-UUIDs-default-formatter-2.patch544 bytes

Status: Needs review » Needs work

The last submitted patch, 2: 2450153-UUIDs-default-formatter-2.patch, failed testing.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

There's a missing trailing comma after no_ui = TRUE.

josephleon’s picture

josephleon CreditAttribution: josephleon commented

I am starting work on this.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
Issue tags: +Needs tests 
+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/UuidItem.php
@@ -19,6 +19,7 @@
+ *   default_formatter = "basic_string",

Actually a trailing comma for the last item will break annotation parsing. The easiest thing would be to move this line before no_ui.

amateescu’s picture

amateescu CreditAttribution: amateescu commented

@plach, that's not true anymore, @chx fixed it in Doctrine :)

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Actually a trailing comma for the last item will break annotation parsing.

This is not longer the case, see https://github.com/doctrine/annotations/pull/11

josephleon’s picture

josephleon CreditAttribution: josephleon commented
Status: Needs work » Needs review
FileSize
interdiff-2450153-2-5.txt513 bytes
2450153-UUIDs-default-formatter-5.patch583 bytes
dinarcon’s picture

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

Thanks @josephleon

+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/UuidItem.php
@@ -18,7 +18,8 @@
+ *   default_formatter = "basic_string",

Could you please remove the trailing comma?

josephleon’s picture

josephleon CreditAttribution: josephleon commented
FileSize
2450153-UUIDs-default-formatter-11.patch582 bytes
interdiff-2450153-2-11.txt591 bytes
josephleon’s picture

josephleon CreditAttribution: josephleon commented
Status: Needs work » Needs review
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Mh, I'm not sure whether we should really drip the last comma.

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented

dawehner before it didn't have a trailing comma

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
Status: Needs review » Needs work

Comma or not we need some additional test coverage :)

josephleon’s picture

josephleon CreditAttribution: josephleon commented

I'm new to testing, still reading documentation. Will try and tackle it though.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented

Thanks!

anksy’s picture

anksy CreditAttribution: anksy commented

I am a novice working on writing a test for this issue and I cant find any test files where tests for this issue will fit therefore I have created a file for tests related to Uuid formatting. It has the following address "core/modules/field/src/Tests/UuidFormatterTest.php". In the absence of a more appropriate name for the test file I went with "UuidFormatterTest.php" until I can find a more appropriate name.

josephleon’s picture

josephleon CreditAttribution: josephleon commented

@anksy I was running into the same issue. I would be more than happy to work with you on that. Could you post what you have so far?

anksy’s picture

anksy CreditAttribution: anksy commented
Status: Needs work » Needs review
FileSize
add_a_default_formatter-2450153_tests-20.patch552 bytes

@josephleon, Here is the progress that I have made so far. Will need some guidance on writing further tests.

Status: Needs review » Needs work

The last submitted patch, 20: add_a_default_formatter-2450153_tests-20.patch, failed testing.

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
CreditAttribution: joshi.rohit100 commented
Assigned: Unassigned » joshi.rohit100
Issue tags: +SprintWeekend2015
joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
CreditAttribution: joshi.rohit100 commented
Assigned: joshi.rohit100 » Unassigned

I am not sure whether this should have a test case or not as I am seeing that Email field is also using the basic_string formatter and there is not formatter testcase for this as we have StringFormatterTest class and I think, this will do the thing (If I am not wrong).

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Needs work » Needs review
FileSize
2450153-24.patch2.48 KB

How about some simple test coverage like this?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented 
  1. +++ b/core/modules/field/src/Tests/String/UuidFormatterTest.php
@@ -0,0 +1,61 @@
+/**
+ * Class UuidFormatterTest
+ * @package Drupal\field\Tests\String
+ */

    Mh, default output 

  2. +++ b/core/modules/field/src/Tests/String/UuidFormatterTest.php
@@ -0,0 +1,61 @@
+    $render_array = $uuid_field->view(['settings' => ['link_to_entity' => TRUE]]);
+    $this->assertIdentical($render_array[0]['#type'], 'link');
+    $this->assertIdentical($render_array[0]['#title'], $entity->uuid());
+    $this->assertIdentical($render_array[0]['#url']->toString(), $entity->url());

    Note: #2353611: Make it possible to link to an entity by UUID is some sort of issue which would be great to get in but just sorta related.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

One thing we could do as well here is to uncomment all the uuid lines of subclasses of \Drupal\views\Tests\Handler\FieldFieldAccessTestBase

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Issue tags: -Novice, -Needs tests
FileSize
2450153-27.patch6.82 KB
interdiff-2450153-27.txt4.92 KB

Here, we go. If we are enabling those lines in the tests we need to special case for the uuid field in FieldFieldAccessTestBase.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Thank you a lot!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Category: Task » Bug report
Status: Reviewed & tested by the community » Fixed

A missing formatter for something we want to output in views is a bug. Committed f2fdaeb and pushed to 8.0.x. Thanks!

diff --git a/core/modules/field/src/Tests/String/UuidFormatterTest.php b/core/modules/field/src/Tests/String/UuidFormatterTest.php
index 15956c0..2eaf7e9 100644
--- a/core/modules/field/src/Tests/String/UuidFormatterTest.php
+++ b/core/modules/field/src/Tests/String/UuidFormatterTest.php
@@ -8,12 +8,8 @@
 namespace Drupal\field\Tests\String;

 use Drupal\simpletest\KernelTestBase;
-use Drupal\Core\Entity\FieldableEntityInterface;
-use Drupal\Core\Entity\Display\EntityViewDisplayInterface;
-use Drupal\Component\Utility\Unicode;
-use Drupal\field\Entity\FieldConfig;
-use Drupal\field\Entity\FieldStorageConfig;
 use Drupal\entity_test\Entity\EntityTest;
+
 /**
  * Tests the output of a UUID field.
  *

Fixed all the unused uses on commit.

  • alexpott committed f2fdaeb on 8.0.x 
    Issue #2450153 by josephleon, damiankloip, joshi.rohit100, anksy: Add a...

Status: Fixed » Closed (fixed)

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