Skip to content

9742: Default welcome message returns after being deleted #9742 #12328

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

Merged
merged 1 commit into from
Nov 28, 2017
Merged

Conversation

RomaKis
Copy link
Contributor

@RomaKis RomaKis commented Nov 17, 2017

Default welcome message returns after being deleted #9742

Description

Column value in core_config_data was nullable. When we saved '' as column was nullable in lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php line 3001 the value became null, so it was null in db. If value is null it will take the default value, so we couldn't save '' and see '' at the form.
Now column value isn't nullable, so when we save '' it will save in db as '' and show at the form as ''.

Fixed Issues (if relevant)

  1. Default welcome message returns after being deleted #9742:Default welcome message returns after being deleted

Manual testing scenarios

  1. Go to Content -> Configuration -> Click Edit on theme being used.
  2. Open header tab -> clear Welcome Text.
  3. Click save and continue.
  4. Open header tab -> Welcome Text field should be empty.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@magento-engcom-team magento-engcom-team added bugfix Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Nov 17, 2017
@sidolov sidolov self-assigned this Nov 23, 2017
@sidolov sidolov added this to the November 2017 milestone Nov 23, 2017
@sidolov
Copy link
Contributor

sidolov commented Nov 23, 2017

Hi @RomaKis, there is no bug with configuration persistence, you can save empty welcome message, field is empty in db (screenshot added).
17607381ee

Issue caused by incorrect form data population (fallback to default value) in \Magento\Theme\Model\Design\Config\DataProvider\MetadataLoader::getData() method.
The same behavior with Copyright field.

On frontend both fields are empty
94a239e4a4

@RomaKis
Copy link
Contributor Author

RomaKis commented Nov 23, 2017

Hi @sidolov, the bug is that they are not empty in backend. When you tried to delete it and save. It will reappear after save.
You are right that it is getting data from \Magento\Theme\Model\Design\Config\DataProvider\MetadataLoader::getData(), but the problem is deeper.
The problem is that if we save (for example) Footer -> Copyright = '' (as empty line) it will be converted as null and saved to db. As Magento see that value is null, so it show the default copyright from app/code/Magento/Theme/etc/config.xml. So it is the same: if Magento has clean installation - it is null or we save it as '' it is null too.

@sidolov
Copy link
Contributor

sidolov commented Nov 24, 2017

@RomaKis after clean Magento installation records about welcome message and copyrights are absent on db. They will be created if saved values were different then default. It's expected result that empty values stored in db as null, doesn't make sense store empty strings, we don't consider this to be a good design. Described problem exists only on backend form, frontend works as expected, so, issue should be fixed in form data population flow (for example system configuration show empty values correctly).

@RomaKis
Copy link
Contributor Author

RomaKis commented Nov 27, 2017

@sidolov, ok, I agree with you. I change code. Please, review it again.

if ($value !== null) {
$fieldData->setValue($value);

if ($value == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use strict comparison

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sidolov, done.


if ($value === null) {
$value = '';

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove blank line at end of control structure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sidolov, done and squashed.

@okorshenko okorshenko merged commit 147f638 into magento:2.2-develop Nov 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Progress: accept Release Line: 2.2 Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants