Save/ Publish actions don't reflect state of modified block page

[brynwhyman]

Overview

The Publish action is not correctly reflecting the state of a 'modified' block page.

Here's my attempt at showing the flow (✅ = expected behaviour; ❌ = unexpected behaviour):

Action/ state	Save/ Publish actions
New page is created in DRAFT state ✅
Content block is added to the page and the state of the block page is altered by adding some content ✅
The Save button is selected and the new content is applied to a 'Draft' page version ✅
The Publish button is selected and the new 'draft' content is applied to a 'Published' page version ✅
The state of the block page is altered by adding some new content ✅
The Save button is selected and the new content is applied to a 'Modified' page version ❌	Expected:
The (broken) 'Published' button is selected and the new 'modified' content is applied to a 'Published' page version ✅
The state of the block page is again altered by adding some new content ✅

Other notes

At the point of the unexpected behaviour, the site tree also is not correctly reflecting that the block page is in a 'Modified' state, see:

However, the block status icon highlights the modified state correctly, see:

Versions:

CMS 4.5.0
Elemental 4.3.0

Potential workaround

See: #790 (comment)

[brynwhyman]

I actually thought that this was an existing issue, as I expect it's been around for sometime. However I couldn't find an open issue, so have raised this!

[mfendeksilverstripe]

@brynwhyman I can confirm that this is a missing feature. I'm familiar with a solution from one of the large projects that I worked on. I will outline the solution here to kick off a discussion. Happy to create a follow up PR for it if needed ;-)

Non-technical description

We implemented a feature which allows each block to provide the block page with information if it needs to be published or not. The block page collects this information from all its blocks and renders correct publish button state based on that. This also covers objects which are under the blocks, i.e. Carousel block will contain carousel items which also may require publishing.

This feature is very useful because we can't expect the content author to go through all objects which are under a block page and publish them individually. In fact, most projects I worked on did not even have the option to publish individual objects, the only option was to publish the whole page. This makes this feature even more valuable.

Technical description

The feature consists of multiple parts. Note that these code snippets are meant to be used as examples only.

• block page loops through each block and checks the published state
• a block can declare a NestedBlockObjectsInterface in which case a method which is expected to return publish state of the nested objects is called
• this is done in resource-efficient manner, i.e if something needs publishing the process bails out early as the result is already clear
• we also considered other approach which would not require an interface - using owns and owned_by, this approach has such problems as not having enough flexibility to specify which object needs to be published or not (for example a block can own an image, but this is a shared resource so we may not want to indicate that the page needs publishing based on the owned image)

Performance considerations

• in memory cache for list of all block objects for block page (especially useful with deep nesting)
• publish state could be cached in DB record of the block page as we have a performant way to traverse up the object tree (see TopPage)

Interface

/**
 * Interface NestedBlockObjectsInterface
 *
 * Provides a way to extract the information if nested non-block objects need to be published
 * this is used to determine block page button states
 *
 * @package App\Interfaces
 */
interface NestedBlockObjectsInterface
{
    public function checkNeedPublishNestedObjects(): bool;
}

Helper

/**
 * Class PublishStateHelper
 *
 * functionality which is related to detecting the need of publishing nested objects within a block page
 *
 * @package App\Helpers
 */
class PublishStateHelper
{
    /**
     * @param DataObject|Versioned $item
     * @return bool
     */
    public static function checkNeedPublishingItem($item): bool
    {
        /** @var Extensible $class */
        $class = get_class($item);

        if ($class::has_extension(Versioned::class)) {
            /** @var $item Versioned */
            return !$item->isPublished() || $item->stagesDiffer();
        }

        return false;
    }

    /**
     * @param SS_List $list
     * @return bool
     */
    public static function checkNeedPublishingList(SS_List $list): bool
    {
        /** @var $item Versioned */
        foreach ($list as $item) {
            if (static::checkNeedPublishingItem($item)) {
                return true;
            }
        }

        return false;
    }

    /**
     * @param SS_List $blocks
     * @return bool
     */
    public static function checkNeedPublishingBlocks(SS_List $blocks): bool
    {
        if (empty($blocks)) {
            return false;
        }

        /** @var $block Versioned */
        foreach ($blocks as $block) {
            // check if block itself is published
            $needsPublish = static::checkNeedPublishingItem($block);

            // check for if nested non-block objects need to be published
            if ($block instanceof NestedBlockObjectsInterface) {
                $needsPublish = $needsPublish || $block->checkNeedPublishNestedObjects();
            }

            if ($needsPublish) {
                return true;
            }

            // check if nested blocks are published
            if ($block instanceof ElementList) {
                if (static::checkNeedPublishingBlocks($block->Elements()->Elements())) {
                    return true;
                }
            }
        }

        return false;
    }

    /**
     * @param DataList $items
     * @param int $parentId
     * @return bool
     */
    public static function checkNeedPublishVersionedItems(DataList $items, int $parentId): bool
    {
        // check for differences in models
        foreach ($items as $item) {
            if (PublishStateHelper::checkNeedPublishingItem($item)) {
                return true;
            }
        }

        // check for deletion of a model
        $draftCount = $items->count();

        // we need to fetch live records and compare amount because if a record was deleted from stage
        // the above draft items loop will not cover the missing item
        $liveCount = Versioned::get_by_stage(
            $items->dataClass(),
            Versioned::LIVE,
            ['ParentID' => $parentId]
        )->count();

        if ($draftCount != $liveCount) {
            return true;
        }

        return false;
    }
}

Block page

public function getCMSActions()
{
    $fields = parent::getCMSActions();

    // You can't publish a page that is archived so we don't need to update the field
    if ($this->isArchived()) {
        return $fields;
    }

    if (PublishStateHelper::checkNeedPublishingBlocks($this->getBlocks())) {
        $this->setPublishButtonActiveState($fields);
    }

    return $fields;
}

Carousel block

public function checkNeedPublishNestedObjects(): bool
{
    foreach ($this->Items() as $item) {
        if (PublishStateHelper::checkNeedPublishingItem($item)) {
            return true;
        }

        if ((int) $item->CustomDealID === 0) {
            continue;
        }

        if (!$item->CustomDeal()->isPublished()) {
            return true;
        }
    }

    return false;
}

[brynwhyman]

Hey @mfendeksilverstripe thank you for the in-depth comment, that definitely makes sense to me but I'll let others chime in on your suggested implementation before go ahead and raise a PR.

I vaguely recall that when the in-line editing functionality was developed that there was something built at the React/Redux level to kind of work around this limitation and force the change of the Save/Publish actions? For example, we can see that the block area is aware of the state of each block (modified in this case):

I could definitely be wrong though :)

[mfendeksilverstripe]

I think this works only with blocks, but not with objects nested below the blocks @brynwhyman. I pointed out a need to have some block interface to indicate if block needs to be published or not possibly taking other objects in consideration.

For example, a very common block is a carousel block which has many items. Each item represents one slide of the carousel. If any of the carousel items need publishing the block should indicate that it needs publishing as well (even though the block model is already published).

[sachajudd]

Noting here that the modified state indicator colours are wrong here and were fixed in: silverstripe/silverstripe-admin#1021

[brynwhyman]

Noting that the workaround for this is to use the versioned-snapshots module which does a better job at recording versions of child objects of a page (like blocks and blocks within blocks).

There are migration issues to consider so it's not necessarily and easy workaround but it's definitely recommended.

[chrispenny]

The Snapshots module does a good job of indicating to an author that their page needs to be updated if/when they refresh the page. The trouble we still have is that inline editing does not trigger the same indication. EG: If you edit a block and save it using the inline editor tools, the page will lose the status.

For example, I edit this field of a block. The "save" and "publish" actions update.

But as soon as I use the inline editor to save.

The indicators disappear.

With the snapshots module, if I refresh the page, then the appropriate indicator is returned.

[brynwhyman]

Hey @chrispenny thanks for reporting that issue. If you haven't already, do you mind please recording that as an issue over on the snapshots module as well?

[filiplikavcan]

Have you considered not storing blocks into a database table(s) and just serialising and storing them in Content field? Something like WordPress/Gutenberg does. I think it would be great even as a lower level option in Silverstripe to be able to decide whether has_one and has_many relations should be stored in parent data object field or in separate tables.

This would solve change tracking, search indexing (given a "good" serialisation is used - Gutenberg uses html with comments, I can imagine a second *_Index field where block would serialise only searchable content, etc.), performance (no need to do multilevel sql queries), page duplication, etc.

Generally I would recommend to study Gutenberg and take an inspiration from it.