Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the underscore hyphen debacle #5787

Merged
merged 19 commits into from
Sep 20, 2016

Conversation

rossriley
Copy link
Contributor

@rossriley rossriley commented Sep 19, 2016

OK, so this is an interim fix that's designed to workaround the existing issues with hyphens / underscores without changing too much of the code that drives config itself.

The underlying issue is that currently the main config check doesn't happen until boot, whereas most of the new storage system assumes that the config is already read and processed on register.

In reality there's probably only a small sub-set of functionality that needs to wait until boot but in the current state, nothing is processed in time.

So this PR moves config to a two stage process that hopefully gives us the functionality that we need in most use cases, at the expense of consistency between register and boot states.

Edit to add:

This also adds a soft warning when a user puts a hyphen in their contenttypes definition.

Under the hood it is translated to an underscore, but with the warning that they should update.

screen shot 2016-09-19 at 11 10 05

* @author Ross Riley <riley.ross@gmail.com>
*/

namespace Bolt\Configuration;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There must be one blank line after the namespace declaration

KernelEvents::REQUEST => ['onKernelRequest', 34]
];
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected 1 newline at end of file; 0 found

@GwendolenLynch
Copy link
Contributor

In reality there's probably only a small sub-set of functionality that needs to wait until boot but in the current state, nothing is processed in time.

Just to add context for those reading along … the configuration loading/validation is being replaced in 3.3, and will address this.

The problem currently is the validation & loading needs services that shouldn't be started until boot.

* compiled and validated configuration.
* @package Bolt\Configuration
* @author Ross Riley <riley.ross@gmail.com>
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

G-pick … could you push that down on to of the class declaration and drop the @package

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given it is uncertain the life time of the one, could you also tag it @internal

@bobdenotter
Copy link
Member

Interim fix or not, this makes me quite happy! 👍

@bobdenotter
Copy link
Member

As discussed in the dev-meeting: Merging in for 3.2, so we can test and bugfix before release! See https://github.com/bolt/bolt/wiki/Dev-meeting-2016-09-20

@bobdenotter bobdenotter added this to the Bolt 3.2 - Feature release milestone Sep 20, 2016
@bobdenotter bobdenotter merged commit 5e65343 into bolt:release/3.2 Sep 20, 2016
@dadaxr
Copy link
Contributor

dadaxr commented Sep 28, 2016

This fix is not working on my side :

just tried to add a content type with those 2 fields :

...
image:
            type: image
            attrib: title # Note: retrieve this in your template with {{ record.values.image.title }}
            extensions: [ jpg, png ]
mobile_image:
            type: image
            extensions: [ jpg, png ]

Tried to add a new record of that type using the admin panel and got when saving :

Uncaught Fatal Error:

Details

Doctrine\DBAL\Exception\NotNullConstraintViolationException in AbstractMySQLDriver.php line 112

An exception occurred while executing 'INSERT INTO bolt_sliders (slug, datecreated, datechanged, datepublish, datedepublish, username, ownerid, status, templatefields, title, caption, css_class, position, url, target, image, mobile_image) VALUES(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)' with params ["slug-f25d016fbae3576b0c01e1777e8a0e60", "2016-09-28 09:19:29", "2016-09-28 09:19:29", "2016-09-28 09:19:12", null, "", "1", "published", "[]", "Slider 1", "", "", 0, "#", "_self", "{\"file\":\"2016-09\\\/slider-1.jpg\"}", null]:

SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'mobile_image' cannot be null

Of course the both 'image" and 'mobile_image' was filled in the form ...

@rossriley
Copy link
Contributor Author

Ok, I've tracked this down now. The bug is actually at another part of the codebase, so this PR is still ok.

I'll put in another PR shortly to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants