Skip to content

Subscription api fix for subscribable product#312

Merged
kennyadsl merged 7 commits intosolidusio:mainfrom
gms-electronics:subscription_api_fix_for_subscribable_product
Jan 16, 2025
Merged

Subscription api fix for subscribable product#312
kennyadsl merged 7 commits intosolidusio:mainfrom
gms-electronics:subscription_api_fix_for_subscribable_product

Conversation

@JustShah
Copy link
Copy Markdown
Contributor

@JustShah JustShah commented Jan 10, 2025

Summary:


This PR introduces improvements and new features to the Solidus Subscriptions module, with a focus on subscription validation and handling subscription-related logic.

Changes:


Validation for Subscribable Products:

  • A custom validation ensure_subscribable_valid is added to the SolidusSubscriptions::LineItem model. This validation ensures that only products marked as subscribable can be added to a subscription. If a product is not subscribable, an error is added to the subscribable field with the message "The requested item cannot be subscribed".

Localization:

  • Added an entry to config/locales/en.yml to define the error message for the subscribable validation ("The requested item cannot be subscribed").

Subscription Form Enhancements:

  • Updated the subscription form in app/views/cart_line_items/_subscription_fields.html.erb to display subscription fields dynamically based on whether a product variant is subscribable. The form will show the subscription options only if the selected variant is subscribable.

Controller Enhancements:

  • Updated the CreateSubscription concern to improve the logic for handling subscription line items. A new helper method valid_subscription_line_item_params? is introduced to ensure that all required parameters are present before processing subscription-related actions.

Promotion Rules Registration:

  • Ensured that promotion rules for subscriptions (SubscriptionCreationOrder and SubscriptionInstallmentOrder) are only registered if the Spree::Promotion class is defined, ensuring compatibility with different Solidus versions.

Test Coverage:

  • Added comprehensive tests to ensure the new validation behavior is functioning correctly.
  • The tests include cases for both valid and invalid subscription line items, verifying that the appropriate error messages are triggered when the product is not subscribable.

New Tests:

  • Added tests for CreateSubscription concern, particularly for the new helper method valid_subscription_line_item_params?.
  • Expanded coverage for the SolidusSubscriptions::LineItem model to test the new subscribable validation.
  • Updated various existing tests to work with the new subscribable functionality, including those for the API controllers and subscription logic.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

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 opened a PR to update the guides.
  • ✅ I have added automated tests to cover my changes.
  • 📸 I have attached screenshots to demo visual changes.

@fthobe
Copy link
Copy Markdown

fthobe commented Jan 10, 2025

@shahmayur001 postgres keeps failing

@fthobe
Copy link
Copy Markdown

fthobe commented Jan 10, 2025

Fully fixes #308

@JustShah
Copy link
Copy Markdown
Contributor Author

@shahmayur001 postgres keeps failing

I had manually added the solidus_support gem on my gemfile to run the test suites

gem 'solidus_support', git:'https://github.com/mamhoff/solidus_support', branch:'revert-legacy-event-compat-support-removal'

My tests were passing I am not sure why the cases are failing here as the branch that was resolving the issue of the test suites has been merged already but on Circle CI the errors on tests are still related to that.

NameError: uninitialized constant SolidusSupport::LegacyEventCompat

@kennyadsl Do I have to do anything specific to load the support gem changes ?

@fthobe
Copy link
Copy Markdown

fthobe commented Jan 13, 2025

@kennyadsl done and ready to merge

@kennyadsl
Copy link
Copy Markdown
Member

there are some specs still failing. Can you take a look please?

@fthobe
Copy link
Copy Markdown

fthobe commented Jan 13, 2025

@kennyadsl they seem to fail only on sqlite and do not seem to be related to us.

@JustShah
Copy link
Copy Markdown
Contributor Author

there are some specs still failing. Can you take a look please?

I think its related to SQLite-relaed issue in Circle CI, Please refer to below screenshot, It also failing last time.

image

@fthobe
Copy link
Copy Markdown

fthobe commented Jan 16, 2025

@kennyadsl should be possible to merge this.

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.

3 participants