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

In subtransaction_payment jbuilder, get created from the paymentable, not subtransaction_payment #672

Merged
merged 6 commits into from
May 16, 2023

Conversation

awadyas
Copy link

@awadyas awadyas commented May 11, 2023

NOTE: DO NOT discuss internal CommitChange information in your PR; this PR will be public.
Link back to the issue in the Tix repo when you need to do that.

Closes https://github.com/CommitChange/tix/issues/4040

@awadyas awadyas requested a review from wwahammy May 11, 2023 22:12
Copy link
Member

@wwahammy wwahammy left a comment

Choose a reason for hiding this comment

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

Hmmm, this does fix the problem in a sense but it's not quite the right layer.

We'll need to be able to search and sort SubtransactionPayments by their created value. That means, for this PR, SubtransactionPayment#created to be set or updated when the legacy_payment.date is set or changed.

@awadyas
Copy link
Author

awadyas commented May 12, 2023

We'll need to be able to search and sort SubtransactionPayments by their created value. That means, for this PR, SubtransactionPayment#created to be set or updated when the legacy_payment.date is set or changed.

@wwahammy How does this sound as an approach: Adding a method on Payment to sync the date from the legacy payment to SubtransactionPayment#created, and putting the sync method in create and update in PaymentsController, conditionally, only if the payment has an associated subtransaction payment?

@awadyas
Copy link
Author

awadyas commented May 12, 2023

So something like this

(in payments_controller.rb)

def create
  ...
  @payment.sync_date_to_subtransaction_payment if @payment.subtransaction_payment
end

def update
  ...
  @payment.sync_date_to_subtransaction_payment if @payment.subtransaction_payment
end

def sync_date_to_subtransaction_payment
  subtransaction_payment.created = date
end

Also: is there a way to only update created when the date specifically is updated? I think it would be cool / even better if we could do that

@awadyas awadyas requested a review from wwahammy May 12, 2023 18:55
@wwahammy
Copy link
Member

wwahammy commented May 12, 2023

Unfortunately, there are ways in which payments can be updated and created that are not through an http request. For example, a recurring donation could run and a payment would be created for that but no http request would have occurred.

This will have to happen in InsertDonation, UpdateDonation, UpdateTickets, etc.

Copy link
Member

@wwahammy wwahammy left a comment

Choose a reason for hiding this comment

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

Looks good. Is it possible to unit test the changes in InsertTickets too?

@awadyas
Copy link
Author

awadyas commented May 15, 2023

Is it possible to unit test the changes in InsertTickets too?

Yes, forgot about that lol 😂 I"m on it

@wwahammy wwahammy force-pushed the subtransaction-payment-jbuilder-tweak branch from 5e33e40 to 11af9db Compare May 15, 2023 23:40
Copy link
Member

@wwahammy wwahammy left a comment

Choose a reason for hiding this comment

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

One quick tweak I think

spec/lib/insert/insert_tickets_spec.rb Outdated Show resolved Hide resolved
Co-authored-by: Eric Schultz <eric@commitchange.com>
@wwahammy wwahammy merged commit e13261c into supporter_level_goal May 16, 2023
@wwahammy wwahammy deleted the subtransaction-payment-jbuilder-tweak branch May 16, 2023 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants