8000 Strengthten upgrade process by Hlavtox · Pull Request #853 · PrestaShop/autoupgrade · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Strengthten upgrade process #853

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Sep 19, 2024
Merged

Strengthten upgrade process #853

merged 5 commits into from
Sep 19, 2024

Conversation

Hlavtox
Copy link
Contributor
@Hlavtox Hlavtox commented Aug 22, 2024 •
edited
Loading
Questions Answers
Description? This should reduce the amount of potential failure points for upgrade. It creates tables only if they don't exist. It adds columns to tables only if they don't exist. If they do, they are updated. Common case when people backport some feature from newer versions, or upgrade failed mid-point somehow.
Type? bug fix
BC breaks? no
Deprecations? no
Fixed ticket?
Sponsor company TRENDO s.r.o.
How to test? Tests green.

What I changed

  • New script add_column that adds a column if it doesn't exist OR modifies it, if it already does.
  • I made sure that all ALTER TABLE ADD are using this script.
  • I checked that all CREATE TABLE are conditioned with IF EXISTS.
  • I checked that all DROP TABLE are conditioned with IF EXISTS.
  • I checked that all DROP COLUMN are using our script drop_column_if_exists.

What it improves

  • When the upgrade accidentaly runs twice, @kpodemski somehow reproduced it.
  • When people backport some features from newer versions and already have some extra column. Or when some module added it.
  • (Maybe) When an upgrade failed and people just restored the files.

Ping @Quetzacoalt91

@Hlavtox Hlavtox added this to the 6.1.0 milestone Aug 22, 2024
@Hlavtox Hlavtox force-pushed the strengthten-upgrade branch 2 times, most recently from 57d763c to a57a339 Compare August 22, 2024 07:30
matks
matks reviewed Aug 22, 2024
View reviewed changes
Copy link
Contributor
@matks matks left a comment

Choose a reason for hiding this comment

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

Looks good and a safe way to do it 👍

Wdyt @M0rgan01 @Quetzacoalt91 @ga-devfront ?

@Hlavtox
Copy link
Contributor Author
Hlavtox commented Aug 22, 2024

Thanks. :-))) I extended the PR description.

@M0rgan01
Copy link
Contributor

@Hlavtox > When the upgrade accidentaly runs twice, @kpodemski somehow reproduced it.

How could this happen ?

@Hlavtox
Copy link
Contributor Author
Hlavtox commented Aug 27, 2024

@M0rgan01 No idea, ask @kpodemski, but he won't know, just happened 🤣

M0rgan01
M0rgan01 reviewed Aug 27, 2024
View reviewed changes
@Quetzacoalt91 Quetzacoalt91 removed this from the 6.1.0 milestone Sep 3, 2024
@M0rgan01 M0rgan01 added Blocked Status: The issue is blocked by another task and removed waiting for author labels Sep 5, 2024
@M0rgan01
Copy link
Contributor
M0rgan01 commented Sep 5, 2024

waiting for #874 merge

@Hlavtox Hlavtox force-pushed the strengthten-upgrade branch from 219de49 to c8b38b6 Compare September 5, 2024 15:11
@Hlavtox
Copy link
Contributor Author
Hlavtox commented Sep 5, 2024

@M0rgan01 @Quetzacoalt91 Rebased, let's see what tests say :-)

@M0rgan01 M0rgan01 removed the Blocked Status: The issue is blocked by another task label Sep 5, 2024
@Hlavtox
Copy link
Contributor Author
Hlavtox commented Sep 5, 2024

Yaaaaay 🥳

@Quetzacoalt91 Quetzacoalt91 added the enhancement Type: Improvement label Sep 17, 2024
AureRita
AureRita approved these changes Sep 19, 2024
View reviewed changes
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.294.webm

Tested with UI from :
1.7.7.8 to 8.1.7
1.7.7.8 to 1.7.8.11
8.0.5 to 8.1.7
8.1.7 to 9.0.0
8.0.5 to 9.0.0

Tested with SeamlessUpgradeToolbox :
1.7.0.1 to 1.7.2.0
1.7.0.1 to 1.7.3.0
1.7.5.1 to 1.7.6.0
1.7.5.1 to 1.7.6.6
1.7.5.1 to 1.7.7.0
1.7.5.1 to 1.7.8.0
1.7.5.1 to 8.0.0
1.7.5.1 to 8.1.0
8.0.4 to 9.0.0

With this test I see this part that disapear with the upgrade :

image

That why i tested to put a note with an order. I think it disapear because we don't put often a note on order at the installation. That's why I tested it on the video

Because the PR seems to works as expected, It's QA ✔️

Thank you

@Hlavtox Hlavtox merged commit 5e2cee1 into PrestaShop:dev Sep 19, 2024
34 checks passed
@Hlavtox Hlavtox added this to the 7.0.0 milestone Sep 19, 2024
@Hlavtox Hlavtox mentioned this pull request Oct 23, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matks matks matks left review comments

@M0rgan01 M0rgan01 M0rgan01 approved these changes

@ga-devfront ga-devfront ga-devfront approved these changes

@AureRita AureRita AureRita approved these changes

Assignees

@Hlavtox Hlavtox

Labels
enhancement Type: Improvement QA ✔️
Projects
PR Dashboard
Archived in project
Milestone
7.0.0
Development

Successfully merging this pull request may close these issues.
7 participants
@Hlavtox @M0rgan01 @matks @ga-devfront @AureRita @Quetzacoalt91 @ps-jarvis
0