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 backup tool creating nulls #35351

Merged
merged 2 commits into from
Feb 26, 2024
Merged

Fix backup tool creating nulls #35351

merged 2 commits into from
Feb 26, 2024

Conversation

Hlavtox
Copy link
Contributor

@Hlavtox Hlavtox commented Feb 14, 2024

Questions Answers
Branch? 8.1.x
Description? This PR fixes an issue when the prestashop backup was adding NULLs to columns, where there is an empty string. If you restore this database, you get errors in admin. More info below.
Type? bug fix
Category? CO
BC breaks? no
Deprecations? no
How to test? Create backup in prestashop backoffice, restore it in phpMyAdmin, see that backoffice product page works normally. :-)
UI Tests
Fixed issue or discussion? Fixes #33818
Related PRs
Sponsor company

Description

  • We have some issues when there are NULL values in database - Change default NULL value #34454. They can get there multiple ways. One of the ways is the builtin backup tool that adds NULLs to columns, where there is an empty string.
  • The original logic was following. It put the value into pSQL method. The method converts NULLs to empty string. So, it checked if the column is NULLable in the database and added NULL or empty string. This is bad, because it will make every empty string a NULL. :-)
  • So, fix was simple. Check if the column is NULL before running it through pSQL method.

@Hlavtox Hlavtox requested a review from a team as a code owner February 14, 2024 10:37
@Hlavtox Hlavtox added this to the 8.1.4 milestone Feb 14, 2024
@prestonBot prestonBot added 8.1.x Branch Bug fix Type: Bug fix labels Feb 14, 2024
@PrestaShop PrestaShop deleted a comment from prestonBot Feb 14, 2024
@Hlavtox Hlavtox mentioned this pull request Feb 14, 2024
2 tasks
@Hlavtox Hlavtox mentioned this pull request Feb 25, 2024
26 tasks
@Hlavtox Hlavtox changed the base branch from develop to 8.1.x February 25, 2024 17:28
@Hlavtox Hlavtox changed the base branch from 8.1.x to develop February 25, 2024 17:28
@Hlavtox Hlavtox changed the base branch from develop to 8.1.x February 25, 2024 17:30
@kpodemski kpodemski closed this Feb 25, 2024
@kpodemski kpodemski reopened this Feb 25, 2024
@kpodemski kpodemski added the Waiting for QA Status: action required, waiting for test feedback label Feb 25, 2024
@AureRita AureRita self-assigned this Feb 26, 2024
Copy link
Contributor

@AureRita AureRita left a comment

Choose a reason for hiding this comment

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

Hi @Hlavtox

Thank you for your PR, I tested it and it seems to works as you can see :

recording.116.webm

Link to the auto test : https://github.com/AureRita/testing_pr/actions/runs/8051889112

Because the auto test is 🟢 and the PR seems to works as expected, It's QA ✔️

Thank you

@AureRita AureRita added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Feb 26, 2024
@Hlavtox
Copy link
Contributor Author

Hlavtox commented Feb 26, 2024

Cool!!! Thank you @AureRita!!

@Hlavtox Hlavtox merged commit 9f01802 into PrestaShop:8.1.x Feb 26, 2024
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.1.x Branch Bug fix Type: Bug fix QA ✔️ Status: check done, code approved
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Backup changes data
5 participants