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

Added SQL commands related to doctrine and remove Doctrine usage #636

Merged
merged 16 commits into from
May 22, 2024

Conversation

M0rgan01
Copy link
Contributor

@M0rgan01 M0rgan01 commented Sep 26, 2023

Questions Answers
Description? Added SQL commands related to doctrine, removing the execution of the command 'doctrine:schema:update'
Type? improvement
BC breaks? no
Deprecations? no
Fixed ticket? Fixes PrestaShop/PrestaShop#33874, Fixes PrestaShop/PrestaShop#18113
Sponsor company -
How to test? Use https://github.com/PrestaShop/SeamlessUpgradeToolbox/tree/main/autoupgrade-scripts for test versions concerned. The SQL dump must be equivalent to a fresh install

@M0rgan01 M0rgan01 force-pushed the without_upgrade_modules branch 3 times, most recently from 6d29334 to 6732dd2 Compare September 26, 2023 15:15
@matks matks changed the title Added SQL commands related to doctrine Added SQL commands related to doctrine and remove Doctrine usage Sep 27, 2023
@Hlavtox
Copy link
Contributor

Hlavtox commented Nov 24, 2023

Guys what about this PR?

@M0rgan01 M0rgan01 self-assigned this Apr 30, 2024
@M0rgan01 M0rgan01 force-pushed the without_upgrade_modules branch from 6732dd2 to 90190a5 Compare April 30, 2024 16:22
@M0rgan01 M0rgan01 added this to the 5.0.2 milestone Apr 30, 2024
@M0rgan01 M0rgan01 force-pushed the without_upgrade_modules branch from a251bdb to ac1f4e8 Compare May 2, 2024 10:08
@M0rgan01 M0rgan01 force-pushed the without_upgrade_modules branch from ac1f4e8 to 9199581 Compare May 13, 2024 14:00
@M0rgan01 M0rgan01 closed this May 13, 2024
@M0rgan01 M0rgan01 reopened this May 13, 2024
@M0rgan01 M0rgan01 force-pushed the without_upgrade_modules branch from 9199581 to ac25f01 Compare May 13, 2024 14:05
@M0rgan01 M0rgan01 marked this pull request as ready for review May 13, 2024 14:06
@@ -312,7 +312,7 @@ ALTER TABLE `PREFIX_connections` CHANGE `http_referer` `http_referer` varchar(25
ALTER TABLE `PREFIX_product_download` CHANGE `display_filename` `display_filename` varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci NULL;

/* Doctrine update happens too late to update the new enabled field, so we preset everything here */
ALTER TABLE `PREFIX_tab` ADD enabled TINYINT(1) NOT NULL;
ALTER TABLE `PREFIX_tab` ADD enabled TINYINT(1) NOT NULL, ADD route_name VARCHAR(256) DEFAULT NULL, CHANGE class_name class_name VARCHAR(64) NOT NULL;
Copy link
Member

Choose a reason for hiding this comment

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

We usually have one change per line, can you please split this one in 3?

Suggested change
ALTER TABLE `PREFIX_tab` ADD enabled TINYINT(1) NOT NULL, ADD route_name VARCHAR(256) DEFAULT NULL, CHANGE class_name class_name VARCHAR(64) NOT NULL;
ALTER TABLE `PREFIX_tab` ADD enabled TINYINT(1) NOT NULL;
ALTER TABLE `PREFIX_tab` ADD route_name VARCHAR(256) DEFAULT NULL;
ALTER TABLE `PREFIX_tab` CHANGE class_name class_name VARCHAR(64) NOT NULL;

DROP INDEX id_shop ON `PREFIX_shop_url`;
DROP INDEX full_shop_url ON `PREFIX_shop_url`;
DROP INDEX full_shop_url_ssl ON `PREFIX_shop_url`;
ALTER TABLE `PREFIX_shop_url` CHANGE id_shop_url id_shop_url INT AUTO_INCREMENT NOT NULL, CHANGE id_shop id_shop INT NOT NULL;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ALTER TABLE `PREFIX_shop_url` CHANGE id_shop_url id_shop_url INT AUTO_INCREMENT NOT NULL, CHANGE id_shop id_shop INT NOT NULL;
ALTER TABLE `PREFIX_shop_url` CHANGE id_shop_url id_shop_url INT AUTO_INCREMENT NOT NULL;
ALTER TABLE `PREFIX_shop_url` CHANGE id_shop id_shop INT NOT NULL;

Comment on lines 244 to 246
ALTER TABLE `PREFIX_feature_flag`
CHANGE label_wording label_wording VARCHAR(512) DEFAULT '' NOT NULL,
CHANGE description_wording description_wording VARCHAR(512) DEFAULT '' NOT NULL;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ALTER TABLE `PREFIX_feature_flag`
CHANGE label_wording label_wording VARCHAR(512) DEFAULT '' NOT NULL,
CHANGE description_wording description_wording VARCHAR(512) DEFAULT '' NOT NULL;
ALTER TABLE `PREFIX_feature_flag` CHANGE label_wording label_wording VARCHAR(512) DEFAULT '' NOT NULL;
ALTER TABLE `PREFIX_feature_flag` CHANGE description_wording description_wording VARCHAR(512) DEFAULT '' NOT NULL;

scopes LONGTEXT NOT NULL COMMENT '(DC2Type:array)',
INDEX IDX_6E064442D8BFF738 (id_authorized_application),
PRIMARY KEY (id_api_access)
) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_unicode_ci` ENGINE = ENGINE_TYPE;
Copy link
Member

Choose a reason for hiding this comment

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

This may lead to the same result, but I saw the syntax was different for the other CREATE TABLE statements.

Suggested change
) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_unicode_ci` ENGINE = ENGINE_TYPE;
) ENGINE=ENGINE_TYPE DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;

description LONGTEXT NOT NULL,
UNIQUE INDEX UNIQ_475B9BA55E237E06 (name),
PRIMARY KEY (id_authorized_application)
) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_unicode_ci` ENGINE = ENGINE_TYPE;
Copy link
Member

Choose a reason for hiding this comment

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

This may lead to the same result, but I saw the syntax was different for the other CREATE TABLE statements.

Suggested change
) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_unicode_ci` ENGINE = ENGINE_TYPE;
) ENGINE=ENGINE_TYPE DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;

@M0rgan01 M0rgan01 force-pushed the without_upgrade_modules branch from ac25f01 to 8fdf343 Compare May 14, 2024 08:03
Quetzacoalt91
Quetzacoalt91 previously approved these changes May 14, 2024
kpodemski
kpodemski previously approved these changes May 17, 2024
Copy link
Contributor

@kpodemski kpodemski left a comment

Choose a reason for hiding this comment

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

Awesome! I still think we could bring back Doctinre at some point, but this change now should save a lot of trouble during the upgrades! well done @M0rgan01

@AureRita AureRita self-assigned this May 17, 2024
@M0rgan01 M0rgan01 dismissed stale reviews from kpodemski and Quetzacoalt91 via 2fbe466 May 20, 2024 12:39
@M0rgan01 M0rgan01 force-pushed the without_upgrade_modules branch from 2fbe466 to 803c6f3 Compare May 20, 2024 12:41
@M0rgan01 M0rgan01 closed this May 21, 2024
@M0rgan01 M0rgan01 reopened this May 21, 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 @M0rgan01

Thank you for your PR, I tested it :

From 1.7.0.1 to :

  • 1.7.1..0
  • 1.7.1.1
  • 1.7.2.0
  • 1.7.3.0
  • 1.7.4.0
  • 1.7.6.0
  • 1.7.7.0
  • 1.7.8.11

From 1.7.5.1 to :

  • 8.0.5
  • 8.1.5

From 8.0.5 to 9.0.0

Because all seems to be corrected as you said, It's QA ✔️

Thank you

@M0rgan01 M0rgan01 merged commit b698ae1 into PrestaShop:dev May 22, 2024
48 of 58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
5 participants