Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove all code and references for ChurnBuster #234

Conversation

garciajordy
Copy link

@garciajordy garciajordy commented Jun 17, 2021

fix #209

Remove ChurnBuster Integration

Removed Files:

  • app/decorators/models/solidus_subscriptions/spree/wallet_payment_source/report_default_change_to_subscriptions.rb
  • app/subscribers/solidus_subscriptions/churn_buster_subscriber.rb
  • spec/lib/solidus_subscriptions_spec.rb
  • spec/lib/solidus_subscriptions/churn_buster/client_spec.rb
  • spec/models/spree/wallet_payment_source_spec.rb
  • spec/subscribers/solidus_subscriptions/churn_buster_subscriber_spec.rb
  • spec/support/vcr.rb

solidus_subscriptions/lib/solidus_subscriptions/churn_buster Folder

  • client.rb
  • order_serializer.rb
  • serializer.rb
  • subscription_customer_serializer.rb
  • subscription_payment_method_serializer.rb
  • subscription_serializer.rb

solidus_subscriptions/spec/fixtures/cassettes Folder

  • churn_buster.yml

Changed Files:

README.md:

  • Removed ChurnBuster integration section.

solidus_subscriptions.gemspec:

  • Remove "vcr" from development dependecy.
  • Remove "webmock" from development dependency.

app/models/solidus_subscriptions/subscription.rb:

  • Remove ".with_default_payment_source" scope.
  • Remove
384-      if previous_changes.key?('payment_source_id') || previous_changes.key?('payment_source_type') || previous_changes.key?('payment_method_id')
385-        ::Spree::Event.fire(
386-          'solidus_subscriptions.subscription_payment_method_changed',
387-          subscription: self,
388-        )
389-      end

config/initializers/subscribers.rb:

  • Remove churn_buster_subscriber.

lib/solidus_subscriptions.rb:

  • Remove all the require paths for churn_buster.
  • Remove the 'churn_buster' method.

lib/solidus_subscriptions/configuration.rb:

  • Remove "churn_buster_account_id" and "churn_buster_api_key" from attr_accessor.
  • Remove the "churn_buster?" method.

dispatcher/payment_failed_dispatcher.rb:

  • Remove the Spree::Event.fire.

dispatcher/success_dispatcher.rb:

  • Remove the Spree::Event.fire.

spec/lib/solidus_subscriptions/dispatcher/payment_failed_dispatcher_spec.rb:

  • Remove the test for installments_failed_payment event.

spec/lib/solidus_subscriptions/dispatcher/success_dispatcher_spec.rb:

  • Remove the test for installments_succeeded event.

spec/models/solidus_subscriptions/subscription_spec.rb:

  • Remove test for tracking payment method changes.

@garciajordy
Copy link
Author

@aldesantis

@garciajordy garciajordy force-pushed the Remove-ChurnBuster-integration-209 branch 2 times, most recently from 401853b to 25c164d Compare June 17, 2021 19:37
Comment on lines 384 to 389
if previous_changes.key?('payment_source_id') || previous_changes.key?('payment_source_type') || previous_changes.key?('payment_method_id')
::Spree::Event.fire(
'solidus_subscriptions.subscription_payment_method_changed',
subscription: self,
)
end
Copy link
Member

Choose a reason for hiding this comment

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

We should keep all the Spree::Event.fire calls in place. Users of the extension can listen to those events to fire their own logic — they are not specific to the ChurnBuster integration, even though the integration did make use of them.

Copy link
Author

Choose a reason for hiding this comment

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

Done!

@garciajordy garciajordy force-pushed the Remove-ChurnBuster-integration-209 branch from 25c164d to 012199e Compare June 22, 2021 07:47
@garciajordy
Copy link
Author

garciajordy commented Jun 22, 2021

Changes

app/models/solidus_subscriptions/subscription.rb:

  • Add removed Spree::Event.fire as users of the extension can listen to those events to fire their own logic.

Thanks for taking the time to review my changes 😄

Btw I squashed both commits as I heard it is better to have everything into one commit for overview.

@stale
Copy link

stale bot commented Aug 22, 2021

This issue has been automatically marked as stale because it has not had recent activity. It might be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Aug 22, 2021
@kennyadsl
Copy link
Member

Closing for now, we can reopen if we want to move this forward.

@kennyadsl kennyadsl closed this Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove ChurnBuster integration
3 participants