[DC-728] Updating slug in I18N behaviour yields non-unique slug Created: 11/Jun/10  Updated: 05/Oct/10

Status: Open
Project: Doctrine 1
Component/s: I18n, Sluggable
Affects Version/s: 1.2.0, 1.2.1, 1.2.2
Fix Version/s: None

Type: Bug Priority: Major
Reporter: Florian Aschenbrenner Assignee: Jonathan H. Wage
Resolution: Unresolved Votes: 5
Labels: None
Environment:

Symfony 1.4.5
Doctrine ORM



 Description   

There seems to be a bug in the way, doctrine tries to find a unique slug for a table that is I18N-enabled and has a slug in it.

Following example:

Problem:
  actAs:
    I18n:
      fields: [name]
      actAs:
        Sluggable: { fields: [name], uniqueBy: [lang, name], canUpdate: true }
  columns:
    name: { type: string(255), notnull: true }

Now, if we insert a new entry, the slug get's created as expected. If we now insert another entry that would yield the same slug, the slug is made unique by adding the postfix as expected - all well so far.

Now, if we try to update the entry with the slug with the postfix, what i think is an error happens (Excerpt from Sluggable.php, lib/Doctrine/Template/Listener/Sluggable.php):

 
public function getUniqueSlug($record, $slugFromFields)
...
if ($record->exists()) {
            $identifier = $record->identifier();
            $whereString .= ' AND r.' . implode(' != ? AND r.', $table->getIdentifierColumnNames()) . ' != ?';
            $whereParams = array_merge($whereParams, array_values($identifier));
        }

        foreach ($this->_options['uniqueBy'] as $uniqueBy) {
            if (is_null($record->$uniqueBy)) {
                $whereString .= ' AND r.'.$uniqueBy.' IS NULL';
            } else {
                $whereString .= ' AND r.'.$uniqueBy.' = ?';
                $value = $record->$uniqueBy;
                if ($value instanceof Doctrine_Record) {
                    $value = current((array) $value->identifier());
                }
                $whereParams[] = $value;
            }
        }
...

So, the $record->exists() check evaluates to true and $table->getIdentifierColumnNames() yields the fields id and lang, so the whereString is something like

 
AND r.id != ? AND r.lang != ?

Now for the problematic part:
uniqueBy is lang and name; so the code adds to the whereString:

 
AND r.lang = ? AND r.name = ?

So the resulting whereString has something like

 
r.lang != ? AND r.lang = ?

which will never yield a result in my eyes, thus, the postfix is never incremented and the slug defaults to the the slug of name without postfix.



 Comments   
Comment by Rodrigo Borrego Bernabé [ 24/Jun/10 ]

Hi,

We've also found this problem and make a temporal workaround. But maybe our workaround can give someone an idea on how to fix the bug so I'm pasting the changed code here

In file sfDoctrinePlugin\lib\vendor\doctrine\Doctrine\Template\Listener\Sluggable.php (getUniqueSlug method)

Replace

if ($record->exists()) {
  $identifier = $record->identifier();
  $whereString .= ' AND r.' . implode(' != ? AND r.', $table->getIdentifierColumnNames()) . ' != ?';
  $whereParams = array_merge($whereParams, array_values($identifier));
}

With

if ($record->exists()) {
  $identifier = $record->identifier();
  $commonFields = array_uintersect($table->getIdentifierColumnNames(), $this->_options['uniqueBy'], "strcasecmp");
  foreach ($commonFields as $key => $value)
    unset ($identifier[$value]);
  
  $whereFields = array_diff($table->getIdentifierColumnNames(), $this->_options['uniqueBy']);
  $whereString .= ' AND r.' . implode(' != ? AND r.', $whereFields) . ' != ?';
  $whereParams = array_merge($whereParams, array_values($identifier));
}
Comment by Christian Seaman [ 05/Oct/10 ]

Rodrigo, I think the solution is simpler than you might expect...

The problem is that it is doing an AND where it should be doing an OR to find records other than the current one.

So, you just need to replace this part of Sluggable::getUniqueSlug():

Sluggable::getUniqueSlug()
...
        if ($record->exists()) {
            $identifier = $record->identifier();
            $whereString .= ' AND r.' . implode(' != ? AND r.', $table->getIdentifierColumnNames()) . ' != ?';
            $whereParams = array_merge($whereParams, array_values($identifier));
        }
...

With this code (the 3rd line has extra brackets and OR instead of AND):

Sluggable::getUniqueSlug() (modified)
        if ($record->exists()) {
            $identifier = $record->identifier();
            $whereString .= ' AND (r.' . implode(' != ? OR r.', $table->getIdentifierColumnNames()) . ' != ?)';
            $whereParams = array_merge($whereParams, array_values($identifier));
        }

In more detail, the WHERE clause being built up is saying:
"find me records WHERE (the slug begins with the same string) AND (it's not the same record) AND (the other key uniqueBy values are the same)"

However, to check that it's not the same record you just need one of the key values to be different (not all of them) so the check should be something like:

... AND (p.id != '2' OR p.lang != 'en') ...

instead of:

... AND p.id != '2' AND p.lang != 'en' ...

The modification to the code as stated above generates queries that look like this, which I'm pretty sure is correct:

... WHERE (p.url LIKE 'foo%' AND (p.id != '2' OR p.lang != 'en') AND p.lang = 'en' AND p.other_uniqueby_field = '1')

C





[DC-640] Slugs aren't created when inserting/updating a table row Created: 21/Apr/10  Updated: 03/May/10

Status: Open
Project: Doctrine 1
Component/s: Sluggable
Affects Version/s: 1.2.2
Fix Version/s: None

Type: Bug Priority: Major
Reporter: Daniel König Assignee: Jonathan H. Wage
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Using Symfony 1.4 form svn:

URL: http://svn.symfony-project.com/branches/1.4
Revision: 29223
Last Changed Rev: 29218
Last Changed Date: 2010-04-20 08:04:09 +0200 (Di, 20 Apr 2010)



 Description   

Basically the slug column doesn't get updated in spite of all fields being added correctly, as far as i can tell.
It seems to be related to additional options because when i don't have either of the uniqueBy or canUpdate options specified, it just works.

What's been missing is the 'slug' column value in the sql statements (providing 3 examples):

2400 Query INSERT INTO `custom_category` (`block_id`, `name`) VALUES ('1', 'slug test')
2526 Query INSERT INTO `custom_category` (`name`, `block_id`) VALUES ('please do work..', '1')
2560 Query UPDATE `custom_category` SET `name` = 'still not ok' WHERE `id` = '12'

The generated schema.sql includes the correct create table statement (it's also correctly inserted in the db):

CREATE TABLE `custom_category` (
  `id` INT UNSIGNED AUTO_INCREMENT,
  `name` VARCHAR(255) NOT NULL,
  `block_id` INT UNSIGNED NOT NULL,
  `slug` VARCHAR(255),
  UNIQUE INDEX `custom_category_sluggable_idx` (`slug`, `block_id`, `name`),
  INDEX `block_id_idx` (`block_id`),
  PRIMARY KEY(`id`)
) ENGINE = INNODB;

The schema.yml entry is the following:

CustomCategory:
  connection: removed
  tableName: custom_category
  actAs:
    Sluggable:
      fields: [name]
      uniqueBy: [block_id, name]
      canUpdate: true
  columns:
    id:
      type: integer(4)
      unsigned: true
      notnull: true
      primary: true
      autoincrement: true
    name:
      type: string(255)
      notnull: true
    block_id:
      type: integer(4)
      unsigned: true
      notnull: true
  relations:
    Products:
      class: Product
      foreignAlias: CustomCategories
      refClass: ProductCustomCategory
    Block:
      class: Block
      local: block_id
      foreign: id
      foreignType: many
      foreignAlias: CustomCategories
      type: one

And finally the appropriate auto generated base class (removed doc block):

<?php
// Connection Component Binding
Doctrine_Manager::getInstance()->bindComponent('CustomCategory', 'removed');

abstract class BaseCustomCategory extends aaDoctrineRecord
{
    public function setTableDefinition()
    {
        $this->setTableName('custom_category');
        $this->hasColumn('id', 'integer', 4, array(
             'type' => 'integer',
             'unsigned' => true,
             'primary' => true,
             'autoincrement' => true,
             'length' => '4',
             ));
        $this->hasColumn('name', 'string', 255, array(
             'type' => 'string',
             'notnull' => true,
             'length' => '255',
             ));
        $this->hasColumn('block_id', 'integer', 4, array(
             'type' => 'integer',
             'unsigned' => true,
             'notnull' => true,
             'length' => '4',
             ));
    }

    public function setUp()
    {
        parent::setUp();
        $this->hasMany('Product as Products', array(
             'refClass' => 'ProductCustomCategory',
             'local' => 'custom_category_id',
             'foreign' => 'product_id'));

        $this->hasOne('Block', array(
             'local' => 'block_id',
             'foreign' => 'id'));

        $sluggable0 = new Doctrine_Template_Sluggable(array(
             'fields' =>
             array(
              0 => 'name',
             ),
             'uniqueBy' =>
             array(
              0 => 'block_id',
              1 => 'name',
             ),
             'canUpdate' => true,
             ));
        $this->actAs($sluggable0);
    }
}


 Comments   
Comment by Christian Seaman [ 01/May/10 ]

What is the code that you're using to generate the INSERT query?

Comment by Daniel König [ 03/May/10 ]

just instantiating the object, calling the setters, followed by a save().

it's related to the uniqueBy and canUpdate options, though - it works like a charm without these two.





[DC-623] Impossible to create non-unique slugs Created: 09/Apr/10  Updated: 31/Aug/10

Status: Open
Project: Doctrine 1
Component/s: Sluggable
Affects Version/s: 1.2.2
Fix Version/s: None

Type: Bug Priority: Major
Reporter: Adwise Internetmarketing Assignee: Jonathan H. Wage
Resolution: Unresolved Votes: 2
Labels: None

Attachments: Text File dc-623.patch    

 Description   

I want to create a non-unique slug of a title field from my table.

But I've found that when I change unique from 'true' to 'false' the slugs were not created or updated

In lib\Doctrine\Template\Listener\Sluggable.php inside preUpdate() function the first if() statement disables the unique (false) option.



 Comments   
Comment by Jacob Mather [ 30/Aug/10 ]

Theoretically this should clear up the issue. Sorry for not having the time for a complete test at the moment,

Comment by webPragmatist [ 30/Aug/10 ]

The patch by Jacob doesn't work. The reason for the unique check is to bypass appending a numeric value to the end of a slug in the even two exist.

It just needs to somehow be directly sent to the urlize function.

Comment by Jacob Mather [ 31/Aug/10 ]

I would just like to post an update after I've been able to test:

The only functionality that i found actually broken was updating a slug with unique set to false.

The reason was the wrapped if.

If you set unique to false, slugs will still fail to update as canUpdate must also be set to true.

The patch, so far as I can determine, does correct the underlying issue if a slug failing to update when unique is false and canUpdate is true.





Generated at Tue Sep 16 11:39:14 UTC 2014 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.