Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DataTransformer cookbook – strip_tags is a bad example? #6431

Closed
MacDada opened this issue Apr 4, 2016 · 15 comments
Closed

DataTransformer cookbook – strip_tags is a bad example? #6431

MacDada opened this issue Apr 4, 2016 · 15 comments
Labels
actionable Clear and specific issues ready for anyone to take them. bug Form Hack Day hasPR A Pull Request has already been submitted for this issue.

Comments

@MacDada
Copy link
Contributor

MacDada commented Apr 4, 2016

The cookbook suggests to use DataTransformer to strip_tags from input.

Isn't that violation of "Transforms a value between different representations"?

I mean, when u strip tags, it's not that representation of data changes – it's the data itself that gets changed. Stripping tags is a losing data procedure.

A cleaner way would be to use form event listener to change the data.

The example is almost right to use DataTransformer to change '<br>'s to "\n" and vice versa – almost, because the input could have both '<br>'s and "\n" – the original information, which should be where, would be lost as well.

@xabbuh
Copy link
Member

xabbuh commented Apr 5, 2016

You are right. Stripping some information means that the transformer no longer is bijective. Can we come up with a better example use case?

@HeahDude
Copy link
Contributor

HeahDude commented Apr 5, 2016

The use case of @dmaicher described in symfony/symfony#18173 might fit. What do you think ?

@MacDada
Copy link
Contributor Author

MacDada commented Apr 5, 2016

@HeahDude I think it should be something simpler, more obvious.

How about base64_encode/base64_decode?

@javiereguiluz
Copy link
Member

Another proposal: implode() / explode() to turn strings into/from arrays.

@HeahDude
Copy link
Contributor

HeahDude commented Apr 5, 2016

I don't use transformers myself for now, but I thought that giving a doctrine ArrayCollection to a field and expecting the form to treat it as an array and give you back a collection might help others (new comers?) dealing with entities in any form type.

That does not seem that complicated to me :)

In any case I think the example should match real use cases.

@MacDada @javiereguiluz Have you an example where base64 encoding/decoding or array implode/explode might be useful? I guess there must be some :)

What about simple prefix or suffix used in model data but not in an editable form representation:

class PrefixDataTransformer implements DataTransformerInterface
{
    private $prefix;

    public function __construct($prefix = null)
    {
        $this->prefix = $prefix;
    }

    public function transform($value)
    {
        if (null === $value || '' === $value) {
            return;
        }

        if (0 === strpos($value, $this->prefix) {
            return substr($value, strlen($this->prefix));
        }

        return $value
    }

    public function reverseTransform($value)
    {
        if (null === $value || '' === $value) {
            return;
        }

        if (0 !== strpos($value, $this->prefix) {
            return $this->prefix.$value;
        }

        return $value
    }

?

@javiereguiluz
Copy link
Member

@HeahDude I used the implode/explode example myself in the past because I had a tags property of type array in one Doctrine entity and I wanted to edit it in a <input text> field separating the elements with commas. Maybe there's a better solution for this ... but the Form component is not my strong suit.

@linaori
Copy link
Contributor

linaori commented Apr 5, 2016

I've had a case with bank account numbers, people tend to enter them with spaces in between, we basically have a transformer that does a 1 way "clean-up" when posting the data and keeps this information. Might be a nice example too.

@xabbuh
Copy link
Member

xabbuh commented Apr 5, 2016

@iltar The issue with the bank account number example again is that it only works well in one direction (you cannot add back removed spaces).

I like Javier's example with tags (we already some parts in the docs that use tags as an example too and this also is something that is easy to understand).

@HeahDude
Copy link
Contributor

HeahDude commented Apr 5, 2016

@javiereguiluz 👍

@MacDada
Copy link
Contributor Author

MacDada commented Apr 5, 2016

How about a telephone number?

TextType. People might enter the number with spaces, with dashes, without delimiter, mixed, etc.

In the model data we want the number without spaces/dashes, in the view we present it with spaces.

Simplified example:

class PhoneNumberDataTransformer
{
    public function transform($value)
    {
        return join(' ', str_split($value, 3));
    }

    public function reverseTransform($value)
    {
         return str_replace([' ', '-', '–'], '', $value);
    }
}

@linaori
Copy link
Contributor

linaori commented Apr 5, 2016

@xabbuh that's correct and the reason I propose that, is because the documentation currently do not have a clear explanation on where to add input normalization into a specific format after submitting (or maybe it got added now). I think this is done with a view transformer.

@dmaicher
Copy link
Contributor

dmaicher commented Apr 5, 2016

I like the tags example that @javiereguiluz mentioned 👍 Much simpler to edit with a simple text field in the view rather than using a collection with JS adding/removing with prototypes etc. but we keep the flexibility of separating the tags in the model with an array/collection.

@xabbuh
Copy link
Member

xabbuh commented Apr 6, 2016

@iltar We may have to make that more clear. But the use case you described should be handled in an event listener.

@webmozart has a nice talk about forms where he explains that quite well:

Data transformers should never change the information stored in a form, but only the data's representation.

To change information, use events.

@wouterj wouterj added good first issue Ideal for your first contribution! (some Symfony experience may be required) actionable Clear and specific issues ready for anyone to take them. and removed needs comments labels May 21, 2016
@wouterj
Copy link
Member

wouterj commented May 21, 2016

Let's implement the implode/explode one suggested by @javiereguiluz

@wouterj wouterj added Hack Day and removed good first issue Ideal for your first contribution! (some Symfony experience may be required) labels May 21, 2016
@mccullagh
Copy link
Contributor

I'll try to work on this one!

mccullagh added a commit to mccullagh/symfony-docs that referenced this issue May 21, 2016
mccullagh added a commit to mccullagh/symfony-docs that referenced this issue May 21, 2016
@wouterj wouterj added the hasPR A Pull Request has already been submitted for this issue. label May 21, 2016
wouterj added a commit that referenced this issue May 21, 2016
…(mccullagh)

This PR was submitted for the master branch but it was merged into the 2.3 branch instead (closes #6581).

Discussion
----------

[#6431] changing "Simple Example" to use implode/explode

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | all
| Fixed tickets | #6431

Commits
-------

4b8a755 [#6431] changing "Simple Example" to use implode/explode
@wouterj wouterj closed this as completed May 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
actionable Clear and specific issues ready for anyone to take them. bug Form Hack Day hasPR A Pull Request has already been submitted for this issue.
Projects
None yet
Development

No branches or pull requests

8 participants