Versioned::deleteFromStage clarification

45 views
Skip to first unread message

Matthew Bonner

unread,
Feb 24, 2015, 7:44:28 AM2/24/15
to silverst...@googlegroups.com
I'm not sure if this is a bug or not, or whether this should even be discussed here, to be fair I'm pretty confused on this issue full stop so I'm posting here as I'm hoping for clarification or confirmation if possible.

We have a NewsItem class extending the DataObjectAsPage and the majority of code was migrated from SS2 to SS3.1.

The NewsItem class has the extension Versioned so that NewsItems can be staged.

The NewsItems were not deleting upon the migration, and the error we were getting was that the doDelete method was missing.

So we added the doDelete method like so:
  public function doDelete () {
    if ($this->hasExtension('Versioned')) {
      $this->deleteFromStage("Live");
      $this->deleteFromStage("Stage");
    }
    $this->delete();
  }

However, the NewsItems were not deleting correctly.

Digging through the code, I narrowed the issue down to a line in the DataObject.php file:
$srcQuery = DataList::create($this->class, $this->model)->where("ID = $this->ID")->dataQuery()->query();

This line is present in the delete method.

Stepping through the code, the this->deleteFromStage("Live"); calls Versioned::deleteFromStage('Live') naturally as the extension contains the method.

Subsequently, Versioned::deleteFromStage('Live') calls DataObject::delete, which contains the following line:
$srcQuery = DataList::create($this->class, $this->model)->where("ID = $this->ID")->dataQuery()->query();

The problem with the above line is that $this->class contains NewsItem, not NewsItem_Live which means that when the DataList is created, it loses the stage you want to delete from.

Now I would have assumed that the DataList::create is probably not used correctly or something, but like I said, I'm not sure.

There is a comment attached which says that this is basically a hack anyway.

Can someone clarify whether I'm in the wrong, or whether this is a code bug (and if so if they can find a bug report because I can't), or whether someone is looking into this, or whether anyone is aware of this.

As at the moment, I'm pretty stumped to what the best solution is. To get around the issue, I've updated the doDelete method so that it uses SQLQuery to delete the records from the appropriate tables, as I think this might be the first question, in terms of a "why don't you do..." but I also feel this is a pretty nasty workaround.

Gregory Smirnov

unread,
Feb 24, 2015, 8:47:23 AM2/24/15
to silverst...@googlegroups.com

Hello Matthew,

 

The expected behaviour in SS3 is covered by DataQuery Parameters feature. You should never specify tables for DataList with ‘_Live’ suffix.

When you call $dataList->dataQuery()->query() you should get properly formed SQLQuery with tabled renamed with ‘_Live’ suffix according to the environment.

 

When original SQLQuery was created, the method augmentSQL( $query ) is called on all assigned extensions. Versioned extension sets a special parameter Versioned.mode when DataQuery is created, and later uses it to augment SQL according to the set stage. So in your example when you call $object->delteFromStage(‘Live’) you should have properly formatted query (as $srcQuery in the DataObject::delete() method) with table names renamed with suffix ‘_Live’.

 

In your example of doDelete() you actually try to delete 3 times when dataobject have Versioned extension: from Live, from Stage, from Current Stage (should be Live by default, but then depends on CMS session). You missed ‘else’ there, since delete() is actually called inside deleteFromStage().

 

Gregory

--
You received this message because you are subscribed to the Google Groups "SilverStripe Core Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to silverstripe-d...@googlegroups.com.
To post to this group, send email to silverst...@googlegroups.com.
Visit this group at http://groups.google.com/group/silverstripe-dev.
For more options, visit https://groups.google.com/d/optout.

Matthew Bonner

unread,
Feb 24, 2015, 9:25:47 AM2/24/15
to silverst...@googlegroups.com
Hi Gregory

Thanks for the explanation, I'll have to do some more digging, because when augmentSQL is called, the $queryParams property is NULL and based on your explanation it shouldn't be.

This will probably explain a few random issues we've been having so I'm going to see if I can get to the bottom of the issue.

Thanks for your quick response.

Matthew Bonner

unread,
Feb 25, 2015, 10:24:34 AM2/25/15
to silverst...@googlegroups.com
Looks to just be a problem around the DataObjectAsPage inheriting classes, has anyone else encountered this issue before?

Matthew Bonner

unread,
Feb 25, 2015, 10:50:49 AM2/25/15
to silverst...@googlegroups.com
Sorry, I can see there has been a pull request to fix this issue which has not been merged in:
https://github.com/dhensby/DataObject-as-Page/commit/ad6e2245353414a6387690e48732987ac13845e0

All sorted now. Thanks, and hopefully this will help others having the same problem.
Reply all
Reply to author
Forward
0 new messages