silverstripe / silverstripe-elemental

Create pages in Silverstripe CMS using content blocks
http://dna.co.nz
BSD 3-Clause "New" or "Revised" License
110 stars 115 forks source link

Draft changes to a page with a modified pagetype can stop blocks from displaying on the live site #823

Closed sunnysideup closed 4 years ago

sunnysideup commented 4 years ago

DNA/Elemental 4.4.0 / SS/Framework 4.6.1

When edit a page and save (i.e. published and draft version of page are different). Elemental block is published.

When we visit the page, logged in, all shows perfectly On logged out browser, ALL elemental blocks go missing.

I think it relates to this: https://github.com/dnadesign/silverstripe-elemental/blob/4/src/Models/ElementalArea.php#L236

because when I change DRAFT to LIVE on that line, it works fine.

I had a comparison of the SiteTree, Page and ElementalArea table for Draft and Live.

There is one thing that is not right. The ElementalArea.OwnerClassName does not match the class name of the page (it is Page but it should be MyPage) in both ElementalArea and ElementalArea_Live.

I have tried to save and publish (with changes) the page a few times, but that does not make a difference. Also adding elemental blocks does not matter.

When I duplicate the page, the error does not occur, and so the duplicated page works as expected and in the OwnerClassName field says MyPage, not Page.

sunnysideup commented 4 years ago

To make it easier for yous I have put together a test case:

https://github.com/sunnysideup/silverstripe-test-elemental

(also see DB!)

Cheddam commented 4 years ago

Hey @sunnysideup, this should be resolved by #671, which will be included in the next minor Elemental release. Can you confirm this fixes the issue for you by installing Elemental 4.x-dev?

sunnysideup commented 4 years ago

@Cheddam - sorry, no, it does not solve it.

Here I am logged in: image

Here I am not logged in: image

As you can see on Firefoxy - it shows nothing even though the LIVE page has content.

here is the version I am using:

dnadesign/silverstripe-elemental          4.x-dev 6ef05b8
sunnysideup commented 4 years ago

Here is how we fixed it:

<?php

namespace BestClientInTheUniverse\App\Tasks

use SilverStripe\Orm\DB;
use SilverStripe\Dev\BuildTask;

class FixPagesWithElementalArea extends BuildTask
{
    protected $title = 'Fix pages with bad elemental areas';

    protected $description = 'Goes through all the pages and fixes the elemental area OwnerClassName field';

    protected $enabled = true;

    public function run($request)
    {
        foreach(['', '_Live'] as $ext) {
            DB::query('
                UPDATE ElementalArea'.$ext.'
                    INNER JOIN Page'.$ext.'
                        ON Page'.$ext.'.ElementalAreaID = ElementalArea'.$ext.'.ID
                    INNER JOIN SiteTree'.$ext.'
                        ON Page'.$ext.'.ID = SiteTree'.$ext.'.ID
                SET ElementalArea'.$ext.'.OwnerClassName = SiteTree'.$ext.'.ClassName

            ');
        }
        echo 'DONE';
    }
}
brynwhyman commented 4 years ago

This sounds very similar to https://github.com/dnadesign/silverstripe-elemental/issues/786 ?

emteknetnz commented 4 years ago

@sunnysideup when you say that updating to the latest elemental 4.x-dev did not fix the issue, did you try resaving the page? (or saving all the pages, which seems like a giant hassle)

I did notice you mention that When I duplicate the page, the error does not occur, and so the duplicated page works as expected and in the OwnerClassName field says MyPage, not Page.

The fix @Cheddam was referring to is this https://github.com/dnadesign/silverstripe-elemental/pull/816/files - which updates OwnerClassName after before write, which a page duplciate will trigger

@brynwhyman I'm wondering if it's worth us also releasing a build task in the elemental module like what @sunnysideup outlined above to complement the community PR that Garion merged and mention it in patch notes. I'd say it's going to be a pretty small amount of work given we already know what the problem and the solution is

sunnysideup commented 4 years ago

Saving it again seems to make it work!

Leapfrognz commented 4 years ago

Also experiencing this issue

michalkleiner commented 4 years ago

Our govt client using CWP is also affected by this problem.

rafaeldsousa commented 4 years ago

4.x-dev branch seems to have the issue resolved. Would be ideal to have this on a new release asap, as this can turn into a big blocker for our clients trying to load new content on sites that can be yet viewed.

brynwhyman commented 4 years ago

@dnsl48 I'm going to bump the label to impact/critical. While it's resolved in the dev branch, the fix deserves back porting (already happening)

At this stage, I don't think we should be committing a dev task to this module to resolve the issue for existing blocks. Workarounds and collaboration in comments isn't a bad thing! (Thanks for sharing your work @sunnysideup)

dnsl48 commented 4 years ago

Hey @sunnysideup, thank you for reporting the issue!

I've been trying to reproduce it several times with CWP 2.6.1-rc1 and then with your testcase (https://github.com/sunnysideup/silverstripe-test-elemental). However, looks like I cannot catch it in action. Could you please explain step by step how should I do that? /cc @Leapfrognz @michalkleiner @rafaeldsousa

Currently, I'm trying to:

  1. Create a new page with elemental area
  2. Add a new block
  3. Publish the page
  4. Check the page in 2 browsers (as admin in one browser, as unauthenticated user in another)
  5. The content is in place in both browsers at this point.
  6. Edit the block content, "Save" but not publish the page
  7. Refresh the page in both browsers
  8. Result: I'm seeing the content in both browsers at this point

Do I need to do any extra steps or something differently?


first-try

michalkleiner commented 4 years ago

@dnsl48 your step 6 should be "edit a field on the page itself and save it as draft", so the difference would be you don't touch any of the published blocks, but the page.

dnsl48 commented 4 years ago

your step 6 should be "edit a field on the page itself and save it as draft"

@michalkleiner still cannot reproduce. Can see the content in both browsers. Tried in both CWP 4.6.1-rc1 and https://github.com/sunnysideup/silverstripe-test-elemental

rafaeldsousa commented 4 years ago

@dnsl48 I can reproduce in one specific site of mine, even if I downgrade to elemental 4.3.0. Other devs in house have tried and couldn't reproduce. I've also tried in a few other local sites and couldn't reproduce. Some suggested that this could possibly be related to incompatibility with another module (elemental module perhaps), but I'd need to check further to see which one could be causing the issue. For the site I'm having issues these are the modules.

"require": {
        "benmanu/silverstripe-styleguide": "0.1.x",
        "silverstripe/recipe-cms": "^4.4",
        "silverstripe/fulltextsearch": "dev-fix/queuedreindex",
        "silverstripe/recipe-authoring-tools": "^1.2",
        "silverstripe/recipe-collaboration": "^1.2",
        "silverstripe/recipe-reporting-tools": "^1.2",
        "silverstripe/recipe-blog": "^1.1",
        "silverstripe/recipe-form-building": "^1.2",
        "silverstripe/recipe-content-blocks": "^2.2",
        "silverstripe/redirectedurls": "2.x",
        "wilr/silverstripe-googlesitemaps": "^2.1",
        "dnadesign/silverstripe-elemental-list": "dev-master",
        "unclecheese/display-logic": "^2.0",
        "jonom/silverstripe-betternavigator": "4.x",
        "sheadawson/silverstripe-linkable": "2.0.x",
        "symbiote/silverstripe-multivaluefield": "5.0.x",
        "stevie-mayhew/silverstripe-svg": "2.1.0",
        "symbiote/silverstripe-addressable": "4.0.x-dev",
        "heyday/silverstripe-responsive-images": "^2.0",
        "silverstripe/securityreport": "2.x",
        "bummzack/sortablefile": "2.*",
        "jonom/focuspoint": "^3.1",
        "maxmind-db/reader": "~1.0",
        "hubertusanton/silverstripe-seo": "dev-master",
        "sinergi/browser-detector": "^6.1.2",
        "dnadesign/silverstripe-responsiveimageset": "dev-master",
        "dnadesign/silverstripe-elemental-virtual": "1.x-dev",
        "fullscreeninteractive/silverstripe-dropdownimagefield": "1.x-dev",
        "tractorcow/silverstripe-autocomplete": "4.x-dev",
        "silverstripe/crontask": "^2.1",
        "silverstripe/dynamodb": "^4.0",
        "wilr/silverstripe-tasker": "dev-master",
        "symbiote/silverstripe-advancedworkflow": "^5.2",
        "dnadesign/silverstripe-elemental-responsivetable": "1.0",
        "dorsetdigital/silverstripe-simpleinstagram": "^1.0",
        "dnadesign/silverstripe-signature": "^2.0",
        "silverstripe/raygun": "^3.2",
        "heyday/silverstripe-colorpalette": "^2.0",
        "ryanpotter/silverstripe-color-field": "^1.0",
        "dnadesign/silverstripe-elemental-media": "^0.12.0",
        "silverware/validator": "^1.2",
        "dnadesign/silverstripe-elemental-spotlights": "0.9.0"
}

I have tried 4.x.dev and the issue isn't present when using that branch.

Cheddam commented 4 years ago

@dnsl48 I think you were missing one other crucial step - changing the pagetype. My steps to reproduce against @sunnysideup's project:

  1. Create a Page page
  2. Add a block, publish
  3. Observe correct behaviour in incognito window
  4. Change the pagetype to My Page, publish
  5. Observe correct behaviour in incognito window
  6. Set the 'Header pre title' field to 'test', save
  7. Observe incorrect behaviour in incognito window
  8. Publish, observe correct behaviour has been restored

I can also confirm that applying the patch fixes the issue.

Cheddam commented 4 years ago

Thanks for all of the support and engagement on this issue! I've just issued Elemental 4.4.1, which includes the fix, and I encourage everyone to update at their earliest convenience. It may also be useful to reference @sunnysideup's earlier comment for a BuildTask to patch all affected pages.

If anyone is still able to reproduce this issue after upgrading to 4.4.1, please get in touch.

dnsl48 commented 4 years ago

you were missing one other crucial step - changing the pagetype

True. I did not change the page type, as I created the page as My Page initially.