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

Incorrect "Published" status in GridField when using Fluent #75

Open
JorisDebonnet opened this issue Mar 26, 2018 · 11 comments
Open

Incorrect "Published" status in GridField when using Fluent #75

JorisDebonnet opened this issue Mar 26, 2018 · 11 comments

Comments

@JorisDebonnet
Copy link

JorisDebonnet commented Mar 26, 2018

When using the Blog module together with Fluent, any BlogPosts created in one locale will show up in the GridField as "Published" in any other locale, too. That status is not correct though, as is confirmed by going to the BlogPost's details (where everything works fine).

I'm not entirely sure whether this is something that should be fixed in Fluent or in Lumberjack. I guess it may need a 'speciale case' code block in one of both. In any case, those posts are not considered "Published" in other locales (..until they are), so I believe they should show up in the GridField as "Draft".

(I have 2.0.0-alpha1 installed of this module, and fluent 4.0.0)

@JorisDebonnet
Copy link
Author

Now that I think about it, this might be a Blog-specific thing after all, since it also has "Scheduled to publish" content in there. So perhaps the issue should be created over there instead? (I actually already did, but then saw you already reacted here)

@robbieaverill
Copy link

Hrmmm. Can you maybe provide some more specific steps to reproduce this?

I've done this:

  • Created three blog posts
  • Published two of them
  • Changed my fluent locale

For me the states in the lumberjack GridField are the same in both locales. Per your bug report I'd expect them all to be reported as "Published", but I'm not seeing that.

Side note: there are a couple of instances of incorrect Versioned API usage in blog and lumberjack that are preventing the MODIFIED badge from being displayed. PRs to fix at #80
and silverstripe/silverstripe-blog#530

@JorisDebonnet
Copy link
Author

JorisDebonnet commented Apr 20, 2018

Sure, I made a few screenshots. As in your case, I have three blog posts and two of them are published. I created the 'base' version of the Blog and the BlogPosts in Dutch.
image

Then I switched to English. This immediately shows an issue because you get the exact same overview as above. The reason this is an issue is because those two "published" posts are not, in fact, published, due to the pages not 'existing' yet in this locale.

After translating the first post to English, I get this overview:
image

There is no visual difference between BlogPost 1 and 2, but 1 is present in the English locale, while 2 is not.

So that is the issue. Perhaps this should actually be tested on a Lumberjack instance without Blog, to see how it behaves then, as Blog makes it extra confusing because of its PublishDate. (that's why I also created the issue over there)

In total, I think we have three different published states here:

  • The basic SiteTree publish
  • A BlogPost's PublishDate, a date that is usually visible on the website as the article's publication date
  • A Fluent on/off switch depending on whether the object already exists in that locale

In any case, what I would expect in the BlogPosts overview inside Blog is that I can see which articles do not exist yet in this locale. Perhaps it can be handled in Lumberjack by greying them out or something, after all, non-translated Pages appear greyed out in the SiteTree-overview too. And then Blog might have to do something to make the "Published on ..." less confusing because it may not be published in all locales.

@NightJar
Copy link

Hi @JorisDebonnet that's helpful, thanks.
Are you able to check the database to see which Locales it lists Nederlandse titel 2 as, in the SiteTree/BlogPost _Localised table?
Translatable works by copying the state of (the translatable fields for) a page, so it's possible that the EN content is still Dutch because it hasn't been altered since being copied to the English locale.

Another thing to check is that the EN Localse doesn't have a fallback to NL?

@JorisDebonnet
Copy link
Author

Hm, the database confuses me, because contrary to what I expected, all three BlogPosts are also present in the English locale in _Localised. I'm not sure when or how that happened.

So I went ahead and created a fourth BlogPost on the Dutch one and check what happens in the DB, but this time it does stay isolated on the Dutch locale, and so the explanation in my previous post holds for that one. I might have done something afterwards with those first three...

Here's what I see on the English Blog now:
image

While this shows the _Localised database for all these BlogPosts:
image

So the fourth one exists only in the Dutch locale, but still shows up visually on the English blogpost-list "as if it were published there".

@robbieaverill
Copy link

Ah yes, ok - I see the issue now. Thanks for the follow up information!

@robbieaverill robbieaverill removed their assignment Apr 22, 2018
@NightJar NightJar self-assigned this Jun 28, 2018
@NightJar
Copy link

NightJar commented Jun 29, 2018

Neat, minimal steps to reproduce:

  1. create silverstripe/installer
  2. require silverstripe/recipe-blog
  3. Add 2 locales.
  4. publish a blog page in default locale.
  5. add a blog post in default locale.
  6. copy and publish blog (not post) in second locale.
  7. observe that blog posts from default locale are shown (correct) as published (incorrect) in the CMS for this second locale.

image

Things do at least show correctly on the front-end:
image

Looking into this now.

Editing actual post page shows as expected (it is just a Page after all):
image

@NightJar
Copy link

LumberJack's GridFieldSiteTreeState::getColumnContent looks specifically at Versioned::isPublished, where as FluentSiteTreeExtension::updateSavePublishActions is what modifies the default behaviour elsewhere in the CMS. I can't imagine any module that inspects published state not running into this issue also.

The ideal in this case would be to have some kind of extension point in the Versioned... extension 🙄
But it seems ugly. I'll have a think about how best to override isPublished with isPublishedInLocale (and similar friends).

@robbieaverill
Copy link

Maybe isPublished needs an updateIsPublished extension point inside it for fluent to return isPublishedInLocale

@NightJar
Copy link

NightJar commented Jun 29, 2018

One can beat Versioned to it by introducing another extension for Fluent that applies before Versioned (where FluentVersionedExtension must be applied after). This allows us to add isPublished via CustomMethods before Versioned can, and direct the execution path down isPublishedInLocale instead.

The trick comes in attempting to think about what other checks (e.g. isModifiedOnDraft) might be in place - and the third state...
No longer is it just DRAFT or LIVE, but also NOT IN LOCALE as well. this would hold true no matter the method used to modify the published status.

All of this doesn't stop e.g. LumberJack simply checking and returning a binary state of LIVE or DRAFT only - at least not without introducing some coupling :/ (or some serious refactorying and probably API changes).

So this is quickly exploding and is worthy of more thought. I'm curious to get your thoughts on this too @tractorcow

@robbieaverill
Copy link

Was just doing some testing on how Fluent modifies the page tree and noticed that it's not completely accurate either at the moment: https://github.com/tractorcow/silverstripe-fluent/issues/434

@NightJar NightJar removed their assignment Oct 19, 2018
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

4 participants