Skip to content

Conversation

@boherm
Copy link
Member

@boherm boherm commented Dec 18, 2025

Questions Answers
Description? Initiate new discount types table in 9.1.0
Type? improvement
BC breaks? no
Deprecations? no
Fixed ticket? ~
Sponsor company PrestaShop SA
How to test? Upgrade into 9.1.0 version

jolelievre
jolelievre previously approved these changes Dec 18, 2025
Copy link
Contributor

@jolelievre jolelievre left a comment

Choose a reason for hiding this comment

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

Thanks @boherm

jolelievre
jolelievre previously approved these changes Dec 18, 2025
Copy link
Contributor

@Hlavtox Hlavtox left a comment

Choose a reason for hiding this comment

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

Two things:

  1. These translations won't be discoverable, are you sure they will be scannned?

  2. I see cart rule compatibility that only resolves compatibility types, this is not enough and it was promised that this will be resolved no?

tleon
tleon previously approved these changes Dec 18, 2025
@boherm
Copy link
Member Author

boherm commented Dec 18, 2025

Hi @Hlavtox,

  1. We have create a CartRuleTypeLang.php in classes/lang/KeysReference (like we do for others things) to discover some translations. (as you see here, the translation tool can discover this new strings)

  2. I can't answer to this, but maybe @ibahloul-ps or @jolelievre could 😜

@jolelievre jolelievre dismissed Hlavtox’s stale review December 18, 2025 13:32
  1. yep don't worry these wordings have been correctly scanned
  2. this question is not relevant with the upgrade PR, the new system is indeed based on incompatibility based on types, the purpose is to find alternatives to the current system that creates a compatibility table that is exponentially getting bigger Maybe it's not enough, but that's what the experimental phase is made for and that's why we need the comunity to test this new discount feature
@boherm boherm dismissed stale reviews from tleon and jolelievre via f8680c6 December 18, 2025 13:34
@boherm boherm force-pushed the new-discount-system branch 2 times, most recently from f8680c6 to 23ba4ab Compare December 18, 2025 13:35
jolelievre
jolelievre previously approved these changes Dec 18, 2025
@Hlavtox
Copy link
Contributor

Hlavtox commented Dec 18, 2025

@jolelievre Yeah, but by pushing this to 9.1 you create a BC break contract that you cannot take back.

Maybe it's not enough, but that's what the experimental phase is made for and that's why we need the comunity to test this new discount feature

So you are implementing something, completely breaking up the discount system and you don't know if it's gonna work? Is this for real?

@Quetzacoalt91
Copy link
Member

@boherm what's the related PR on PrestaShop/PrestaShop repository?

@jolelievre
Copy link
Contributor

@Hlavtox stop the drama! This is fully retro compatible with the current system
How is it a breaking change?

@Hlavtox
Copy link
Contributor

Hlavtox commented Jan 2, 2026

@jolelievre It's not, in the current system, you define cart rule compatibility with specific cart rules.

In your new "improved" system (a.k.a we removed some functionality), it's only defined by cart rule types.

If you push it now, you cannot take it back, if you don't want to piss developers who already started preparing solutions and modules for it.

And then you openly admit: Maybe it’s not enough, but that’s what the experimental phase is for and why we need the community to test this new discount feature. I thought there was a huge amount of exploration behind this with highly esteemed French merchants?

What kind of design is this, where discount compatibility is defined only by the rule type? So a discount is either incompatible with everything, or compatible with everything? What kind of nonsense is that?

@jolelievre
Copy link
Contributor

The new system is behind a feature flag, it doesn't prevent the current system from working as long as you don't enable the new feature The purpose is indeed to test things and see if it meets the needs, if it doesn't the feature can still evolve And if we realize the cart rule types are a bad iea we can still remove them from the DB, since they are related to an experimental feature it's still possible without a BC

@AureRita AureRita self-assigned this Jan 6, 2026
@boherm boherm force-pushed the new-discount-system branch 3 times, most recently from 1bfab5d to 7ea3f8a Compare January 7, 2026 14:10
@boherm boherm force-pushed the new-discount-system branch from 7ea3f8a to fd02b6b Compare January 7, 2026 14:14
jolelievre
jolelievre previously approved these changes Jan 7, 2026
mattgoud
mattgoud previously approved these changes Jan 7, 2026
@boherm boherm dismissed stale reviews from mattgoud and jolelievre via 8880248 January 7, 2026 14:52
@boherm boherm force-pushed the new-discount-system branch 2 times, most recently from 8880248 to a1b51f8 Compare January 7, 2026 15:36
@boherm boherm force-pushed the new-discount-system branch from a1b51f8 to c2c2392 Compare January 7, 2026 15:43
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 7, 2026

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 @boherm

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

Capture.video.du.2026-01-07.18-07-30.mp4

Tested from :
8.1.4 to 9.1.0
9.0.2 to 9.1.0

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

Waiting for CLI green to remove the waiting QA

Thank you

@Quetzacoalt91 Quetzacoalt91 added the Blocked Status: The issue is blocked by another task label Jan 8, 2026
@boherm
Copy link
Member Author

boherm commented Jan 8, 2026

@Quetzacoalt91 Quetzacoalt91 removed the Blocked Status: The issue is blocked by another task label Jan 8, 2026
@Quetzacoalt91 Quetzacoalt91 added this to the 7.6.0 milestone Jan 8, 2026
@Quetzacoalt91 Quetzacoalt91 merged commit 0872ccf into PrestaShop:dev Jan 8, 2026
56 checks passed
@github-project-automation github-project-automation bot moved this from Ready for review to Merged in PR Dashboard Jan 8, 2026
@boherm boherm deleted the new-discount-system branch January 9, 2026 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Merged

Development

Successfully merging this pull request may close these issues.

8 participants