search

pimcore / ecommerce-framework-bundle

Ecommerce Framework community bundle provides e-commerce functionality such as product listing and filtering, pricing, carts and checkouts for Pimcore.
https://pimcore.com/docs/platform/Ecommerce_Framework/
Other
8 stars 31 forks source link

Standardize Column Naming in EcommerceFramework Product Index Table #160

Closed rliebi closed 6 months ago

rliebi commented 7 months ago

Standardize Column Naming in EcommerceFramework Product Index Table

See Issue #159

This pull request introduces a migration that standardizes the column naming conventions in the ecommerceframeworkproductindex table by removing the 'o' prefix from its columns. This change aims to enhance readability and maintain consistency across the database schema.

Key Changes:

Migration Implementation: A new migration (Version20240215093348) has been added to handle the renaming of columns by removing the 'o' prefix. This includes both up() and down() methods to ensure that changes can be rolled back if necessary. Column Renaming Logic: The migration iterates over specified columns with the 'o' prefix, checks for their existence, and renames them accordingly. A safeguard is in place to handle potential non-existing columns gracefully, logging the attempt without causing errors. Down Migration: To ensure reversibility, the down migration logic reinstates the 'o_' prefix to the column names, should a rollback be required.

Motivation:

This update is part of an ongoing effort to improve the codebase's consistency and adherence to best practices. By standardizing column names, we aim to improve developer experience and facilitate easier maintenance of the database schema.

Testing:

Comprehensive testing has been conducted to ensure that the migration executes as expected in both directions (up and down). Additional tests have been added to verify that the application's functionality remains unaffected by the schema changes.

rliebi commented 6 months ago

rebased to 1.0

Corepex commented 6 months ago

Hi @rliebi,

thanks for creating this PR. It is absolutely awesome that you help us with the housekeeping stuff :partying_face:

Afaik, this is not enough to get rid of the o_ prefix since we potentially have multiple index tables (dependent on the tenant, used product technology, etc.). The ecommerceframework_productindex table is only present in one of the simplest use cases.

That also means that we talk of a BC-Break if we would really adjust these settings ...

rliebi commented 6 months ago

Hey @Corepex Thanks for your reply. We had this issue in our environment so I thought I'd fix it for you because we could not deploy without this, so I thought I'd share.. As this is more than a month in the past I cannot provide you more details without having another look into it, which unfortunately, rn I don't have the time for :-(

Corepex commented 6 months ago

@rliebi, thanks for your PR. Sadly, I can't merge that right now because there are too many open questions. This would create more than one BC break and, therefore, require a major version.