fixes #523: NestedSet + useScope + autoIncrement #524

Open
wants to merge 1 commit into
from

Projects

None yet

3 participants

@marc-mabe

Check if the scope value is NULL on auto incremented scopes before check if root already exists

Marc Bennewitz fixes #523: NestedSet + useScope + autoIncrement
Check if the scope value is NULL on auto increment scope before check if root already exists
9adf274
@willdurand
Member

Thanks for this patch, could you please add a unit test to prove the bug?

@marc-mabe

@willdurand Sorry, I can't simply write a testcase because it's not trivial:

  • Don't know how to start the testsuite (failes on so many cases)
  • Don't know how the testsuite is organized
  • Don't know where to configure database connection (I require to set a different host, db, pwd for mysql)
  • Don't know how to run with pdo_mysql only installed
  • How to run only one test instead of the hole suite
  • Can't find a nestedset behavior on any of the schemas of the testsuite
  • ...

So I'm @ work an there are many limitations on the environment. It would be very great if you could help writing a test for this issue.

@willdurand
Member

ok, could you just provide some code that proves the bug? For example, your schema and what you tried to achieve in your app?

@willdurand
Member

ping @marc-mabe

@marc-mabe

@willdurand Sorry for the late response. As described in #523 I have a tree of keywords with different root levels - use_scope and the scope_column is marked autoIncrement.

Now I would like to import a tree with the following function:

function importKeywords(array $keywords, KeywordTree $parent = null) {
    foreach ($keywords as $keyword => $childs) {
        $obj = new KeywordTree();
        $obj->setKeyword($keyword);
        if ($parent) {
            $obj->insertAsLastChildOf($parent);
        } else {
            $obj->makeRoot();
        }
        $obj->save();
        importKeywords($childs, $obj);
    }
}

But it doesn't work before the PR and shows A root node already exists in this tree with scope ""

@marcj
Member
marcj commented Feb 24, 2013

hey @marc-mabe, we've improved our documentation that explains how to write tests and how to use and setup the phpunit environment.
Please take a look here: http://propelorm.org/cookbook/working-with-unit-tests.html

If you're willing to add a unit test, then I'd like to say we wait for your feedback or we close this PR and I'll create a new PR with your commit merged and additionally a unit test. What do you think?

@marc-mabe

@marcj It would be better if you create a new PR for that - if haven't time to work on it right now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment