silverstripe / cwp

Common Web Platform (CWP) features module. We strongly recommend using it for all new CWP projects. Future features will be delivered here.
https://www.cwp.govt.nz
BSD 3-Clause "New" or "Revised" License
10 stars 26 forks source link

Save/Publish buttons work, but occasionally don't adjust state to Saved/Published #121

Closed madmatt closed 6 years ago

madmatt commented 6 years ago

This is being raised as a bit of a catch-all issue here (as suggested by @robbieaverill), as it's not currently clear what's happening (can't be traced to a particular module etc.)

Occasionally when saving content (e.g. on an element via the elemental module) the 'Save' and 'Publish' buttons, when clicked, perform their task but don't update their status (to say 'Saved' or 'Published' instead).

It appears as though the save/publish does actually happen, but isn't being noted by the buttons, leading to inconsistent state and confusion around the state of the content.

There is a potentially related issue where the buttons don't save correctly, but it's unclear if this is due to the same bug or something else (e.g. permissions not being checked correctly or similar).

Bit of a weird one to try and investigate - but I believe @robbieaverill has seen this before locally as well, so hopefully it can be traced back. If there's any specific info that would help, let me know and I can pass this onto the client.

Thanks!

madmatt commented 6 years ago

@tractorcow suggested it might be similar to silverstripe/silverstripe-siteconfig#96, but this hasn't been investigated as yet.

robbieaverill commented 6 years ago

Possibly related: https://github.com/tractorcow/silverstripe-fluent/pull/429

ScopeyNZ commented 6 years ago

Hey @madmatt

(e.g. on an element via the elemental module)

FYI the edit forms for individual elemental modules aren't meant to change to "Saved" and "Published" when you click those buttons. It's only for SiteTree (and asset admin does it too).

ScopeyNZ commented 6 years ago

This is also an issue because of incorrectly configured Solr. This causes fatal errors but the ajax responses turn 200. This means you get no feedback client side, and the save has worked, but there's no html partial response with the updated button.

I'll keep trying to reproduce with other circumstances. @madmatt @robbieaverill do you think we should raise (or are you guys aware of) an issue to handle those types of error responses better?

robbieaverill commented 6 years ago

I do think we should have a think about the Solr thing. I feel like from the SilverStripe 3.x to 4.x version of fulltextsearch it started throwing exceptions when it wasn't configured where it didn't previously. This has caused a bunch of weird things to happen in the CMS because of it throwing the exceptions in a PHP shutdown handler at the end of the response body...

We could log a separate issue for this though.

In the interim we added support for disabling FTS via config: https://github.com/silverstripe/silverstripe-fulltextsearch/pull/210

ScopeyNZ commented 6 years ago

I'm going to close this issue. With the Solr fixes as part of silverstripe/silverstripe-fulltextsearch#222 I can't find another situation that causes this issue.

If there's another case of this happening can we please get the page type that causes the issue and some information about the AJAX request used to save the page. Incorrectly formed JSON responses can give very little feedback and cause the issue.

madmatt commented 6 years ago

Here's one that we found in project code:

Result: Page with the shortcode is saved correctly, but the CMS breaks because when running the shutdown function, there is no current Controller stack, so Controller::curr() returns the "No current controller available" error.

The fix here for us was to not include the page during processing (which isn't ideal, because we want the accordion to only link to stuff on the page it's added to, but whatevs).

I haven't tested this to see how it works using the deferred FTS indexer (which does it via queuedjobs), but it's an example of something that causes the CMS to mess up due to the Solr shutdown handler.