Skip to content

Commit

Permalink
Merge pull request #7629 from laboro/bug/CRM-5529
Browse files Browse the repository at this point in the history
CRM-5529: Recipient names which contain a comma may be perceived as t…
  • Loading branch information
alex-n-2k7 authored Jun 21, 2016
2 parents e869af1 + 22b1010 commit 163d293
Show file tree
Hide file tree
Showing 11 changed files with 193 additions and 45 deletions.
4 changes: 2 additions & 2 deletions src/Oro/Bundle/EmailBundle/Controller/EmailController.php
Original file line number Diff line number Diff line change
Expand Up @@ -577,15 +577,15 @@ public function autocompleteRecipientAction(Request $request)
{
$query = $request->query->get('query');
if ($request->query->get('search_by_id', false)) {
$emails = explode(',', $query);
$emails = EmailRecipientsHelper::extractFormRecipientIds($query);
$results = array_map(function ($email) {
$recipient = $this->getEmailRecipientsHelper()->createRecipientFromEmail($email);
if ($recipient) {
return $this->getEmailRecipientsHelper()->createRecipientData($recipient);
}

return [
'id' => $email,
'id' => EmailRecipientsHelper::prepareFormRecipientIds($email),
'text' => $email,
];
}, $emails);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php

namespace Oro\Bundle\EmailBundle\Form\DataTransformer;

use Symfony\Component\Form\DataTransformerInterface;
use Oro\Bundle\EmailBundle\Provider\EmailRecipientsHelper;

/**
* Transforms form value between array of full email addresses and string of base64 encoded full email addresses.
*/
class EmailAddressRecipientsTransformer implements DataTransformerInterface
{
/**
* {@inheritdoc}
*/
public function transform($value)
{
if (!is_array($value)) {
return $value;
}

return EmailRecipientsHelper::prepareFormRecipientIds($value);
}

/**
* {@inheritdoc}
*/
public function reverseTransform($value)
{
if (is_array($value)) {
return $value;
}

return EmailRecipientsHelper::extractFormRecipientIds($value);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@
use Symfony\Component\Form\FormInterface;
use Symfony\Component\Form\FormView;
use Symfony\Component\OptionsResolver\OptionsResolverInterface;
use Symfony\Component\Form\FormBuilderInterface;

use Oro\Bundle\EmailBundle\Form\Model\Email;
use Oro\Bundle\ConfigBundle\Config\ConfigManager;
use Oro\Bundle\EmailBundle\Form\DataTransformer\EmailAddressRecipientsTransformer;
use Oro\Bundle\EmailBundle\Provider\EmailRecipientsHelper;

class EmailAddressRecipientsType extends AbstractType
{
Expand All @@ -25,6 +28,17 @@ public function __construct(ConfigManager $cm)
$this->cm = $cm;
}

/**
* {@inheritdoc}
*/
public function buildForm(FormBuilderInterface $builder, array $options)
{
$builder->resetViewTransformers();
$builder->addViewTransformer(
new EmailAddressRecipientsTransformer()
);
}

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -61,6 +75,7 @@ public function setDefaultOptions(OptionsResolverInterface $resolver)
'configs' => [
'allowClear' => true,
'multiple' => true,
'separator' => EmailRecipientsHelper::EMAIL_IDS_SEPARATOR,
'route_name' => 'oro_email_autocomplete_recipient',
'minimumInputLength' => $this->cm->get('oro_email.minimum_input_length'),
'per_page' => 100,
Expand Down
49 changes: 45 additions & 4 deletions src/Oro/Bundle/EmailBundle/Provider/EmailRecipientsHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
class EmailRecipientsHelper
{
const ORGANIZATION_PROPERTY = 'organization';
const EMAIL_IDS_SEPARATOR = ';';

/** @var AclHelper */
protected $aclHelper;
Expand Down Expand Up @@ -167,9 +168,9 @@ public function createRecipientData(Recipient $recipient)
}

return [
'id' => $recipient->getId(),
'text' => $recipient->getName(),
'data' => json_encode($data),
'id' => self::prepareFormRecipientIds($recipient->getId()),
'text' => $recipient->getName(),
'data' => json_encode($data),
];
}

Expand Down Expand Up @@ -325,7 +326,7 @@ protected function recipientsFromResult(array $result, $entityClass)
foreach ($result as $row) {
$recipient = new CategorizedRecipient(
$row['email'],
sprintf('%s <%s>', $row['name'], $row['email']),
sprintf('"%s" <%s>', $row['name'], $row['email']),
new RecipientEntity(
$entityClass,
$row['entityId'],
Expand All @@ -351,4 +352,44 @@ protected function getPropertyAccessor()

return $this->propertyAccessor;
}

/**
* Prepares base64 encoded emails to be used as ids in recipients form for select2 component.
*
* @param array|string $ids
* @return string;
*/
public static function prepareFormRecipientIds($ids)
{
if (is_string($ids)) {
return base64_encode($ids);
}

$ids = array_map("base64_encode", $ids);

return implode(self::EMAIL_IDS_SEPARATOR, $ids);
}

/**
* Extracts base64 encoded selected email values, that are used as ids in recipients form for select2 component.
*
* @param array|string $value
* @return array;
*/
public static function extractFormRecipientIds($value)
{
if (is_array($value)) {
return $value;
}
/*
* str_getcsv is used to cover the case if emails are pasted directly with ";" separator
* and it protects from ";" used in full email address. (example: "Recipient Name; Name2" <[email protected]>)
*/
$idsEncoded = str_getcsv($value, self::EMAIL_IDS_SEPARATOR);
$idsDecoded = array_map(function ($idEncoded) {
return base64_decode($idEncoded, true) ? : $idEncoded;
}, $idsEncoded);

return $idsDecoded;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,9 @@ public function testReply()
$this->assertEquals(1, $crawler->filter('div.widget-content input[name=\'oro_email_email[cc]\']')->count());
$this->assertEquals(
1,
$crawler->filter('div.widget-content input[value=\'' . $email->getFromName() . '\']')->count()
$crawler->filter('div.widget-content input[value=\''
. base64_encode($email->getFromName())
. '\']')->count()
);
$cc = $email->getCc()->first()->getEmailAddress()->getEmail();
$this->assertEquals(
Expand All @@ -258,12 +260,14 @@ public function testReplyAll()
$this->assertEquals(1, $crawler->filter('div.widget-content input[name=\'oro_email_email[cc]\']')->count());
$this->assertEquals(
1,
$crawler->filter('div.widget-content input[value=\'' . $email->getFromName() . '\']')->count()
$crawler->filter('div.widget-content input[value=\''
. base64_encode($email->getFromName())
. '\']')->count()
);
$cc = $email->getCc()->first()->getEmailAddress()->getEmail();
$this->assertEquals(
1,
$crawler->filter('div.widget-content input[value=\'' . $cc . '\']')->count()
$crawler->filter('div.widget-content input[value=\'' . base64_encode($cc) . '\']')->count()
);
$bcc = $email->getBcc()->first()->getEmailAddress()->getEmail();
$this->assertEquals(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ protected function setUp()

public function testPreciseFullEmailAddressIsFullQualifiedName()
{
$emailAddress = 'Admin <[email protected]>';
$emailAddress = '"Admin" <[email protected]>';

$this->entityRoutingHelper->expects($this->never())
->method('getEntity');
Expand All @@ -131,7 +131,7 @@ public function testPreciseFullEmailAddressIsFullQualifiedName()
public function testPreciseFullEmailAddressViaRoutingHelper()
{
$emailAddress = '[email protected]';
$expected = 'Admin <[email protected]>';
$expected = '"Admin" <[email protected]>';

$ownerClass = 'Oro\Bundle\UserBundle\Entity\User';
$ownerId = 1;
Expand Down Expand Up @@ -183,7 +183,7 @@ public function testPreciseFullEmailAddressViaRoutingHelperWithExcludeCurrentUse
public function testPreciseFullEmailAddressViaAddressManager()
{
$emailAddress = '[email protected]';
$expected = 'Admin <[email protected]>';
$expected = '"Admin" <[email protected]>';

$ownerClass = 'Oro\Bundle\UserBundle\Entity\User';
$ownerId = null;
Expand Down Expand Up @@ -307,19 +307,19 @@ public function preciseFullEmailAddressProvider()
{
return [
[
'FirstName LastName <[email protected]>',
'"FirstName LastName" <[email protected]>',
'[email protected]',
null,
null
],
[
'FirstName LastName <[email protected]>',
'"FirstName LastName" <[email protected]>',
'[email protected]',
'Oro\Bundle\EmailBundle\Tests\Unit\Fixtures\Entity\TestUser',
null
],
[
'OwnerFirstName OwnerLastName <[email protected]>',
'"OwnerFirstName OwnerLastName" <[email protected]>',
'[email protected]',
'Oro\Bundle\EmailBundle\Tests\Unit\Fixtures\Entity\TestUser',
123
Expand Down Expand Up @@ -410,7 +410,7 @@ public function testBuildFullEmailAddress()
$user = $this->getMock('Oro\Bundle\UserBundle\Entity\User');
$email = 'email';
$format = 'format';
$expected = 'format <email>';
$expected = '"format" <email>';

$user->expects($this->once())
->method('getEmail')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,4 +192,56 @@ public function filterRecipientsDataProvider()
],
];
}

/**
* @dataProvider prepareFormRecipientIdsDataProvider
*/
public function testPrepareFormRecipientIds($ids, $expectedResult)
{
$this->assertEquals($expectedResult, EmailRecipientsHelper::prepareFormRecipientIds($ids));
}

public function prepareFormRecipientIdsDataProvider()
{
return [
[
[
'"Recipient1 Name; Name2" <[email protected]>',
'"Recipient2 Name, Name2" <[email protected]>'
],

base64_encode('"Recipient1 Name; Name2" <[email protected]>') . ';'
. base64_encode('"Recipient2 Name, Name2" <[email protected]>')
]
];
}

/**
* @dataProvider extractFormRecipientIdsDataProvider
*/
public function testExtractFormRecipientIds($value, $expectedResult)
{
$this->assertEquals($expectedResult, EmailRecipientsHelper::extractFormRecipientIds($value));
}

public function extractFormRecipientIdsDataProvider()
{
return [
[
base64_encode('"Recipient1 Name; Name2" <[email protected]>') . ';'
. base64_encode('"Recipient2 Name, Name2" <[email protected]>'),
[
'"Recipient1 Name; Name2" <[email protected]>',
'"Recipient2 Name, Name2" <[email protected]>'
]
],
[
'[email protected];[email protected]',
[
'[email protected]',
'[email protected]'
]
]
];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ public static function buildFullEmailAddressProvider()
['[email protected]', null, '[email protected]'],
['[email protected]', '', '[email protected]'],
['[email protected]', null, '[email protected]'],
['[email protected]', 'John Smith', 'John Smith <[email protected]>'],
[' [email protected] ', ' John Smith ', 'John Smith <[email protected]>'],
['[email protected]', 'John Smith', '"John Smith" <[email protected]>'],
[' [email protected] ', ' John Smith ', '"John Smith" <[email protected]>'],
];
}

Expand Down Expand Up @@ -126,7 +126,7 @@ public static function extractEmailAddressFirstNameProvider()
{
return [
['John Smith IV. <[email protected]>', 'John'],
['John Smith <[email protected]>', 'John'],
['"John Smith" <[email protected]>', 'John'],
['John <[email protected]>', 'John'],
['[email protected]', 'john'],
['[email protected]', 'john'],
Expand All @@ -137,7 +137,7 @@ public static function extractEmailAddressLastNameProvider()
{
return [
['John Smith IV. <[email protected]>', 'Smith IV.'],
['John Smith <[email protected]>', 'Smith'],
['"John Smith" <[email protected]>', 'Smith'],
['John <[email protected]>', 'example.com'],
['[email protected]', 'smith'],
['[email protected]', 'example.com'],
Expand Down
Loading

0 comments on commit 163d293

Please sign in to comment.