Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
telephone.module
Priority:
Normal
Category:
Task
Assigned:
Issue tags:
Reporter:
Created:
4 Feb 2014 at 21:27 UTC
Updated:
29 Jul 2014 at 23:21 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
lokapujyaBasic implementation of telephone field formatter test. Working with Kay @rdfcodesprint.
Comment #2
lokapujyaFor manual testing, the field mapping needs to be created manually by editing the rdf.mapping.node.[content type].yml file by copying a field mapping (last 3 lines of the file) and changing the schema.
Comment #3
scor commentedWe should step down in the telephone field formatter because the value of the link can be changed in the field formatter settings, here is an example:
In this case, if you paste this markup in the RDFa play tool and look at the raw data tab, you will see that the string "Call me maybe" is extracted, not the telephone value. This is because RDFa parsing will extract the text inside the div element. This is easy to solve by placing the property attribute in the a element. This is similar to how taxonomy's LinkFormatter.php does it.
Comment #5
lokapujyaCombined the asserts into one test.
Comment #6
lokapujyaThis code handles the issue scor mentioned in #3:
We still need a test for this.
Comment #7
lokapujyaComment #8
lokapujyaI need to fix the last patch, since some of the email formatter code got mixed in.
Also, I'm reading this issue: #1778122: Enable modules to inject attributes into field formatters, so that RDF attributes get output to better understand how to set up the test the attributes issue from comment #3.
Comment #9
lokapujyaFixed up the patch.
I think we need a test that asserts that rdf reads the number from the href and not the value from the div ("Call me maybe.") I am not sure how we would set the title for the telephone field in a test. I think it can be done in the entity_create() or by setting $this->entity->{$this->fieldName}->title?
Comment #11
scor commentedThe test failed because now that we've moved the property attribute inside the a HTML element, we're extracting a link in the RDFa. The expected value (which will need to be adjusted) is the content of the href attribute (tel:123) and the 4th argument of assertFormatterRdfa() will have to be set to 'uri' (the default is literal aka string).
Comment #12
lokapujyaImplemented the suggestion in #11.
Comment #13
scor commentedHere is an example similar to what we have to do (taken from another test in core):
Here I believe we just need to set $formatter_settings to be something like array('title' => 'Contact us'). Since we need to be able to do that we will have to change the signature of assertFormatterRdfa so that formatter is not longer just the machine name of the formatter to test (string), but an array describing the formatter like above: type and settings (we don't really care about the label).
I've created an issue to take care of that: #2203065: Adjust assertFormatterRdfa() parameters to allow for more advanced testing.
You can view how the settings of the field formatters are composed in entity.view_display.node.article.default.yml. I believe all you have to specify is for the field formatter settings to include a 'title' key set to your string, e.g. "Contact us".
This is the caller code:
Comment #14
lokapujyaI created a test that passes. But, do I need to use datatype? I have it commented out below.
Comment #15
scor commentedNo, in the case of the telephone field formatter, we won't need to use datatype because the expected value will not have a datatype. We will need the datatype for the date field issue only (note that the date field issue is RTBC at the moment as it implements a workaround for #2203065: Adjust assertFormatterRdfa() parameters to allow for more advanced testing - we will update the date field code in #2203065 once the date field patch is committed).
Comment #16
lokapujyaAdded a test and rerolled for the updated assertFormatterRdfa().
Comment #17
lokapujyaComment #18
scor commentedGood job @lokapujya! This builds on top of #2203065: Adjust assertFormatterRdfa() parameters to allow for more advanced testing and is good to go...
Comment #19
alexpottCommitted 85392bf and pushed to 8.x. Thanks!
Fixed during commit. The use is unnecessary since the classes are in the same namespace. Class properties shouldn't really be declared dynamically.