-
Notifications
You must be signed in to change notification settings - Fork 0
Nonprofit and cards references #1208
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
base: supporter_level_goal
Are you sure you want to change the base?
Changes from all commits
665d307
2313873
9d3eb36
837b59f
ea62a7f
880a571
6cb2ddb
75414f3
c5faf33
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ module InsertCard | |
| # If a stripe_customer_id is present, then update that customer's primary source; otherwise create a new customer | ||
| # @param [ActiveSupport::HashWithIndifferentAccess] card_data card data | ||
| # @option card_data [Integer] holder_id the primary key of the card's holder | ||
| # @option card_data [String] holder_type the type of the card holder. Must be 'Nonprofit' or 'Supporter' | ||
| # @option card_data [String] holder_type the type of the card holder. Must be'Supporter' | ||
|
|
||
| # @option card_data [String] stripe_card_token the card token from stripe | ||
| # @option card_data [String] stripe_card_id the card id from stripe | ||
|
|
@@ -18,7 +18,7 @@ module InsertCard | |
| def self.with_stripe(card_data, stripe_account_id = nil, event_id = nil, current_user = nil) | ||
| begin | ||
| ParamValidation.new(card_data.to_deprecated_h.merge({event_id: event_id}), { | ||
| holder_type: {required: true, included_in: ["Nonprofit", "Supporter"]}, | ||
| holder_type: {required: true, included_in: [ "Supporter"]}, | ||
| holder_id: {required: true}, | ||
| stripe_card_id: {not_blank: true, required: true}, | ||
| stripe_card_token: {not_blank: true, required: true}, | ||
|
|
@@ -32,15 +32,12 @@ def self.with_stripe(card_data, stripe_account_id = nil, event_id = nil, current | |
| # validate that the user is with the correct nonprofit | ||
|
|
||
| card_data = card_data.slice(:holder_type, :holder_id, :stripe_card_id, :stripe_card_token, :name) | ||
| holder_types = {"Nonprofit" => :nonprofit, "Supporter" => :supporter} | ||
| holder_types = { "Supporter" => :supporter} | ||
| holder_type = holder_types[card_data[:holder_type]] | ||
| holder = nil | ||
| begin | ||
| if holder_type == :nonprofit | ||
| holder = Nonprofit.select("id, email").includes(:cards).find(card_data[:holder_id]) | ||
| elsif holder_type == :supporter | ||
| holder = Supporter.select("id, email, nonprofit_id").includes(:cards, :nonprofit).find(card_data[:holder_id]) | ||
| end | ||
| holder_type == :supporter | ||
| holder = Supporter.select("id, email, nonprofit_id").includes(:cards, :nonprofit).find(card_data[:holder_id]) | ||
| rescue ActiveRecord::RecordNotFound | ||
| return {json: {error: "Sorry, you need to provide a nonprofit or supporter"}, status: :unprocessable_entity} | ||
| end | ||
|
|
@@ -86,18 +83,14 @@ def self.with_stripe(card_data, stripe_account_id = nil, event_id = nil, current | |
| source_token = nil | ||
| begin | ||
| Card.transaction { | ||
| if holder_type == :nonprofit | ||
| # @type [Nonprofit] holder | ||
| card = holder.create_active_card(card_data) | ||
| elsif holder_type == :supporter | ||
| holder_type == :supporter | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What affect does this line have on the behavior of the code? |
||
| # @type [Supporter] holder | ||
| card = holder.cards.create(card_data) | ||
| params = {} | ||
| if event | ||
| params[:event] = event | ||
| end | ||
| source_token = InsertSourceToken.create_record(card, params).token | ||
| end | ||
| card.save! | ||
| } | ||
| rescue ActiveRecord::ActiveRecordError => e | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,6 @@ class BillingSubscriptionMailer < BaseMailer | |
| def failed_notice(np_id) | ||
| @nonprofit = Nonprofit.find(np_id) | ||
| @billing_subscription = @nonprofit.billing_subscription | ||
| @card = @nonprofit.active_card | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So you've deleted this line but what is this mailer used for? What about the view associated with the mailer action, does this change break it? Or do we even need the mailer or view? |
||
| @billing_plan = @billing_subscription.billing_plan | ||
| @emails = QueryUsers.all_nonprofit_user_emails(@nonprofit.id) | ||
| mail(to: @emails, subject: "Action Needed, Please Update Your #{Settings.general.name} Account") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,13 +9,11 @@ class Card < ApplicationRecord | |
| :stripe_card_token, # str | ||
| :stripe_card_id, # str | ||
| :stripe_customer_id, # str | ||
| :holder, :holder_id, :holder_type, # polymorphic cardholder association | ||
| :inactive # a card is inactive. This is currently only meaningful for nonprofit cards | ||
|
|
||
| scope :amex_only, -> { where("cards.name ILIKE ? OR cards.name ILIKE ?", "American Express%", "amex%") } | ||
| :holder, :holder_id, :holder_type # polymorphic cardholder association | ||
|
|
||
| scope :amex_only, -> { where("cards.name ILIKE ? OR cards.name ILIKE ?", "American Express%", "amex%") } | ||
| scope :not_amex, -> { where("cards.name NOT ILIKE ? AND cards.name NOT ILIKE ?", "American Express%", "amex%") } | ||
|
|
||
| scope :held_by_nonprofits, -> { where("cards.holder_type = ? ", "Nonprofit") } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should keep this, just because we removed the references in the code doesn't mean we've removed all of the records for nonprofit cards. At some point, we'll need to clear those out and delete them. Please note that we need to do that. |
||
| scope :held_by_supporters, -> { where("cards.holder_type = ? ", "Supporter") } | ||
|
|
||
| attr_accessor :failure_message | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,13 +15,8 @@ | |
| <p><a class='u-color--red u-small' href='/nonprofits/<%=@nonprofit.id%>/billing_subscription/cancellation'>Unsubscribe from plan</a></p> | ||
| <% end %> | ||
| <% if @nonprofit.active_card %> | ||
| <hr> | ||
| <label>Payment Method</label> | ||
| <p>Current payment method for your nonprofit: | ||
| <strong><%= @nonprofit.active_card.name %></strong> | ||
| </p> | ||
| <% end %> | ||
| <hr> | ||
| <label>Payment Method</label> | ||
|
Comment on lines
+18
to
+19
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is being labelled by the Payment Method label? |
||
| <p class='note u-marginTop--10'>If you have any questions about your current plan, please contact <a href='mailto:<%= Settings.devise.mailer_sender %>'><%= Settings.devise.mailer_sender %></a></p> | ||
| </div> | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are traits in this file which reference active_card. If we're removing the active_card attribute and other associated methods, those need to be addressed too. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,25 +1,4 @@ | ||
| # License: AGPL-3.0-or-later WITH Web-Template-Output-Additional-Permission-3.0-or-later | ||
| require "rails_helper" | ||
| describe "nonprofits factory" do | ||
| describe :with_billing_subscription_on_stripe do | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why keep an empty spec? It's a headache to keep a spec file with nothing in it. |
||
| it "creates one Nonprofit" do | ||
| create(:nonprofit_base, :with_billing_subscription_on_stripe) | ||
| expect(Nonprofit.count).to eq 1 | ||
| end | ||
|
|
||
| it "creates one BillingSubscription" do | ||
| create(:nonprofit_base, :with_billing_subscription_on_stripe) | ||
| expect(BillingSubscription.count).to eq 1 | ||
| end | ||
|
|
||
| it "creates 1 BillingPlan" do | ||
| create(:nonprofit_base, :with_billing_subscription_on_stripe) | ||
| expect(BillingPlan.count).to eq 1 | ||
| end | ||
|
|
||
| it "creates 1 Card" do | ||
| create(:nonprofit_base, :with_billing_subscription_on_stripe) | ||
| expect(Card.count).to eq 1 | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,4 @@ | ||
| # License: AGPL-3.0-or-later WITH Web-Template-Output-Additional-Permission-3.0-or-later | ||
| require "rails_helper" | ||
| describe "nonprofits factory" do | ||
| describe :with_billing_subscription_on_stripe do | ||
| it { | ||
| nonprofit = create(:nonprofit_base, :with_billing_subscription_on_stripe) | ||
| expect(nonprofit).to have_attributes(attributes_for(:nonprofit_base, :with_billing_subscription_on_stripe)) | ||
| } | ||
| end | ||
| end |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What affect does this line have on the behavior of the code?