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

Potentially unexpected behaviour when performing write() on Versioned DataObject in reading mode #382

Open
chrispenny opened this issue Nov 29, 2022 · 1 comment

Comments

@chrispenny
Copy link
Contributor

Hi team,

The context for this one is that I have some logic to create missing (Versioned) DB records, and this can be triggered by author actions, but also by frontend user requests (if for some reason the records were missing for a frontend user's request).

What I noticed was that if a frontend user is browsing the website (in a LIVE stage), then my DB records are (effectively) published even though I was only ever calling write() on them.

This isn't necessarily problematic, but it was a bit unexpected for me, so I thought I should raise it, even if just for visibility for others.

Set up

MyVersionedDataObject:

class MyVersionedDataObject extends DataObject
{
    private static string $table_name = 'MyVersionedDataObject';

    private static array $db = [
        'Title' => 'Varchar',
    ];

    private static array $extensions = [
        Versioned::class,
    ];
}

MyBlock.php:
Maybe not the most elegant set up, but I just had one of my Blocks write a new record whenever I request $Title in the template.

class MyBlock extends BaseElement
{
    public function getTitle(): bool
    {
        $record = MyVersionedDataObject::create();
        $record->Title = 'Test DO';
        // Only performing write(), not publishSingle/Recursive()
        $record->write();

        return $this->Title;
    }
}

MyBlock.ss:

<h1>$Title</h1>

Tables

  • MyVersionedDataObject
  • MyVersionedDataObject_Live
  • MyVersionedDataObject_Versions

Browsing with ?stage=Stage

Browsing to a Page that has this block with ?stage=Stage in my URL has the result that I was expecting. The DataObject is in "draft only".

MyVersionedDataObject

ID ClassName LastEdited Created Version Title
1 App\Models\MyVersionedDataObject 2022-11-30 10:13:44 2022-11-30 10:13:44 1 Test DO

MyVersionedDataObject_Live

ID ClassName LastEdited Created Version Title

MyVersionedDataObject_Versions

ID RecordID Version WasPublished WasDeleted WasDraft AuthorID PublisherID ClassName LastEdited Created Title
1 1 1 0 0 1 1 0 App\Models\MyVersionedDataObject 2022-11-30 10:13:44 2022-11-30 10:13:44 Test DO

Browsing with ?stage=Live

This is where things got "unexpected" for me. Browsing the same Page/Block, but now the user is in a LIVE reading mode.

MyVersionedDataObject

ID ClassName LastEdited Created Version Title
1 App\Models\MyVersionedDataObject 2022-11-30 10:17:24 2022-11-30 10:17:24 1 Test DO

MyVersionedDataObject_Live

ID ClassName LastEdited Created Version Title
1 App\Models\MyVersionedDataObject 2022-11-30 10:17:24 2022-11-30 10:17:24 1 Test DO

MyVersionedDataObject_Versions

ID RecordID Version WasPublished WasDeleted WasDraft AuthorID PublisherID ClassName LastEdited Created Title
1 1 1 1 0 1 1 1 App\Models\MyVersionedDataObject 2022-11-30 10:17:24 2022-11-30 10:17:24 Test DO

Unit test set up

We can simulate the same thing in unit tests / etc with something like this:

Versioned::withVersionedMode(function (): void {
    Versioned::set_stage(Versioned::LIVE);

    $record = MyVersionedDataObject::create();
    $record->write();

    // isPublished() returns true, but I was expecting false since I haven't called publishSingle/Recursive()
    Debug::dump($record->isPublished());

    // I can also re-fetch the record, and that is still published
});

Solution

Well, I'm not sure. I guess it depends if this is expected behaviour or not.

At the very least, I do think that this behaviour will be unexpected to some developers (it certainly was to me). This probably comes down to a lack of understanding on how write() and publishSingle/Recursive() work. I now understand that publish..() is actually just setting the reading mode to LIVE and then performing a write(), and with that knowledge I think that this behaviour makes sense. I think though that most of us come into this thinking that since there are two distinct methods, that write() is for DRAFT and publish...() is for LIVE.

My takeaway from this (for now at least) is that I actually need to be far more aware of the context in which my code might be triggered. If I believe that my code can be triggered within a LIVE stage, then I actually need to wrap any write()/publish() actions within the DRAFT stage.

EG:

Versioned::withVersionedMode(static function () use ($record): void {
    Versioned::set_stage(Versioned::DRAFT);

    $record->write();
});
@kinglozzer
Copy link
Member

I think this is at least intended behaviour, even if it’s a bit unexpected - as far as I remember ->write() has always performed operations on the current stage. I think the docs here are actually a bit misleading too which doesn’t help:

https://docs.silverstripe.org/en/4/developer_guides/model/versioning/#writing-changes-to-a-versioned-dataobject

When you call the write() method on a versioned DataObject, this will transparently create a new version of this DataObject in the Stage stage.

That only happens if the current stage is set to Stage, not Live (as you’ve found out!). We also have a writeToStage() method which only gets a quick mention further down the doc, not sure why that’s categorised as a “low-level” method

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants