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

Extract slug generator to separate method #94

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions src/Knp/DoctrineBehaviors/Model/Sluggable/Sluggable.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,29 @@ public function generateSlug()
}

// generate the slug itself
$sluggableText = implode($usableValues, ' ');
$urlized = strtolower( trim( preg_replace("/[^a-zA-Z0-9\/_|+ -]/", '', iconv('UTF-8', 'ASCII//TRANSLIT', $sluggableText) ), $this->getSlugDelimiter() ) );
$urlized = preg_replace("/[\/_|+ -]+/", $this->getSlugDelimiter(), $urlized);
$transliterator = $this->getTransliterator();

$this->slug = $urlized;
if (!is_callable($transliterator)) {
throw new \RuntimeException("Sluggable::getTransliterator() method must return callable.");
}

$this->slug = call_user_func($transliterator, implode($usableValues, ' '));
}
}

/**
* This method should return any php callable which can create slug from given string.
* Feel free to override this any way you want.
*
* @return callable
*/
protected function getTransliterator()
{
return function($string) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This look kinda weird. Inception function in a method.

If you need to pass parameter, try call_user_func_array

Also it would be nice to keep inner compatibility of this package. Checkout this localeCallable in Translatable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that you don't need to return a closure to call it directly after.

Just name the method "transliterate" (or someting) and return this result.
Anyone will be able to replace it in its class anyway. See what I mean?

Otherwise, (and it was the case before already with iconv), but we need to put ext-intl (or ext-iconv) in the list of requires (or suggests) in composer.json IMHO.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I see :)

$string = transliterator_transliterate("Any-Latin; NFD; [:Nonspacing Mark:] Remove; NFC; [:Punctuation:] Remove; Lower();", $string);
$string = preg_replace('/[-\s]+/', '-', $string);

return trim($string, '-');
};
}
}
32 changes: 32 additions & 0 deletions src/Knp/DoctrineBehaviors/Model/Sluggable/Utils.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

namespace Knp\DoctrineBehaviors\Model\Sluggable;

/**
* This class contains function which transliterates string in russian using traditional russian way of transliterating instead of ISO transliteration rules.
*
* @package Knplabs\DoctrineBehaviors\Model\Sluggable
* @author Alex Panshin <deadyaga@gmail.com>
*/
class Utils
{
private static $ru_alphabet = array( 'а','б','в','г','д','е','ё','ж','з','и','й','к','л','м','н','о','п','р','с','т','у','ф','х','ц','ч','ш','щ','ъ','ы','ь','э','ю','я');
private static $translit_alphabet = array('a','b','v','g','d','e','yo','zh', 'z', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'r', 's', 't', 'u','f', 'h', 'ts', 'ch', 'sh', 'sch', '', 'y', 'j', 'e', 'yu', 'ya');

/**
* This method prepares string to be an url's part
* @param string $string
* @return string
*/
public static function transliterateRussian($string)
{
$string = function_exists('mb_strtolower') ? mb_strtolower($string, 'UTF-8') : strtolower($string);

$string = str_replace(self::$ru_alphabet, self::$translit_alphabet, $string);

$string = transliterator_transliterate("Any-Latin; NFD; [:Nonspacing Mark:] Remove; NFC; [:Punctuation:] Remove;", $string);
$string = preg_replace('/[-\s]+/', '-', $string);

return trim($string, '-');
}
}
36 changes: 36 additions & 0 deletions tests/Knp/DoctrineBehaviors/ORM/SluggableTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,40 @@ public function testUpdatedSlug()

$this->assertEquals($entity->getSlug(), $expected);
}

/**
* @test
*/
public function shouldTransliterateSlug()
{
$em = $this->getEntityManager();
$entity = new \BehaviorFixtures\ORM\SluggableEntity();

$expected = 'privet';

$entity->setName('Привет!');

$em->persist($entity);
$em->flush();

$this->assertEquals($expected, $entity->getSlug());
}

/**
* @test
*/
public function shouldTransliterateSlugInRussian()
{
$em = $this->getEntityManager();
$entity = new \BehaviorFixtures\ORM\RussianSluggableEntity();

$expected = 'ya-budu-zhdatj-tebya';

$entity->setName('Я буду ждать тебя!');

$em->persist($entity);
$em->flush();

$this->assertEquals($expected, $entity->getSlug());
}
}
18 changes: 18 additions & 0 deletions tests/fixtures/BehaviorFixtures/ORM/RussianSluggableEntity.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

namespace BehaviorFixtures\ORM;

use Doctrine\ORM\Mapping as ORM;
use Knp\DoctrineBehaviors\Model;

/**
* @ORM\Entity
* @ORM\Table(name="SluggableEntity")
*/
class RussianSluggableEntity extends SluggableEntity
{
protected function getTransliterator()
{
return ['\Knp\DoctrineBehaviors\Model\Sluggable\Utils', 'transliterateRussian'];
}
}
2 changes: 1 addition & 1 deletion tests/fixtures/BehaviorFixtures/ORM/SluggableEntity.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public function setDate($date)
return $this;
}

protected function getSluggableFields()
public function getSluggableFields()
{
return [ 'name' ];
}
Expand Down