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

Model won't update if @id column name is different to #107

Closed
JamesMahy opened this issue Mar 21, 2020 · 3 comments
Closed

Model won't update if @id column name is different to #107

JamesMahy opened this issue Mar 21, 2020 · 3 comments
Assignees
Labels
bug Fixed Fixed bug

Comments

@JamesMahy
Copy link

Schema

CREATE TABLE `some_table` (
  `id_field` INT(20) NOT NULL AUTO_INCREMENT,
  `value_field` VARCHAR(50) NULL,
  PRIMARY KEY (`id_field`)  
);

Model

namespace models;

/**
 * @table("name"=>"some_table")
 **/
class MyModel
{
	/**
	 * @id
	 * @column("name"=>"id_field")
	 */
	private $idField;
	
	/**
	 * @column("name"=>"value_field")
	 */
	private $valueField;
	
	public function getIdField(){return $this->idField;}
	public function setIdField($id=0){$this->idField = $id;}
	
	public function getValueField(){return $this->valueField;}
	public function setValueField($value){$this->valueField = $value;}	
}

Code

$item = DAO::getOne(MyModel::class, 'id_field = ?', false, [1]);
$item->setValueField('hello');
if(DAO::save(item)){
  echo 'congrats';
}

When i echo out $sql after line 235 of DAOUpdatesTrait.php i get the following sql

UPDATE `some_table` SET `id_field`= :id_field,`value_field`= :value_field WHERE `idField`= :idField

The fields in the where clause should be based on the @column annotation not the variable name!

Thanks

@JamesMahy
Copy link
Author

I'm not sure if in the scheme of the framework this is the right solution but changing

public static function getFieldsAndValues_($instance, $members) {
		$ret = [ ];
		foreach ( $members as $member ) {
			$v = Reflexion::getMemberValue ( $instance, $member );
			$ret [$member] = $v;
		}
		return $ret;
	}

to

public static function getFieldsAndValues_($instance, $members) {
		$ret = [ ];
		foreach ( $members as $member ) {
			$k = self::getFieldName(get_class($instance), $member);			
			$v = Reflexion::getMemberValue ( $instance, $member );
			$ret [$k] = $v;
		}
		return $ret;
	}

Solves the issue, however it does mean that if someone wants to update the ID based on an old ID it will fail - not a common use case however

@jcheron jcheron added the bug label Mar 21, 2020
jcheron added a commit that referenced this issue Mar 22, 2020
@jcheron
Copy link
Contributor

jcheron commented Mar 22, 2020

Thanks @JamesMahy
I just pulled an unnecessarily repeated operation out of the foreach:

public static function getFieldsAndValues_($instance, $members) {
$ret = [ ];
$fieldnames = self::getAnnotationInfo ( \get_class ( $instance ), '#fieldNames' );
foreach ( $members as $member ) {
$v = Reflexion::getMemberValue ( $instance, $member );
$ret [$fieldnames [$member] ?? $member] = $v;
}
return $ret;
}

@JamesMahy
Copy link
Author

Thanks, looks good!

I tried something similar although without prior understanding of how the framework fits together couldn't make it work

@jcheron jcheron added the Fixed Fixed bug label Mar 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixed Fixed bug
Projects
None yet
Development

No branches or pull requests

2 participants