Skip to content

make use of copyInto in addVersion #649

Open
wants to merge 1 commit into from

2 participants

@havvg
Propel member
havvg commented Apr 8, 2013

Do not merge right now!

@staabm
Propel member
staabm commented Apr 8, 2013

Do we have a testcase covering the change?

@havvg
Propel member
havvg commented Apr 8, 2013

I just double-checked and added a note to not merge this yet.

The only difference I see would be a non-PK autoincrement column, which would not be copied by copyInto but the previous code. However I lack any idea what reasonable use-case there is for such a column :(

@staabm
Propel member
staabm commented Apr 8, 2013
@staabm
Propel member
staabm commented Apr 8, 2013

anyhow... I don't know why, when a composite pkey is present, all auto inc columns are gathered to fill them later with a database-default [1]. Couldn't there be a table with a composite pk but without auto-inc?
Also the assumption the other way arround is wired... a simple primary-key is also filled with a database default even if it is not an auto-inc...

[1] https://github.com/propelorm/Propel/blob/master/generator/lib/builder/om/PHP5ObjectBuilder.php#L5364-L5374

@staabm staabm commented on the diff Apr 8, 2013
...sionable/VersionableBehaviorObjectBuilderModifier.php
@@ -288,11 +288,10 @@ public function addVersion(\$con = null)
{
\$this->enforceVersion = false;
- \$version = new {$versionARClassname}();";
- foreach ($this->table->getColumns() as $col) {
- $script .= "
- \$version->set" . $col->getPhpName() . "(\$this->get" . $col->getPhpName() . "());";
- }
+ \$version = new {$versionARClassname}();
+ \$this->copyInto(\$version, false, false);
@staabm
Propel member
staabm added a note Apr 8, 2013

nit: would be easier to read as

\$this->copyInto(\$version, $deepCopy = false, $makeNew = false);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.