U4-6148 - Grid values should not store meta data/grid configuration, the grid config should be accessed globally for use in rendering the grid

Created by Geoff Beaumont 16 Jan 2015, 13:38:15 Updated by Geoff Beaumont 17 Jun 2015, 08:30:42

Relates to: U4-6427

Relates to: U4-6533

Relates to: U4-6102

According to the documentation at http://our.umbraco.org/documentation/using-umbraco/backoffice-overview/Property-Editors/Built-in-Property-Editors-v7/Grid-Layout it's possible to set a custom render view location in /config/grid.editors.config.js using the optional "render" property (over-riding the default of changing the file extension of the editor view set in the "view" property from .html to .cshtml). This is useful for keeping front end presentation views all in the same place - but it doesn't work.

The problem is that although /Views/Partials/Grid/Editors/Base.cshtml does check for the render property, this doesn't appear in the JSON saved to the grid property value (the editor render views don't pull and data from the definitions, they use a copy stored in the content type property - obviously, this also means that any changes made to the definitions will not be reflected in existing content).

Comments

Geoff Beaumont 19 Jan 2015, 12:57:30

Okay, after some further testing I'm not sure why I wasn't getting updated definitions in the property value - it now seems to be getting updated when I save the property (maybe a cacheing issue before?). This still strikes me as a really, really bad design - not only does is massively inflate the amount of data stored for the property for every version of every content item using a grid, but if the render config in grid.editors.config.js is updated then those changes will not be reflected by the front end until each and every content item which uses the affected editors has been edited and republished. If what I saw before was the browser cacheing the editors config, then it would also be necessary to ensure that all editors had cleared their browser cache to avoid the properties getting overridden with the old configuration...

This seems very badly broken by design!


Allan Kirk 24 Apr 2015, 02:31:33

I'm not sure if this is the same bug, so let me try to explain the problem I have: We create a grid, with one layout, and one type of row. In that row we have three editors, say RTE, Title, and Image. We create a page using that configuration, but without adding an image. We now notice that it was actually a mistake to allow Image in the first place, so we remove the option from the row. The page that had previously been created can still use the Image editor.

Is it a variation of the same bug?


Geoff Beaumont 24 Apr 2015, 08:08:44

Hi @AllanKirk, yes, sounds like your editors still have the old grid configuration cached in their browsers. The only fix is to clear the cache on each and every browser used by your editors...

This is obviously a relatively innocuous example of the problem (worst case some adds an image to a page that shouldn't have one), but in other cases it can actually break things (e.g. an updated grid editor that requires configuration changes, maybe new settings).


Geoff Beaumont 24 Apr 2015, 08:13:05

Hi @AllanKirk, which version of Umbraco are you using? The caching issue should be fixed in 7.2.3 per U4-6427.


Allan Kirk 27 Apr 2015, 09:00:59

Hmm, I am not able to reproduce right now. Let me get back to you on this one.

Meanwhile, I have the same problem with row configurations. Even if I remove a row configuration from a layout, they still show up on pages in the content section. This is Umbraco 7.2.4 - is this still the same bug?


Shannon Deminick 27 Apr 2015, 09:07:02

This sounds like a variation of the same issue where the config is stored locally in the item itself and not using the actual global config. If you re-save the item, it should update with the new config and persist it locally. This of course and annoyingly would mean you'd have to re-save every item to see the updates.


Allan Kirk 27 Apr 2015, 09:19:38

Here is what I have tried, with no success:

  • Save
  • Save and publish
  • Republish the entire site with /Umbraco/dialogs/republish.aspx
  • Clean cache
  • Open the page in a browser that have never access the site

I don't think this is a client cache issue.


Shannon Deminick 27 Apr 2015, 09:23:18

Did you read my previous msg?


Allan Kirk 27 Apr 2015, 09:28:23

Yes, I have tried saving, and all the other things in my message.

I am doing all of this on the page in the content section. Is that not what you meant?


Geoff Beaumont 27 Apr 2015, 10:31:38

Hi @AllanKirk could we just clarify #When you tried a browser with a clean cache, this was to edit the content item, not view it in the front end? #When you say you can still add an image to a content item that shouldn't have one, we're talking about in the editor, not that they're still rendered in the front end? #If we are talking editor, can you add new image blocks, or just edit existing ones? My memory is that when I've removed editor availability from a layout cell then that editor has disappeared when I've opened the page in the back office to edit it, but I'm not 100% sure of this.

Note that republishing the site will have no effect - it simply republishes the currently saved data, only editing the individual content item will change that content item.

Client caching is not relevant to the front end (or no more than it is for any other web page) - it only affects editing in the back office.

The whole grid editor configuration thing is a bit of a mess at the moment, but I'm confused about what's actually going here!


Allan Kirk 27 Apr 2015, 11:23:38

Hi I'm very sorry, but I have intermingled two different scenarios here. Let me reproduce, and explain.

  1. Install Umbraco 7.2.4
  2. Create a new grid datatype, with the default layouts, rows etc.
  3. create a new document type with the grid datatype
  4. Create a new page, add a full width layout
  5. Add an Headline row
  6. Add a rich text editor to that row, and add some text.
  7. Remove one of the editors from the row (in the configuration. I used the Quote module). This editor used to remain on the page, but this has been fixed. Thanks for that.
  8. Now go and remove a row configuration from the layout. I have removed the Article row from the 1 column layout.
  9. The row is still available on the page that was previously created. I cannot remove this is any way, except by removing the last row from the page, and recreate everything.

I hope that explains it.


Shannon Deminick 11 May 2015, 07:27:06

Right, yes i can replicate this issue. The unfortunate part is that I'm not sure any good way to fix this right now because there is no unique reference for a grid/row/section, so it cannot be accurately looked up to what it should be in the config. In this scenario, the allowed items are not stored with the item's value because at that time everything was allowed. Similarly it would be the opposite for if you created data with sections that had filtered allowed types and then allowed all. It really comes down to an issue with how grid data is stored -> The value should not store metadata/config.

To fix this we have to do a few things:

  • Uniquely identify everything by immutable Ids: layout, row configs
  • These ids are stored with the grid value and no other metadata/config is stored with the value
  • When we render the editor we reference each layout/row config by ID to the real config stored in pre-values

Until that is done, we can't really fix that specific issue because there's no way to lookup the configuration.... so essentially it's the same issues mentioned in this report


Shannon Deminick 17 Jun 2015, 08:13:41

Ok, so with 7.3, here's what is fixed:

  • We no longer store most configuration values - we still store the column width though
  • We merge the grid configuration values onto the front-end value used at runtime so that the front-end value is not stale
  • We merge the grid pre-values onto the value in the back office at runtime so that the allowed templates/sections/rows are up to date with what is configured and the values are not stale
  • Have created a core Grid property value converter for this runtime logic which is overridable with custom property value converters - IMPORTANT: However, the value of the grid will have changed for any new saved values which means that your custom grid value converter will require the merging logic that we've implemented in the Umbraco.Core.PropertyEditors.ValueConverters.GridValueConverter (https://github.com/umbraco/Umbraco-CMS/blob/dev-v7/src/Umbraco.Core/PropertyEditors/ValueConverters/GridValueConverter.cs) ... Therefore this is marked as a breaking change.

In 7.3+ we need to fix this issue: http://issues.umbraco.org/issue/U4-6533 which should solve the edge case issues mentioned in here


Geoff Beaumont 17 Jun 2015, 08:30:42

Thanks Shannon - that's definitely a big step forward. Looking forward to 7.3!


Priority: Major

Type: Bug

State: Fixed

Assignee: Shannon Deminick

Difficulty: Normal

Category:

Backwards Compatible: False

Fix Submitted:

Affected versions: 7.2.0, 7.2.1, 7.2.4

Due in version: 7.3.0

Sprint:

Story Points:

Cycle: