$list->update($dataObject) rather than $dataObject->write()

Showing 1-2 of 2 messages
$list->update($dataObject) rather than $dataObject->write() Sam Minnée 8/29/12 10:46 PM
Howard Grigg's pull request regarding many_many_extraFields highlights an interesting problem:

The issue is basically that persistence via a $dataObject->write() call only works in situations where it's possible to write an object without reference to t

The pull request comment is included below.  Are we happy to bring in $list->update($dataObject) as the recommended way of persisting data in generic situations such as GridField persistence?


I don't want to bolt on a new API for this without thinking a bit more carefully about what we're doing.

Right now the way you write an object is to call the write method on the object itself:

$list = $something->SomeManyManyRelation();
$object = $list->First();
$object->SomeField = "value";
$object->SomeManyManyExtraField = "value";

This is fine for regular fields but breaks down with many_many_extraFields:

$list = $something->SomeManyManyRelation();
$object = $list->First();
$object->SomeManyManyExtraField = "value";
$object->write(); // nothing happens

Rather than have a special way of dealing with many_many_extraFields, I'd prefer to have a generalised API that classes like GridField can "just use" without worrying about the details. This would work:

$list = $something->SomeManyManyRelation();
$object = $list->First();
$object->SomeManyManyExtraField = "value";
$list->update($object); // calls $object->write() as well as updating the many-many list

This could be extended to things other than many-many list:

$openIssues = Issue::get()->filter(array("Status" => "Open"));
// This new API would define what happens when an record is added to this list
$openIssues->setFieldUpdatingHints(array("Status" => "Open"));

$i = new Issue;
$i->Title = "Something";
$openIssues->update($i); // Status would be set to "Open" as well as $i being written to the database.

If used consistently, the DataObjects wouldn't need to worry about their own persistence nearly as much, which would make it easier to swap in different persistence back-ends.

It's in this context that I think we should solve the problem that Howard has identified. As I've spec'ed it out, it would mean:

  • Create DataList::update($object) as a new way of writing $object.
  • Remove direct $object->write() calls in GridField, etc, in favour of $list->update($object).
  • Override ManyManyList::update() in a manner similar to Howard's suggestions.
  • Further down the line: introduce setFieldUpdatingHints(), or even look at automatically inferring this fromfilter() commands.

However, we'd need to agree on the API first.

Re: $list->update($dataObject) rather than $dataObject->write() Ingo Schommer 8/30/12 1:03 AM
Talked to Sam, my concern is that the semantics won't be very clear once you get into filtered DataList instances.

The setFieldUpdatingHints() helps somewhat, although its a lot of boilerplate work to make a baseline API useable (e.g. it would have to be defined for every filtered GridField). 

It also falls short on further modifiers such as limit().
We'd need to check if a new record would still be included in the list, ideally *before* allowing to add it (= disable the "add" button otherwise?).
$list = Product::get()->filter('Name', 'foo')->limit(10);
// Sets $product->Name = foo
// Fails if there are already 10 items in the list

As soon as you get into joins and such, recreating the correct state to have an item included in a given list will be quite difficult to specify. In terms of UI, it would also need to prepopulate and disable form fields on new and existing records.

That being said, I agree that its a powerful concept, and I'm generally happy about the shift away from the "active record" pattern (DataObject->write()) towards an "entity manager" pattern (DataList->write()).