getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.27k stars 167 forks source link

Panel Discards Data When Changing Templates with Different Field Types #2159

Closed neildaniels closed 3 years ago

neildaniels commented 4 years ago

Describe the bug
In the Panel, if you switch a page template from one to another with a field that has a slightly different type (such as multiselect -> tags or email -> text), then the value for that field will be discarded.

To Reproduce
Steps to reproduce the behavior:

  1. Create template foo1
    
    title: Foo 1

options: changeTemplate:

columns:

  1. Create template foo2
    
    title: Foo 2

options: changeTemplate:

columns:

  1. Create a page with foo1 and select all the options (apple, blueberry, banana, red, blue, yellow) and type in test@example.com in the email field.
  2. Change that page's template to foo2

Expected behavior
The data for testField1 and testField2 should be preserved.

Screenshots

Screen Shot 2019-09-30 at 12 40 52 PM Screen Shot 2019-09-30 at 12 45 05 PM

Kirby Version
3.2.5

Additional context
My expectation with the "Change Template" functionality is that the underlying value stored in the file would stay the same and merely be reinterpreted under the new Blueprint configuration. In other words, the same thing that would happen if simply renamed the content file to foo2.txt from foo1.txt.

For example, if I change any field type (such as checkboxes, tags, select) to simply text, I would expect the resulting text field would contain whatever value was originally saved. The email field illustrates this breaking at a very simple level.

There's probably some discussion that needs to happen in regards to "extreme" changes (such as changing a tags separator), but I think the default should more immediately be "carry over the value as-is" instead of simply discarding it completely.

neildaniels commented 4 years ago

This issue discussion will probably be loosely related to the discussion that occurred with https://github.com/getkirby/kirby/issues/1670. That issue dealt specifically with what happens when a field's options change (which obviously can happen if the entire Blueprint is changing).

distantnative commented 4 years ago

That's not really a bug. When you change to a different template where the same field is not of the same type, we have to discard the values. If they stayed in the content file, we would get problems switching e.g. from tags field to structure field etc.

distantnative commented 4 years ago

I would still love to implement a better change template dialog that visually outlines what is going to happen (which fields are the same, which work, which conflict etc.).

moritzebeling commented 4 years ago

Maybe both. There could be a test, if the content from the old field type is somehow compatible to the new field type. If so, the field value can be kept (or reformatted if needed and possible). If not, the dialog suggested by @distantnative will highlight the changes and possible data loss.

E.g. the values of email, tags, tel, ... fields are compatible to the text field, while structure isn’t.

neildaniels commented 4 years ago

I still mentally treat this Panel operation as the same thing as manually renaming the content file. And in the manual case, after the rename occurs, the Panel inherently must re-interpret unchanged content values with a different Blueprint fields. I think that anything that's truly incompatible would be discarded on save.

I think that's "effectively" how this Panel operation should occur. These are the naive steps I would expect to happen:

  1. User changes template in the Panel
  2. Backend renames the content file
  3. Panel refreshes the page to pick up new Blueprint type
  4. Panel "re-saves" the page (which would then discard truly invalid types)

I really don't think it's worth over-thinking compatibility from Field A to Field B (which would seem kind of impossible with all 3rd party fields).

The fact that the Panel currently does something with manual content changescould be the model for this type of Panel operation.

TL;DR: Why does the Panel "change template" operation need to do much more than just renaming the content-file?

moritzebeling commented 4 years ago

TL;DR: Why does the Panel "change template" operation need to do much more than just renaming the content-file?

Good point. But on the other hand, this could lead to a gap between blueprint rules and actual content. (Close to a discussion we just had here #2163, regarding how existing content should behave after field options have changed). Incoherent structure and content can break page rendering. Since changing template is possibly a public in-production-action, the user should at least be warned.

distantnative commented 4 years ago

Why does the Panel "change template" operation need to do much more than just renaming the content-file?

Cause just renaming the content file and providing incompatible values to a different field will break the Panel (and maybe even frontend). When you rename the file manually, you have to ensure that the values fit the new fields manually. When you do it via the Panel, the Panel has to take care of that.

distantnative commented 3 years ago

What I still see here as actionable items:

Regarding a better dialog: https://kirby.nolt.io/128

distantnative commented 3 years ago

Closing this in favor of https://kirby.nolt.io/128 - please upvote and add to the discussion there. Thanks.