search

getkirby / ideas

This is the backlog of ideas and feature requests from the last two years. Use our new feedback platform to post your new ideas or vote on existing ideas.
https://feedback.getkirby.com
20 stars 0 forks source link

Content: remove outdated fields #427

Open hdodov opened 5 years ago

hdodov commented 5 years ago

We just had a confusion in a website we're making because Kirby doesn't delete fields that no longer appear in the blueprint. Here's how it happened:

  1. Blueprint product had field packagingImage
  2. We added product A and product B and set their packagingImage
  3. In the template, we used $page->packagingImage() to get the values
  4. We refactored the blueprint and changed packagingImage to package
  5. Products A and B were updated to reflect the change and package was set on them
  6. Product C was added and it had package set as well

The final result was:

However, in our template, we had forgotten to change $page->packagingImage() to $page->package() and figuring that out was a bit of a hassle.

We didn't look at the template because the first two products were working, which lead us to think that product C was problematic. Then, we looked at the blueprint (which had package in it) and product C's TXT file (which also had package) and that left us confused. Everything seemed like it was set OK and it actually was, but we were misled by products A and B which were showing up correctly. They shouldn't have showed because their templates used old values that are no longer in the blueprint.

Proposal

Whenever a model is saved, fields that don't appear in that model's blueprint should be removed so that the TXT is a correct reflection of the blueprint. If you have a blueprint for a house and you're building said house, you wouldn't add a random room somewhere unexpectedly. That's what the blueprint is for.

If packagingImage was removed from products A and B in step 5, none of the products would have had their images shown. This would have immediately told us that there was something wrong with the blueprint, greatly reducing the time spent debugging.

texnixe commented 5 years ago

Just some thoughts:

Imagine you don't have a simple image selector but a field that contains quite a bit of content. You rename the field in the blueprint but forget to rename your content field in all existing pages. If you accidentally save a page in the Panel, your content will be gone. One way or the other you will always run into issues when renaming fields and your workflow should probably contain measures to prevent issues in such cases in any case. Either cleaning up your content files after such changes, or making sure that your blueprint changes are reflected in your templates/controllers/snippets etc.

Plus, there are probably people who don't declare some fields in blueprints because they don't want them to pop up in the Panel. The hidden field is not always useful in such cases.

hdodov commented 5 years ago

Imagine you don't have a simple image selector but a field that contains quite a bit of content

Imagine you have that large piece of content in two fields, in every page, for no reason. Later if you rename the field again, you'd have it in three places.

One way or the other you will always run into issues when renaming fields

I agree with that, but the point is to make Kirby help you with finding out those issues.

Plus, there are probably people who don't declare some fields in blueprints because they don't want them to pop up in the Panel. The hidden field is not always useful in such cases.

I agree with that. But still, those fields should be mentioned somehow in the blueprint. When a new member joins a project, he should be able to look at a model's blueprint and know what to expect in that model's data file. That's what the word "blueprint" suggests.

there are probably people who don't declare some fields in blueprints because they don't want them to pop up in the Panel

Sound like a pretty good use case for hidden fields:

The hidden field makes it possible to keep fields in content files without actually making them editable or even visible in Panel forms.

But the same is true for fields that are not declared in the blueprint at all. The hidden field doesn't "make it possible to keep fields in content files" because you could already do that. This renders hidden fields pointless, I believe.

hidden field is not always useful in such cases

What are those cases? What are the issues? Perhaps there are better solutions?

I already have a suggestion - blueprints could have a strict option that defaults to true and deletes fields that are not in the blueprint. Setting this option to false allows any fields to be stored, giving you the current behavior. This is valuable because reading the blueprint, you'll know that there's probably something not mentioned in the blueprint and you can expect it.

Back to my example in the beginning - imagine we didn't create product C and we never noticed the problem. Site goes live, devs go on vacation, client adds content, site doesn't work, client isn't happy, devs are shocked because it "worked." Doesn't sound good, right?

I see two solutions:

  1. Delete fields that are missing in the blueprint and:

    If you accidentally save a page in the Panel, your content will be gone.

    To avoid such issues, Kirby could display a warning that some fields will be deleted.

  2. Leave fields in the content file as-is but always return null when a missing field is accessed in the blueprint, even if it has a value. This is best of both worlds because no content is deleted, yet you immediately see the breaking change in the template. However, parsing blueprints on each request could be pretty costly and not a good idea. For this reason, those blueprint checks could be made in debug mode only.

I think the second solution is better because even if missing fields are deleted, that happens on save. If you simply don't save a page, you'd still miss the error. For that reason, I think the second solution is better because the live website would remain flexible and would behave the same way is it always did. However, when devs are working with the debug option, they would immediately see errors and can fix them up. If they want to allow a missing field - they use either a hidden field or the strict option.

Edit: Actually, implementing both solutions would be the best way. If devs have fully described the behavior of content fields via solution 2 and the site is running without error, that would mean Kirby can know which fields are being used and which ones aren't. When the user adds content, Kirby would delete those fields on save and warn about that to avoid unnecessarily polluting the content file. To quote myself:

Imagine you have that large piece of content in two fields, in every page, for no reason.

So basically:

  1. Blueprints utilize hidden fields and the strict option
  2. When debug is enabled, any fields out of the blueprint scope would throw errors
  3. Saving a model with outdated fields would warn about their deletion and perhaps give you an option to assign an outdated field's value to another field that is valid

Result:

GiantCrocodile commented 5 years ago

A legal reason against the current behavior: You will never know what kind of information had been stored in the past without checking every single content file. This is impossible for non-techy people and makes it hard to find out if someone stored sensitive (by an incident like a copy and paste fail or because it was needed in the past) and this leads to an unreasonable effort to watch out for information which is protected by law (waves to Mr. GDPR - who asks you to prune/fix unnecessary, incorrect or outdated information).

My idea for a solution: I would like to have a warning if I open a page: Show me a dialog which shows me outdated/renamed fields and I can approve the renaming (so the field name in the content file and the blueprint matches again), discard the old data or choose to perform no action (this should be saved somewhere so the dialog comes just once if the file doesn't change again). This will prevent a data loss by a human mistake and improves the quality of the files.

What do you think?