Skip to content

2.4.7-p8 Compatibility / Housekeeping / Merge Changes from PR 20#24

Open
ssx wants to merge 10 commits intoopengento:masterfrom
ssx:master
Open

2.4.7-p8 Compatibility / Housekeeping / Merge Changes from PR 20#24
ssx wants to merge 10 commits intoopengento:masterfrom
ssx:master

Conversation

@ssx
Copy link
Copy Markdown

@ssx ssx commented Dec 31, 2025

#20

@ssx ssx changed the title Merge Changes from PR 20 2.4.7-p8 Compatibility / Merge Changes from PR 20 Dec 31, 2025
@ssx ssx changed the title 2.4.7-p8 Compatibility / Merge Changes from PR 20 2.4.7-p8 Compatibility / Housekeeping / Merge Changes from PR 20 Dec 31, 2025
@ssx
Copy link
Copy Markdown
Author

ssx commented Dec 31, 2025

@thomas-kl1 scripts all working now, just going to do some further testing.

@thomas-kl1 thomas-kl1 self-requested a review December 31, 2025 14:05
@thomas-kl1 thomas-kl1 self-assigned this Dec 31, 2025
Copy link
Copy Markdown
Member

@thomas-kl1 thomas-kl1 left a comment

Choose a reason for hiding this comment

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

Thank you very much @ssx for your contribution and improvments!

I've posted a few comments, and open to discussion and I'm looking forward to merge this PR once they are resolved!

Thank you again! 😃

Comment thread scripts/03_cms.sql
-- 03_cms.sql
--

update cms_page set creation_time = update_time WHERE `creation_time` LIKE '%0000%';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you uppercase SQL keywords please? (UPDATE + SET)

Also what situation could lead to this one? (a short comment on top of the query is fine)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

another that came from the other PR :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IMO we should remove this line

Comment thread scripts/03_cms.sql
Comment on lines +69 to +70
-- ALTER TABLE `magento_banner_catalogrule`
-- DROP FOREIGN KEY `MAGENTO_BANNER_CATRULE_RULE_ID_SEQUENCE_CATRULE_SEQUENCE_VAL`;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we remove this commented query? The table magento_banner_catalogrule is removed in ee.sql

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

or add a short comment on top of it with the reason why

Comment thread scripts/05_salesrule.sql
Comment on lines +116 to +124
-- Amasty related
CALL PROC_DROP_FOREIGN_KEY("amasty_ampromo_rule", "AMASTY_AMPROMO_RULE_SALESRULE_ID_SALESRULE_ROW_ID");
CALL PROC_DROP_FOREIGN_KEY("amasty_amrules_rule", "AMASTY_AMRULES_RULE_SALESRULE_ID_SALESRULE_ROW_ID");
CALL PROC_DROP_FOREIGN_KEY("amasty_amrules_usage_limit", "AMASTY_AMRULES_USAGE_LIMIT_SALESRULE_ID_SALESRULE_ROW_ID");
CALL PROC_DROP_FOREIGN_KEY("amasty_free_gift_timer_timer_data", "AMASTY_FREE_GIFT_TIMER_TIMER_DATA_SALESRULE_ID_SALESRULE_ROW_ID");
CALL PROC_DROP_FOREIGN_KEY("amasty_amrules_usage_counter", "AMASTY_AMRULES_USAGE_COUNTER_SALESRULE_ID_SALESRULE_RULE_ID");
CALL PROC_DROP_FOREIGN_KEY("amasty_banners_lite_banner_data", "AMASTY_BANNERS_LITE_BANNER_DATA_SALESRULE_ID_SALESRULE_RULE_ID");
CALL PROC_DROP_FOREIGN_KEY("amasty_banners_lite_rule", "AMASTY_BANNERS_LITE_RULE_SALESRULE_ID_SALESRULE_ROW_ID");
CALL PROC_DROP_FOREIGN_KEY("amasty_banners_lite_rule", "AMASTY_BANNERS_LITE_RULE_SALESRULE_ID_SALESRULE_RULE_ID");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

anything related to third party module should be moved to a dedicated file so it's up to anyone to run this one or not (ex: 99_amasty.sql)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

your problem there will be that the need running early on so that the products one can run without issue (maybe 00_amasty.sql instead)

Comment thread scripts/05_salesrule.sql
ADD CONSTRAINT `SALESRULE_CUSTOMER_RULE_ID_SALESRULE_RULE_ID` FOREIGN KEY (`rule_id`) REFERENCES `salesrule` (`rule_id`);

--
-- Already done above in line 103.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

isn't it line 111?

We should regroup the operation over the same table in the same group of queries for more clarity

Comment thread scripts/07_product.sql
ADD CONSTRAINT `CAT_PRD_BNDL_SELECTION_OPT_ID_CAT_PRD_BNDL_OPT_OPT_ID` FOREIGN KEY (`option_id`) REFERENCES `catalog_product_bundle_option` (`option_id`) ON DELETE CASCADE ON UPDATE RESTRICT;

ALTER TABLE `catalog_product_bundle_selection_price`
-- DROP INDEX `CATALOG_PRODUCT_BUNDLE_SELECTION_PRICE_WEBSITE_ID`, -- we do not need to remove this index
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

could you add a quick explanation?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we could remove this line IMO

Comment thread scripts/07_product.sql
DELETE c.* FROM `catalog_url_rewrite_product_category` c LEFT JOIN catalog_product_entity p ON c.product_id = p.entity_id WHERE p.entity_id IS NULL;
ALTER TABLE `catalog_url_rewrite_product_category`
DROP FOREIGN KEY `CAT_URL_REWRITE_PRD_CTGR_PRD_ID_SEQUENCE_PRD_SEQUENCE_VAL`,
-- DROP FOREIGN KEY `FK_BB79E64705D7F17FE181F23144528FC8`, -- maybe we have to remove this key instead of the previous one
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

was it related to your tests?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

that came from the original other PR

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It may depends on builds, sometimes the FK are not generated with the name from the declarative schema (or setup schema)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

might be worth wrapping it in the func then, so it can run if it's there, otherwise ignore it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

agreed! but I'm not sure that this key name refer to the same FK (I should browse my projects and check)

Comment thread scripts/07_product.sql
DELETE FROM `cataloginventory_stock_item` WHERE `product_id` NOT IN (SELECT `entity_id` FROM `catalog_product_entity`);
ALTER TABLE `cataloginventory_stock_item`
DROP FOREIGN KEY `CATINV_STOCK_ITEM_PRD_ID_SEQUENCE_PRD_SEQUENCE_VAL`,
-- DROP FOREIGN KEY `CATINV_STOCK_ITEM_STOCK_ID_CATINV_STOCK_STOCK_ID`, -- maybe we have to remove this key instead of the previous one
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

was it related to your tests?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

that came from the original other PR

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, my bad didn't paid attention to it! it's safe to remove this line, we should'nt remove the relation for the stock ID anyway

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@thomas-kl1 no worries :D

@ssx
Copy link
Copy Markdown
Author

ssx commented Dec 31, 2025

@thomas-kl1 I'll pick them up ASAP - most came from the other PR rather than me. Happy to drop those out.

@amarroni
Copy link
Copy Markdown

@thomas-kl1 I'm in the middle of discovery for a downgrade. When you think this merge can be merged?

@thomas-kl1
Copy link
Copy Markdown
Member

@ssx are you still willing to work on these changes?
Depending on @ssx answer, @amarroni you may want to pick some of the job here?

Otherwise I would need to do it myself, but that could mean not before next week

@ssx
Copy link
Copy Markdown
Author

ssx commented Feb 19, 2026

@thomas-kl1 @amarroni I am! this is in my diary tomorrow to continue on!

@ssx
Copy link
Copy Markdown
Author

ssx commented Feb 20, 2026

@ssx are you still willing to work on these changes? Depending on @ssx answer, @amarroni you may want to pick some of the job here?

Otherwise I would need to do it myself, but that could mean not before next week

Hello @thomas-kl1 @amarroni I've been working on https://github.com/DeployEcommerce/magento2-ee-to-community-tool today. Basically, a drop in tool to run through these for you. Currently testing on a 2.4.7 and the DB side appears to be working perfectly. Happy for the changes in the sql/ directory there to be lifted out!

@thomas-kl1
Copy link
Copy Markdown
Member

Hi @ssx
In my opinion the tools would be benefit being unified and not confuse the community.
As this repo benefit a great visibility, would you like to move your changes here and being the maintainer of the repo?
Let me know your thoughts as I don't how we could mirrored the changes regarding the sql scripts in an easy and secure way without manual actions.

BTW Opengento is an association and not a company, so we don't benefit any commercial values here.

@amarroni
Copy link
Copy Markdown

@ssx @thomas-kl1 hi guys how are you?
@ssx I just check the migration tool and looks greate! I was working (with 🤖 ) similar tool. If you want I can add or suggest somthing to be check. Patch List, when you run Setup Registry. I create a a third party extensions analyzer in order to take care of the Commerce extension and how should be the best way to migrate this modules.

I'll testing the tool in a fresh magento open source and fresh magento commercer installation and I'll let you know how was.

@ssx
Copy link
Copy Markdown
Author

ssx commented Feb 23, 2026

Hi @ssx In my opinion the tools would be benefit being unified and not confuse the community. As this repo benefit a great visibility, would you like to move your changes here and being the maintainer of the repo? Let me know your thoughts as I don't how we could mirrored the changes regarding the sql scripts in an easy and secure way without manual actions.

BTW Opengento is an association and not a company, so we don't benefit any commercial values here.

Hi @thomas-kl1! :)

Once I've finished testing and @amarroni too - I'll port all of the SQL changes here for you as well. At some point I'll switch out the sql directory in the tool for this repo and use that instead so everyone benefits 🙂

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.

4 participants