2.5.3/2.5.4 breaking structure field if no default value is set in blueprint

[medienbaecker]

Updating to 2.5.3 or 2.5.4 breaks any blueprint containing a structure field without default values.

test:
  label:       Structure field test
  type:        structure
  fields:
    text:
      label:   Text
      type:    textarea

[texnixe]

The problem seems to be the re-implementation of the default values in /panel/app/fields/structure.php. Undoing the changes made to that file solves the issue.

[ralfgoeke]

breaks for me even with following basic structure with no grouping at all (2.5.4)

bugtest:
  label: test
  type: structure
    fields:
      testfield:
        label: foo
        type: text

Shows Invalid argument supplied for foreach()in panel

[texnixe]

@ralfgoeke Yes, it has nothing to do with the grouping feature, it affects all structure fields if a default value is not set.

[medienbaecker]

You're right @ralfgoeke. I didn't even test a regular structure field without grouping.

I've edited the issue.

[lukasbestle]

Arghh, it turns out the Panel form class sets the default param to null if the blueprint doesn't define it. And that overrides my class default of array().

This is now fixed on the develop branch (I hope 😉).

[texnixe]

Yes, the issue is gone, but it doesn't add any default values (at least not in my tests). What is $this->value() === $this->default() supposed to do? The defaults are supposed to be added if there is no value yet, aren't they?

[lukasbestle]

$this->value() === $this->default() makes sure that deliberately empty fields don't get overwritten with the defaults again.

So to test this, please ensure that the field does not exist at all in your content file (even if it is empty there).

[texnixe]

Well, I tested by creating a new page, so there was no field prior to that. Of course, if I remove the field after having created the page, the default values do appear, but that kind of defeats the purpose. Maybe that is the reason why the feature was removed in the first place. Because if you don't use this check, you can't remove the default values anymore.

[olach]

Are there plans to ship v2.5.5 with this fix soon? v2.5.3/2.5.4 has been out for almost a week with this fatal bug.

[medienbaecker]

Anyone testing Kirby right now will have a very frustrating first impression with this version. Most of my websites have a structure or builder field.

If it's not possible to fix the "default values" functionality, it should just be removed from the code and documentation in my opinion.

[wottpal]

Thats true. I can't update any of my sites.

[texnixe]

BTW. Has anyone tested if the fix works as it should? That is, not only removes the bug, but also implements the desired functionality? Because in my tests, it did not. If not, we should really just remove that feature, i.e. undo the changes from 2.5.4

[medienbaecker]

I just tested the fix and it seems to work as intended. Adding the example structure field from the docs on a page will add the default values. What did not work in your test, @texnixe?

emails: 
    label: Emails
    type: structure
    default: 
      - email: bastian@getkirby.com
      - email: sascha@getkirby.com
      - email: nico@getkirby.com
    fields: 
      email: 
        label: Email
        type: email

[texnixe]

If I create a new page in the Panel with a blueprint that contains the structure field from the docs, no default entries are created. It only works for me if I create a page with an empty text file (or a page that does not contain the structure field) outside of the Panel and then open it in the Panel.

[medienbaecker]

Alright, thanks for your explanation.

So it seems like the default values are not added because Kirby adds an empty email: (or whatever the structure field name is) to the content file when you add a new page. Once removing this from the content file, the default values are added.

Other fields seem to check if there is a default value in the blueprint when creating a page. I tried searching for default in the Panel core, but the only thing I found was this. Not sure if this has anything to do with it, though.

[lukasbestle]

It looks like we will need a different check.
The default value implementation of other fields does work fine, but I'm also still trying to figure out where in the Panel code this behavior is handled. I will look into this again for the very next release. :)

[olach]

Would it be best to pull the 2.5.3/2.5.4 version until this bug is fixed? A lot of people will be affected by this bug. New users trying out Kirby for the first time will get a broken Kirby.

It's a nasty bug too. I updated a client site. Everything seemed to be working locally. But after a day or two the client said that they couldn't edit new pages. I found this issue and could manually patch the file so the error doesn't show up for them.

If I had the time I could help find the root cause for this issue, but currently I'm too busy with client work.

[texnixe]

We'd definitely be better off with a working field, even if default values don't work. And we should ship this as soon as possible.

[jenstornell]

I agree.

Back to the point where it was working. Else there will probably be a drop in the Kirby sales, users that will never come back because of a poor first impression of the software. This breaking bug has been here for half a month already. I hope that no new users have tried out the structure field during this time.

As much fun Kirby 3 may be to work on, I think Kirby 2 deserve some love as well. Just keep it stable.

[bastianallgeier]

I totally agree with all this! @lukasbestle what do you think does it take to fix and ship this?

[olach]

As much as i love working with Kirby, the experience with all 2.5.x releases has been not so good with several major bugs introduced. From now on, I will not live on the edge by installing the latest version when they are available anymore. And yes, I do work and test locally before I deploy to a live server, but these bugs where not discovered locally.

To the Kirby developers: I admire your hard work with Kirby and it's the best CMS I have ever used! This is not a criticism of you. Bugs will always exists. We must just find a way to find them before we deploy an update to our clients live server. Maybe start with beta/developer versions?

[bastianallgeier]

@olach I totally understand your position and I know how much this all sucks and I'm very sorry for it. We just discussed that @lukasbestle is going to look into the remaining issues this weekend and we will publish a RC for the next version on Monday and make sure that it will be stable together with all of you before we ship it again during next week.

We won't ever release versions again without proper betas and release candidates. That's a promise!

[Sija]

@bastianallgeier and tests maybe? since your code ATM IS the bleeding edge, no matter how much RCs you'll publish before release...

[lukasbestle]

@Sija Kirby 3 is currently being built from the ground up and will have a lot more tests (not only unit tests, but also integration tests). :)

[Sija]

@lukasbestle better late then never 👍 too bad that @bastianallgeier (as the Kirby creator and lead developer), is such reluctant in responding to any form of criticism addressed to him. I had an idea once of using Kirby at wider scale for many of my and my friends' projects, yet after observing how he manages this project I'm pretty much disappointed in regards to outside communication (with paying customers!), code quality and support in general. Too bad, since there's many interesting and useful ideas floating around here but in the current state of affairs it's more like a mickey-mouse business than sth you could put trust in (I even stopped counting on how many occasions you shipped production release with critical bugs - sometimes taking days, or more to fix 'em, c'mon!). I've seen many opensource projects maintained by ppl with way better attitude than one presented here, and I'll underscore that we're talking here about FREE, opensource projects and not paid, licensed products, which IMO should get BETTER support, not worse...

[wottpal]

I'm totally with you @Sija when it comes to your criticism about especially this bug. It could have been fixed weeks ago or at least be rolled-back. On the other side I've also seen the Kirby team fixing issues way faster in the past. Often even faster than I update my sites.

But when it comes to communication of the team I can't disagree more with you. I've always got unbelievably fast and constructive replies - in the forum, via mail and on github. And in the forum I've never seen Bastian rejecting any ideas nor criticism without explaining himself very well. I can't deny not everybody shares his vision of a slim and focused core of Kirby but I honestly think everybody benefits from that in the long run. Every addition needs to get maintained and makes Kirby more error-prone. (And this is why we are here, right?)

[bastianallgeier]

Wow, I'm quite shocked to read @Sija's comments. I actually don't even remember what you are referring to and I'm sorry if I'm missing something here. Kirby has always been a project I take super seriously. I agree that I sometimes don't manage to reply to everything as fast as it should be and there are many unanswered suggestions here on Github — I'm aware of that. What you might underestimate though is the sheer amount of messages, emails and feedback we get on multiple channels and the effort it takes to deal with it. We are also not a company with lots of employees and support staff. In fact we've all been working part-time on Kirby until I started full time this year and except me, the rest of the team is only hired for a very small amount of hours per months. We don't even add up to two full time employees in total at the moment. Plus, we always have to balance support with actual development, marketing, documentation and all the other Mickey Mouse business stuff. We are also not an enterprise-level business. Our pro licenses cost 79€, not 999€. You can do some simple math around this quite quickly. I often spend more than half an hour on answering most pro support requests, logging into customer servers, debugging stuff or simply replying to long complicated questions. Next to me Sonja is spending all her time on the forum even replying to people who are not customers at all, or just own a personal license. Hell, I even have to deal with people regularly sending me pro support requests with just a personal license. We still do all this because we really love it and we don't raise our license prices or add some anti-piracy protection, because we believe in a good mixture between running a healthy company and being open and transparent and helpful.

That's why we still try to do our best to take in as much feedback as possible and I think if you have a look at our changelog, most of the features in there are community suggestions.

Regarding this bug and version 2.5 I agree 100% though and I even said that before here and apologized for it. I also take full responsibility for this. I just focused too much on Kirby 3 and left all update work to @lukasbestle. I know it's frustrating and I promise we will fix it and do better in the future. We have many customers trusting us for years and we are running this long enough to not simply throw that earned trust away.

There's a history behind this system and it sometimes shows. You probably all know the situation when you finish a client project and you learned so much in the meantime that you'd love to start from scratch and make it all cleaner and better. That's how I feel with Kirby all the time. That's why we are taking so much effort to take a better next step with future releases for Kirby 2 and all the work that goes into version 3. I think all our supporters who are following the Kirby next project can see that on a daily basis. Running and developing a content management system for more than five years is a learning process as with everything. We make mistakes to learn from them.

@Sija I'd still love to hear your original criticism and I'm again very sorry that I don't remember what it was.

[lukasbestle]

After a lot of debugging, I might have found a proper solution for this issue.
Please note that the error for structure fields without default value has already been fixed on the develop branch 17 days ago, but today I have tried to make the default values actually work again.

Please test the current version on the develop branch (not only with structure fields, but also in general) and let me know if the changes work for you. As I said, the default values for structure fields should now work fine as well.

[medienbaecker]

Default values for structure fields work perfectly now. Thanks a lot Lukas!

[olach]

I tested here and it seems to be working without problems. Good job!

[jmcguerreiro]

Was having this issue as well, thank you very much for the fix!