Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Improved sortable behavior: onDelete="setnull" #456

Merged
merged 11 commits into from

5 participants

@rozwell

When scope column is a FK and onDelete="setnull", removing related object now moves all objects to the end of null scope.
This is continuation of #359
New method: "moveRelatedObjectsToNullScope()
Note: it generates only 2 SQL queries.

@rozwell rozwell sortable behavior: when scope column is a FK and onDelete="setnull", …
…removing related object now moves all objects to the end of null scope
924839c
@travisbot

This pull request passes (merged 924839c into 19dc671).

@rozwell

2nd query can be changed to shiftRank($maxRank, 1, null, $this->getPrimaryKey(), $con);
but it will generate slower sql:

UPDATE `article` SET `SORTABLE_RANK` = article.SORTABLE_RANK + 20 WHERE article.SORTABLE_RANK>=1 AND article.CATEGORY_ID=8

and now it's:

UPDATE article SET sortable_rank = sortable_rank+20 WHERE article.CATEGORY_ID = 8

What do you guys think about that?

@travisbot

This pull request fails (merged 1e16f2c into 19dc671).

@travisbot

This pull request passes (merged 4c369ba into 19dc671).

@travisbot

This pull request passes (merged ed54050 into 19dc671).

@willdurand
Owner

Ok, great. Is it covered by tests?

@rozwell

Not yet.

@willdurand
Owner

I fixed the test suite, I guess you have to rebase your PR :)

@rozwell

Merge was enough, tests should be this week finally..

@rozwell

Alrighty then, please check if there isn't anything missing ;)
Sorry I couldn't make it faster..

@willdurand
Owner

Perfect. Thank you so much @rozwell

@willdurand willdurand merged commit 15f4fa1 into propelorm:master
@cedriclombardot

When we use namespaces the {$this->queryClassname} does not build the full namesapce class and the use for this class is not added on the top of the file. So i've a fatal when i try to delete my object

weird because there is:

$builder->declareClassFromBuilder($builder->getStubQueryBuilder());
Collaborator

@cedriclombardot could you give us more information and create an issue ?

could you provide the stacktrace, schema, ... to reproduce your issue ?

Well I ve :

 <table name="tool">
        <column name="id" type="integer" primaryKey="true" required="true" autoIncrement="true" description="id, primary key" />
        <column name="category_id" type="integer" required="true" description="the tool category" />
        <column name="name" type="varchar" size="300" required="false" description="The tool title" />
        <column name="small_description" type="varchar" size="500" required="false" description="The tool small description" />
        <column name="description" type="longvarchar" required="false" description="The tool description" />
        <column name="type" type="enum" required="true" valueSet="active, add, soon, vote" default="vote" description="The tool type" />
        <column name="image" type="varchar" required="false" description="The tool picture thumb" />
        <column name="image_thumb" type="varchar" required="false" description="The tool picture" />
        <column name="functionalities" type="array" required="false" description="The functionnalities" />
        <column name="price" type="float" required="false" description="The tool price" />
        <column name="website_poll" type="varchar" required="false" description="The tool poll website" />
        <column name="note" type="float" required="true" default="3.5" description="The tool note" />

        <foreign-key foreignTable="tool_category" onUpdate="cascade" onDelete="cascade" phpName="Category">
            <reference local="category_id" foreign="id" />
        </foreign-key>

        <behavior name="sortable" />

        <behavior name="sluggable">
            <parameter name="slug_pattern" value="{Name}" />
            <parameter name="permanent" value="true" />
        </behavior>
    </table>

And it generate BaseToolPeer

public static function shiftRank($delta, $first = null, $last = null, PropelPDO $con = null)
{
.....
$whereCriteria = ToolQuery::create();
.....

But not add ToolQuery in use statements

Any news ?

Collaborator

@cedriclombardot no news, sorry, could you create an issue for this ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 25, 2012
  1. @rozwell

    sortable behavior: when scope column is a FK and onDelete="setnull", …

    rozwell authored
    …removing related object now moves all objects to the end of null scope
  2. @rozwell
  3. @rozwell

    sortable behavior: changed moveRelatedObjectsToNullScope() method to …

    rozwell authored
    …use improved shiftRank() method
  4. @rozwell

    sortable behavior: changed moveRelatedObjectsToNullScope() to use alr…

    rozwell authored
    …eady existing getMaxRank() method and fixed bug in shiftRank()
  5. @rozwell
Commits on Sep 5, 2012
  1. @rozwell
Commits on Sep 15, 2012
  1. @rozwell
Commits on Sep 20, 2012
  1. @rozwell
Commits on Nov 9, 2012
  1. @rozwell
  2. @rozwell
  3. @rozwell
This page is out of date. Refresh to see the latest.
View
28 generator/lib/behavior/sortable/SortableBehavior.php
@@ -11,12 +11,14 @@
require_once dirname(__FILE__) . '/SortableBehaviorObjectBuilderModifier.php';
require_once dirname(__FILE__) . '/SortableBehaviorQueryBuilderModifier.php';
require_once dirname(__FILE__) . '/SortableBehaviorPeerBuilderModifier.php';
+require_once dirname(__FILE__) . '/SortableRelationBehavior.php';
/**
* Gives a model class the ability to be ordered
* Uses one additional column storing the rank
*
* @author Massimiliano Arione
+ * @author rozwell
* @version $Revision$
* @package propel.generator.behavior.sortable
*/
@@ -36,19 +38,35 @@ class SortableBehavior extends Behavior
*/
public function modifyTable()
{
- if (!$this->getTable()->containsColumn($this->getParameter('rank_column'))) {
- $this->getTable()->addColumn(array(
+ $table = $this->getTable();
+
+ if (!$table->containsColumn($this->getParameter('rank_column'))) {
+ $table->addColumn(array(
'name' => $this->getParameter('rank_column'),
'type' => 'INTEGER'
));
}
- if ($this->getParameter('use_scope') == 'true' &&
- !$this->getTable()->containsColumn($this->getParameter('scope_column'))) {
- $this->getTable()->addColumn(array(
+ if ($this->useScope() &&
+ !$table->containsColumn($this->getParameter('scope_column'))) {
+ $table->addColumn(array(
'name' => $this->getParameter('scope_column'),
'type' => 'INTEGER'
));
}
+
+ if ($this->useScope()) {
+ $keys = $table->getColumnForeignKeys($this->getParameter('scope_column'));
+ foreach ($keys as $key) {
+ if ($key->isForeignPrimaryKey() && $key->getOnDelete() == ForeignKey::SETNULL) {
+ $foreignTable = $key->getForeignTable();
+ $relationBehavior = new SortableRelationBehavior();
+ $relationBehavior->addParameter(array('name' => 'foreign_table', 'value' => $table->getName()));
+ $relationBehavior->addParameter(array('name' => 'foreign_scope_column', 'value' => $this->getParameter('scope_column')));
+ $relationBehavior->addParameter(array('name' => 'foreign_rank_column', 'value' => $this->getParameter('rank_column')));
+ $foreignTable->addBehavior($relationBehavior);
+ }
+ }
+ }
}
public function getObjectBuilderModifier()
View
1  generator/lib/behavior/sortable/SortableBehaviorObjectBuilderModifier.php
@@ -13,6 +13,7 @@
*
* @author François Zaninotto
* @author heltem <heltem@o2php.com>
+ * @author rozwell
* @package propel.generator.behavior.sortable
*/
class SortableBehaviorObjectBuilderModifier
View
15 generator/lib/behavior/sortable/SortableBehaviorPeerBuilderModifier.php
@@ -13,6 +13,7 @@
*
* @author François Zaninotto
* @author heltem <heltem@o2php.com>
+ * @author rozwell
* @package propel.generator.behavior.sortable
*/
class SortableBehaviorPeerBuilderModifier
@@ -50,6 +51,7 @@ protected function setBuilder($builder)
$this->builder = $builder;
$this->objectClassname = $builder->getStubObjectBuilder()->getClassname();
$this->peerClassname = $builder->getStubPeerBuilder()->getClassname();
+ $this->queryClassname = $builder->getStubQueryBuilder()->getClassname();
}
public function staticAttributes($builder)
@@ -338,18 +340,19 @@ protected function addShiftRank(&$script)
$script .= "
* @param PropelPDO \$con Connection to use.
*/
-public static function shiftRank(\$delta, \$first, \$last = null, " . ($useScope ? "\$scope = null, " : "") . "PropelPDO \$con = null)
+public static function shiftRank(\$delta, \$first = null, \$last = null, " . ($useScope ? "\$scope = null, " : "") . "PropelPDO \$con = null)
{
if (\$con === null) {
\$con = Propel::getConnection($peerClassname::DATABASE_NAME, Propel::CONNECTION_WRITE);
}
- \$whereCriteria = new Criteria($peerClassname::DATABASE_NAME);
- \$criterion = \$whereCriteria->getNewCriterion($peerClassname::RANK_COL, \$first, Criteria::GREATER_EQUAL);
- if (null !== \$last) {
- \$criterion->addAnd(\$whereCriteria->getNewCriterion($peerClassname::RANK_COL, \$last, Criteria::LESS_EQUAL));
+ \$whereCriteria = {$this->queryClassname}::create();
+ if (null !== \$first) {
+ \$whereCriteria->add($peerClassname::RANK_COL, \$first, Criteria::GREATER_EQUAL);
}
- \$whereCriteria->add(\$criterion);";
+ if (null !== \$last) {
+ \$whereCriteria->addAnd($peerClassname::RANK_COL, \$last, Criteria::LESS_EQUAL);
+ }";
if ($useScope) {
$script .= "
\$whereCriteria->add($peerClassname::SCOPE_COL, \$scope, Criteria::EQUAL);";
View
90 generator/lib/behavior/sortable/SortableRelationBehavior.php
@@ -0,0 +1,90 @@
+<?php
+
+/**
+ * This file is part of the Propel package.
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ *
+ * @license MIT License
+ */
+
+/**
+ * Moves objects to the end of null scope of related table
+ *
+ * @author rozwell
+ * @version $Revision$
+ * @package propel.generator.behavior.sortable
+ */
+class SortableRelationBehavior extends Behavior
+{
+ protected $builder;
+ protected $name = 'sortable_relation';
+ // default parameters value
+ protected $parameters = array(
+ 'foreign_table' => '',
+ 'foreign_scope_column' => '',
+ 'foreign_rank_column' => '',
+ );
+
+ public function objectMethods($builder)
+ {
+ $script = '';
+ $this->addObjectMoveRelatedToNullScope($script);
+
+ return $script;
+ }
+
+ public function preDelete($builder)
+ {
+ $this->builder = $builder;
+
+ $script = "\$this->{$this->getObjectMoveRelatedToNullScopeMethodName()}(\$con);
+";
+ return $script;
+ }
+
+ protected function getForeignTable()
+ {
+ return $this->getTable()->getDatabase()->getTable($this->getParameter('foreign_table'));
+ }
+
+ protected function getForeignColumnForParameter($param)
+ {
+ return $this->getForeignTable()->getColumn($this->getParameter($param));
+ }
+
+ protected function getRelatedClassPluralForm()
+ {
+ $relatedClass = $this->getForeignTable()->getPhpName();
+ return $this->builder->getPluralizer()->getPluralForm($relatedClass);
+ }
+
+ protected function getObjectMoveRelatedToNullScopeMethodName()
+ {
+
+ return "moveRelated{$this->getRelatedClassPluralForm()}ToNullScope";
+ }
+
+ protected function addObjectMoveRelatedToNullScope(&$script)
+ {
+ $peerClass = $this->builder->getNewStubPeerBuilder($this->getForeignTable())->getClassname();
+ $queryClass = $this->builder->getNewStubQueryBuilder($this->getForeignTable())->getClassname();
+
+ $script .= "
+/**
+ * Moves related {$this->getRelatedClassPluralForm()} to null scope
+ * @param PropelPDO \$con A connection object
+ */
+public function {$this->getObjectMoveRelatedToNullScopeMethodName()}(PropelPDO \$con = null)
+{
+ \$maxRank = $queryClass::create()->getMaxRank(\$this->getPrimaryKey(), \$con);
+ if (null !== \$maxRank) { // getMaxRank() returns null for empty tables
+ $peerClass::shiftRank(\$maxRank, null, null, \$this->getPrimaryKey(), \$con);
+ }
+}
+";
+
+ return $script;
+ }
+
+}
View
15 test/fixtures/bookstore/behavior-sortable-schema.xml
@@ -18,4 +18,19 @@
</behavior>
</table>
+ <table name="fk_scope_table">
+ <column name="id" required="true" primaryKey="true" autoIncrement="true" type="INTEGER" />
+ <column name="title" type="VARCHAR" size="100" primaryString="true" />
+ <column name="scope_id" type="INTEGER" />
+ <foreign-key foreignTable="table11" onDelete="setnull">
+ <reference local="scope_id" foreign="id" />
+ </foreign-key>
+ <column name="position" type="INTEGER" />
+ <behavior name="sortable">
+ <parameter name="rank_column" value="position" />
+ <parameter name="use_scope" value="true" />
+ <parameter name="scope_column" value="scope_id" />
+ </behavior>
+ </table>
+
</database>
View
17 ...tsuite/generator/behavior/sortable/SortableBehaviorObjectBuilderModifierWithScopeTest.php
@@ -15,6 +15,7 @@
* Tests for SortableBehavior class
*
* @author Massimiliano Arione
+ * @author rozwell
* @version $Revision$
* @package generator.behavior.sortable
*/
@@ -55,6 +56,22 @@ public function testPreDelete()
$this->assertEquals(3, $t4->getRank(), 'Sortable rearrange subsequent rows on delete');
$expected = array(1 => 'row5', 2 => 'row6');
$this->assertEquals($expected, $this->getFixturesArrayWithScope(2), 'delete() leaves other suites unchanged');
+ $expected = array(1 => 'row7', 2 => 'row8', 3 => 'row9', 4 => 'row10');
+ $this->assertEquals($expected, $this->getFixturesArrayWithScope(), 'delete() leaves other suites unchanged');
+ }
+
+ public function testPreDeleteFkScope()
+ {
+ $this->populateFkScopeTable();
+
+ $t = Table11Peer::retrieveByRank(2);
+ $t->delete();
+ $expected = array(1 => 'row7', 2 => 'row8', 3 => 'row9', 4 => 'row4', 5 => 'row5', 6 => 'row6');
+ $this->assertEquals($expected, $this->getFixturesArrayWithFkScope(), 'delete() moves related objects to the end of null scope');
+
+ $s = Table11Peer::retrieveByRank(1);
+ $expected = array(1 => 'row1', 2 => 'row2', 3 => 'row3');
+ $this->assertEquals($expected, $this->getFixturesArrayWithFkScope($s->getId()), 'delete() leaves other suites unchanged');
}
public function testIsFirst()
View
1  ...estsuite/generator/behavior/sortable/SortableBehaviorPeerBuilderModifierWithScopeTest.php
@@ -15,6 +15,7 @@
* Tests for SortableBehavior class
*
* @author Massimiliano Arione
+ * @author rozwell
* @version $Revision$
* @package generator.behavior.sortable
*/
View
72 test/tools/helpers/bookstore/behavior/BookstoreSortableTestBase.php
@@ -91,6 +91,64 @@ protected function populateTable12()
$t10->save();
}
+ protected function populateFkScopeTable()
+ {
+ /* List used for tests
+ scope=1 scope=2 scope=null
+ row1 row4 row7
+ row2 row5 row8
+ row3 row6 row9
+ */
+ $this->populateTable11();
+
+ $s1 = Table11Peer::retrieveByRank(1)->getId();
+ $s2 = Table11Peer::retrieveByRank(2)->getId();
+
+ FkScopeTablePeer::doDeleteAll();
+ $t1 = new FkScopeTable();
+ $t1->setRank(1);
+ $t1->setScopeValue($s1);
+ $t1->setTitle('row1');
+ $t1->save();
+ $t2 = new FkScopeTable();
+ $t2->setRank(2);
+ $t2->setScopeValue($s1);
+ $t2->setTitle('row2');
+ $t2->save();
+ $t3 = new FkScopeTable();
+ $t3->setRank(3);
+ $t3->setScopeValue($s1);
+ $t3->setTitle('row3');
+ $t3->save();
+ $t4 = new FkScopeTable();
+ $t4->setRank(1);
+ $t4->setScopeValue($s2);
+ $t4->setTitle('row4');
+ $t4->save();
+ $t5 = new FkScopeTable();
+ $t5->setRank(2);
+ $t5->setScopeValue($s2);
+ $t5->setTitle('row5');
+ $t5->save();
+ $t6 = new FkScopeTable();
+ $t6->setRank(3);
+ $t6->setScopeValue($s2);
+ $t6->setTitle('row6');
+ $t6->save();
+ $t7 = new FkScopeTable();
+ $t7->setRank(1);
+ $t7->setTitle('row7');
+ $t7->save();
+ $t8 = new FkScopeTable();
+ $t8->setRank(2);
+ $t8->setTitle('row8');
+ $t8->save();
+ $t9 = new FkScopeTable();
+ $t9->setRank(3);
+ $t9->setTitle('row9');
+ $t9->save();
+ }
+
protected function getFixturesArray()
{
$c = new Criteria();
@@ -117,4 +175,18 @@ protected function getFixturesArrayWithScope($scope = null)
return $ret;
}
+
+ protected function getFixturesArrayWithFkScope($scope = null)
+ {
+ $c = new Criteria();
+ $c->add(FkScopeTablePeer::SCOPE_COL, $scope);
+ $c->addAscendingOrderByColumn(FkScopeTablePeer::RANK_COL);
+ $ts = FkScopeTablePeer::doSelect($c);
+ $ret = array();
+ foreach ($ts as $t) {
+ $ret[$t->getRank()] = $t->getTitle();
+ }
+
+ return $ret;
+ }
}
Something went wrong with that request. Please try again.