Coupon Code Case Sensitive

[ThuktenSingye]

Summary

This pull request introduces configurable coupon code normalization to Solidus Promotions.
It allows to customize how coupon codes are normalized before saving, lookup, or application, enabling behaviors such as case-insensitive, case-sensitive, or other custom normalization.

By default, coupon codes remain case-insensitive, so promo code like SAVE20, save20, or Save20 will all apply the promotion successfully. Using a custom normalizer or overriding the default class, you can implement strict case-sensitive behavior or any other normalization logic required by your application.

Key changes

• Added a new configurable normalization class SolidusPromotions::CouponCodeNormalizer .
• Updated promotion handlers and models to delegate all coupon code normalization to the configured normalizer class.
• Added detailed documentation:
  • README section describing default behavior, customization options, and examples.
  • YARD documentation for CouponCodeNormalizer and the configuration preference.
• Clarified behavior for code normalization, lookups, and promotion application.
• Updated specs to reflect the new configurable normalizer, including tests for custom behavior.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

• I agree that my PR will be published under the same license as Solidus.
• I have written a thorough PR description.
• I have kept my commits small and atomic.
• I have localized any and all user-facing strings that I added to the source code.
• I have used clear, explanatory commit messages.

The following are not always needed:

• 📖 I have updated the README to account for my changes.
• 📑 I have documented new code with YARD.
• ✅ I have added automated tests to cover my changes.

[Copilot]

Pull Request Overview

This pull request introduces configurable case sensitivity for coupon codes in Solidus Promotions. By default, coupon codes remain case-insensitive (maintaining backward compatibility), but administrators can now enable strict case-sensitive matching where "SAVE20" and "save20" are treated as distinct codes.

• Added a new configuration preference preferred_coupon_code_case_sensitive (defaults to false)
• Implemented case sensitivity logic across promotion models, handlers, and order processing
• Added comprehensive test coverage for both case-sensitive and case-insensitive scenarios

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
promotions/lib/solidus_promotions/configuration.rb	Adds the new boolean configuration preference with detailed documentation
promotions/app/models/concerns/solidus_promotions/coupon_code_case_sensitivity.rb	New concern providing shared methods for checking case sensitivity configuration
promotions/app/models/solidus_promotions/promotion.rb	Updates coupon code lookup to respect case sensitivity setting
promotions/app/models/solidus_promotions/promotion_code.rb	Modifies code normalization to preserve case when sensitivity is enabled
promotions/app/models/solidus_promotions/promotion_handler/coupon.rb	Updates coupon handler to conditionally downcase codes based on configuration
promotions/app/patches/models/solidus_promotions/order_patch.rb	Includes case sensitivity concern in order model
core/app/models/spree/order.rb	Updates coupon code assignment to respect case sensitivity
promotions/README.md	Adds comprehensive documentation explaining the feature and its implications
Multiple test files	Extensive test coverage for case-sensitive and case-insensitive scenarios

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[kennyadsl]

@ThuktenSingye Thanks! Copilot has found some small issues that I think we can easily fix.

Overall, the change seems very good to me, but I'd like to get @mamhoff opinion as well, in case I'm missing something.

[ThuktenSingye]

@kennyadsl Thank you for the feedback. I’ll be glad to incorporate any additional improvements or refactors.

[mamhoff]

I'd love to know the business case for this - usually, as a business I want customers to be able to successfully use my promotion codes, even if they e.g. only heard them on the radio or on a podcast. For the English language, this is a feature that's useful for very, very few stores, and setting it to true might seem like a good idea until the complaints come piling in. I'd rather not have it in the default configuration.

Can we make the coupon code normalization a configurable class instead?

I'm thinking of

# promotions/configuration.rb
class_name_attribute :coupon_code_normalizer_class, default: "Promotions::CaseInsensitiveCode"

# This is the class and its API 
module SolidusPromotions
  class CaseInsensitiveCode
    def self.call(value)
      value.strip.downcase
    end
  end
end

# This is how it would be used, e.g. in `SolidusPromotions::PromotionCode
def normalize_code
  self.value = SolidusPromotions.config.coupon_code_normalizer_class.call(value)
end

This way, y'all are free to implement a CaseSensitiveCode class to your own app, and we don't have to maintain the feature which I see of questionable use.

[ThuktenSingye]

I'd love to know the business case for this - usually, as a business I want customers to be able to successfully use my promotion codes, even if they e.g. only heard them on the radio or on a podcast. For the English language, this is a feature that's useful for very, very few stores, and setting it to true might seem like a good idea until the complaints come piling in. I'd rather not have it in the default configuration.

Can we make the coupon code normalization a configurable class instead?

I'm thinking of

# promotions/configuration.rb
class_name_attribute :coupon_code_normalizer_class, default: "Promotions::CaseInsensitiveCode"

# This is the class and its API 
module SolidusPromotions
  class CaseInsensitiveCode
    def self.call(value)
      value.strip.downcase
    end
  end
end

# This is how it would be used, e.g. in `SolidusPromotions::PromotionCode
def normalize_code
  self.value = SolidusPromotions.config.coupon_code_normalizer_class.call(value)
end

This way, y'all are free to implement a CaseSensitiveCode class to your own app, and we don't have to maintain the feature which I see of questionable use.

I'd love to know the business case for this - usually, as a business I want customers to be able to successfully use my promotion codes, even if they e.g. only heard them on the radio or on a podcast. For the English language, this is a feature that's useful for very, very few stores, and setting it to true might seem like a good idea until the complaints come piling in. I'd rather not have it in the default configuration.

Can we make the coupon code normalization a configurable class instead?

I'm thinking of

# promotions/configuration.rb
class_name_attribute :coupon_code_normalizer_class, default: "Promotions::CaseInsensitiveCode"

# This is the class and its API 
module SolidusPromotions
  class CaseInsensitiveCode
    def self.call(value)
      value.strip.downcase
    end
  end
end

# This is how it would be used, e.g. in `SolidusPromotions::PromotionCode
def normalize_code
  self.value = SolidusPromotions.config.coupon_code_normalizer_class.call(value)
end

This way, y'all are free to implement a CaseSensitiveCode class to your own app, and we don't have to maintain the feature which I see of questionable use.

@mamhoff Thank you for the feedback!. I agree that case-insensitive is the better default for customer experience, which is exactly why I implemented it that way. By default, the behavior is unchanged - codes remain case-insensitive just as Solidus has always maintained.

The preference is opt-in, so developers have to explicitly enable case-sensitivity. Users won't encounter this unless a developer intentionally changes the configuration.

With this boolean preference approach, users who need case-sensitive codes simply add one line to their configuration:

config.preferred_coupon_code_case_sensitive = true

They don't need to create any custom classes and make changes in multiple file. It just seems more straightforward and easy for developers.

As you mention, as a business, it depends. some business prefer this way and some might not though it will made easier for those who want.

[mamhoff]

Your reply does not include a business case, and it misses my point. I think it's preferable to not make this too easy, and we have a well-established pattern in the Solidus community to add custom classes to change behavior.

I'm not going to die on this hill. Core team: You decide. :)

[kennyadsl]

The @mamhoff's point is valid. There's no need to add other possible implementations of the interface, as long as it's easily configurable. Martin is suggesting to keep the class configurable, and you swap it with your own in your Solidus app.

The point is also that we are a small team, and we are not able to maintain all the possible ramifications of the use cases. Still, Solidus can be flexible enough to let you customize this part.

If you can make this change, this would be the preferred way of handling this change for me as well.

[ThuktenSingye]

@mamhoff and @kennyadsl Thank you for the feedback! I was elaborating how my current approach wouldn't affect the default Solidus behavior though it was off from the solidus pattern. The custom configurable class approach makes sense for keeping the core maintainable while still providing flexibility.

I encountered this need working with a client who runs events and they distribute the promotional code exclusively to their subscriber base or VIP customers. It just prevent from unauthorized redemptions and prevent excessive use of coupon code. There might be other business cases which I have no idea but I am happy to update and contribute if you find this needed down the road.

Thank you for you time! :)

[kennyadsl]

@ThuktenSingye Thanks to you! Looking forward to the updated PR!

[mamhoff]

We probably want to move the `coupon_code` accessor into the promotion gems for this, otherwise `solidus_core` depends on a promotion configuration. Alberto Vena ***@***.***> schrieb am Mo., 13. Okt. 2025, 16:23:

…

*kennyadsl* left a comment (solidusio/solidus#6331) <#6331 (comment)> @ThuktenSingye <https://github.com/ThuktenSingye> Thanks to you! Looking forward to the updated PR! — Reply to this email directly, view it on GitHub <#6331 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFLXKK3LOAFA2HQOOJGA7T3XOYX3AVCNFSM6AAAAACI5TDLZKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGOJXG42DSNBUGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[Copilot]

Pull Request Overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[ThuktenSingye]

@kennyadsl and @mamhoff, I have updated my PR based on previous suggestion. Since, i am still new to this, would appreciate your guidance here. Let me know if I need to make any changes and further improvement. Thank you.

[codecov]

Codecov Report

❌ Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.45%. Comparing base (f08a1ba) to head (0723dd3).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...p/patches/models/solidus_promotions/order_patch.rb	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6331      +/-   ##
==========================================
- Coverage   89.45%   89.45%   -0.01%     
==========================================
  Files         973      974       +1     
  Lines       20314    20322       +8     
==========================================
+ Hits        18172    18179       +7     
- Misses       2142     2143       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[mamhoff]

This looks much better, thank you!

Would you mind squashing the 12 commits into one before merging?

[ThuktenSingye]

This looks much better, thank you!

Would you mind squashing the 12 commits into one before merging?

sure @mamhoff. Thank you!

[mamhoff]

I think implementation-wise, this is great. I'm unsure about giving this topic that much space in the README file, because, again, I think the suggested behavior is a bit of a footgun.

[ThuktenSingye]

I think implementation-wise, this is great. I'm unsure about giving this topic that much space in the README file, because, again, I think the suggested behavior is a bit of a footgun.

@mamhoff Thank you. That make sense. maybe I can write a short notes or completely remove it and document it in developer documentation. what do you suggest?

[mamhoff]

I think implementation-wise, this is great. I'm unsure about giving this topic that much space in the README file, because, again, I think the suggested behavior is a bit of a footgun.

@mamhoff Thank you. That make sense. maybe I can write a short notes or completely remove it and document it in developer documentation. what do you suggest?

Write a short note in the Readme about the existence of the option, and comment in the developer docs (in the comments above where you define the class_name_attribute what can be done with it.

[tvdeyen]

I think implementation-wise, this is great. I'm unsure about giving this topic that much space in the README file, because, again, I think the suggested behavior is a bit of a footgun.

@mamhoff Thank you. That make sense. maybe I can write a short notes or completely remove it and document it in developer documentation. what do you suggest?

Write a short note in the Readme about the existence of the option, and comment in the developer docs (in the comments above where you define the class_name_attribute what can be done with it.

A guide in the Solidus guides would also be very much appreciated, but certainly the README is not the right place.

[mamhoff]

The spec failures seem related.

[ThuktenSingye]

The spec failures seem related.

@mamhoff I guess the spec failures is due to MySQL's case-insensitive collation unlike PostgreSQL which is case sensitive. Might need to create migration to make it case sensitive like:

class UpdatePromotionCodeValueCollation < ActiveRecord::Migration[7.0]  
 def up
    # Only run on MySQL
    if ActiveRecord::Base.connection.adapter_name.downcase.include?('mysql')
      change_column :solidus_promotions_promotion_codes, :value, :string,
                    collation: 'utf8mb4_0900_as_cs'
    end
  end

  def down
    if ActiveRecord::Base.connection.adapter_name.downcase.include?('mysql')
      change_column :solidus_promotions_promotion_codes, :value, :string,
                    collation: 'utf8mb4_general_ci'
    end
  end
end

What do you suggest?

[mamhoff]

That would have to be done only for MySQL. Because we're an engine that can be installed into any Rails app, you have to include the logic for finding out the DBMS within the migration. So something like

  def up
    return unless mysql?

    # your code here
  end
   
  def down 
    return unless mysql?

    # your code here
  end
  
  private

  def mysql?
      ActiveRecord::Base.connection.adapter_name.downcase.include?('mysql')
  end

I'm fine with this only because PG and SQLite are also case-sensitive. I'm leaving the check for the MySQL version to you. Also make sure this works with MariaDB.

[ThuktenSingye]

That would have to be done only for MySQL. Because we're an engine that can be installed into any Rails app, you have to include the logic for finding out the DBMS within the migration. So something like

  def up
    return unless mysql?

    # your code here
  end
   
  def down 
    return unless mysql?

    # your code here
  end
  
  private

  def mysql?
      ActiveRecord::Base.connection.adapter_name.downcase.include?('mysql')
  end

I'm fine with this only because PG and SQLite are also case-sensitive. I'm leaving the check for the MySQL version to you. Also make sure this works with MariaDB.

Thank you @mamhoff, I will work on this.

[ThuktenSingye]

@mamhoff, was able to update only today. Well I have created migration for case_sensitive collation and also update the docs. let me know for any changes required. Thank you.

[mamhoff]

I am fine with this implementation. Thank you.

[ThuktenSingye]

@mamhoff, most of the check are failing due to this error
Error: fatal: unable to access 'https://github.com/solidusio/solidus/': The requested URL returned error: 500

is there anything that i need to do here ?

[mamhoff]

@mamhoff, most of the check are failing due to this error Error: fatal: unable to access 'https://github.com/solidusio/solidus/': The requested URL returned error: 500

is there anything that i need to do here ?

No, that's a 500 error indicating Github had some issues. I've rebased the PR and triggered the tests again.