-
Notifications
You must be signed in to change notification settings - Fork 101
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
Field defaults not used #215
Comments
I'll correct myself, seems I was to quick on the draw. |
I think that's the intended behaviour. With the |
That's the problem: it doesn't. Consider this stupid example: namespace Entity;
class Post extends \Spot\Entity {
[...]
public static function fields() {
return array(
'id' => array('type' => 'integer', 'primary' => true),
'title' => array('type' => 'string', 'required' => true, 'value' => 'Title'),
'text' => array('type' => 'text', 'required' => true, 'default' => ''),
);
}
} Then: $entity = $mapper->build(array(
'id' => 1
)); What should be the value for $entity->text? In spot2 current state, it's NULL. $entity = $mapper->get(array(
'id' => 1
)); $entity->_data["text"] is still NULL, and $entity->_dataModified["text"] is "", as expected (but $entity->_dataModified["id"] is then NULL in this example, because the record was not found) |
In my mapper there is getEmptyRecord()
|
I think that you have to save the entity to have the default values filled in. I haven't got any problems with it in the past but currently I can't test it for you. |
You don't have to save it. But getting an entity with an id assumes it will get it from the database, so there is absolutely no need to have fields replaces by its default. The saved database row is alwas the thruth in case of a get(id). |
The https://dev.mysql.com/doc/refman/5.7/en/data-type-defaults.html Let's assume I have the following entity. class Test extends \Spot\Entity
{
protected static $table = "tests";
public static function fields()
{
return [
"id" => ["type" => "integer", "unsigned" => true, "primary" => true, "autoincrement" => true],
"foo" => ["type" => "string", "length" => 32, "default" => "hello"],
"bar" => ["type" => "string", "length" => 32, "default" => "hello", "notnull" => false],
"pop" => ["type" => "string", "length" => 32, "value" => "hello"],
"baz" => ["type" => "string", "length" => 32, "value" => "hello", "notnull" => false],
"created_at" => ["type" => "datetime", "value" => new \DateTime()],
"updated_at" => ["type" => "datetime", "value" => new \DateTime()]
];
}
} Migration query will look like this: CREATE TABLE `tests`
`id` INT UNSIGNED AUTO_INCREMENT NOT NULL,
`foo` VARCHAR(32) DEFAULT 'hello',
`bar` VARCHAR(32) DEFAULT 'hello',
`pop` VARCHAR(32) DEFAULT NULL,
`baz` VARCHAR(32) DEFAULT NULL,
`created_at` DATETIME DEFAULT NULL,
`updated_at` DATETIME DEFAULT NULL,
PRIMARY KEY(`id`) ; Now lets create empty object and save it: $test = new App\Test;
$spot->mapper("App\Test")->save($test); Query will look like this: INSERT INTO `tests`
(`foo`, `bar`, `pop`, `baz`, `created_at`, `updated_at`)
VALUES
(NULL, NULL, 'hello', 'hello', '2016-12-22 07:17:10', '2016-12-22 07:17:10')
Now lets try a query without NULL values: INSERT INTO `tests`
(`created_at`, `updated_at`)
VALUES
('2016-12-22 07:17:10', '2016-12-22 07:17:10')
tl;dr |
Thank you for your reply. I assumed that much but a confirmation from an experienced user is nice. Considering your example, as I said earlier, a user not familiar with Spot would expect both I'll close the issue, since the question "Does |
@Arzaroth I do understand your confusion. It does seem like the name |
I'll re-open the issue so we can talk about this here then. According to my limited experience with Spot, I'd say they are a few things to consider:
The implementation itself would be pretty straightforward, being for instance: diff --git a/lib/Entity.php b/lib/Entity.php
index db3f3a9..78276ff 100644
--- a/lib/Entity.php
+++ b/lib/Entity.php
@@ -101,7 +101,7 @@ abstract class Entity implements EntityInterface, \JsonSerializable
$fields = static::fields();
foreach ($fields as $field => $opts) {
if (!isset($this->_data[$field])) {
- $this->_data[$field] = isset($opts['value']) ? $opts['value'] : null;
+ $this->_data[$field] = isset($opts['value']) ? $opts['value'] : (isset($opts['default']) ? $opts['default'] : null);
}
} The way I see it, rather than changing the way |
Sorry, my bad! :) I thought Spot sets the default values for the entity, but I forgot that my own application does it for me. (And in some cases I set the "value" to the "default" value redundantly. Which is pointless I think.) I remember now that I had this kind of rage/confusion when started using Spot. I don't mind the modification you made. I think it's reasonable and it should be enough to set one of the parameters (default), value is not needed in the most cases. But If you specify the value by hand it should be the primary choice. |
I just started using Spot (it looks like exactly what I want!). However, this really confuses me... I have a table with columns: id, title, color.
and have an entry with a new id and a default color of 'red'. However, Spot is inserting null for color. Is this expected? |
Sorry, user error...setting value to red does what I want. |
I can think of one case for which the separation of "default" and "value" is important and it is when setting a default that changes frequently like dates: This doesn't mean we can't populate the entity with "default", but that "value" has a use and should not be removed. |
I think the current behaviour is correct. This is more of a documentation problem. |
I think that @Arzaroth's modification is better than the current implementation. |
Resolved with #229 being merged. |
Hi,
First of all, thanks for this ORM. I needed something simple, non-intrusive, without annotations, and this delivers.
So this issue is fairly simple, in the documentation (http://phpdatamapper.com/docs/entities/), we can see the 'status' field is using "'default' => 0" in its definition. so one would assume it's building an Entity with such fields sets values according to this field if existing.
While this is the case when "'value' => 0" is set in the field definition, using 'default' doesn't seem to do much.
My understanding would be 'default' sets field value on insert if value is missing (i.e. NULL) and required, and 'value' sets it on entity build.
AFAIK, Entity::initFields() takes 'value' into account, and 'default' is used in Entity\Manager::fields() [206-207] to populate Entity\Manager::fieldDefaultValues, which is later used in Mapper::get() [518], but doesn't set any value on create.
So my question is fairly simple: is 'default' really relevant, since in almost any use case, 'value' does the trick? It seems there is, as of now, no point in using it. And if it's not, my guess is it shouldn't be referenced in the documentation.
The text was updated successfully, but these errors were encountered: