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

[Bye bye spree] Bring base_helper and log_entry from spree core #5924

Merged
merged 5 commits into from Sep 16, 2020

Conversation

luisramos0
Copy link
Contributor

@luisramos0 luisramos0 commented Aug 19, 2020

What? Why?

Continues #4826
Here we move the following files to OFN:

  • core/app/helpers/spree/base_helper.rb MERGE WITH DECORATOR
  • core/spec/helpers/base_helper_spec.rb MERGE WITH DECORATOR
  • core/app/models/spree/log_entry.rb COPY

No code changes were made, here we only move the code from spree, adapt the specs if needed and improve the code a little with rubocop and transpec.

What should we test?

Re Spree::BaseHelper:

  • pretty time - make sure a payment date is displayed correctly in the order payments page, like this:
    image

re Spree::LogEntry

  • make a Stripe payment then run this SQL in the database of the server (replace order number):
    select l.details from spree_orders o, spree_payments p, spree_log_entries l where o.number = 'R820014810' and o.id = p.order_id and p.id = l.source_id;
    There should be some log info about the transaction with Stripe.

Release notes

Changelog Category: Changed
Brought code needed in OFN from Spree so that we can make OFN independent of Spree.

@luisramos0 luisramos0 changed the title Basic spree core [Bye bye spree] Bring some base classes from spree core Aug 19, 2020
@luisramos0 luisramos0 marked this pull request as draft August 19, 2020 03:51
@luisramos0 luisramos0 self-assigned this Aug 19, 2020
@luisramos0 luisramos0 changed the title [Bye bye spree] Bring some base classes from spree core [Bye bye spree] Bring base_controller, base_helper and log_entry from spree core Aug 19, 2020
@luisramos0 luisramos0 marked this pull request as ready for review August 19, 2020 16:01
@luisramos0 luisramos0 changed the title [Bye bye spree] Bring base_controller, base_helper and log_entry from spree core [Bye bye spree] Bring base_helper and log_entry from spree core Aug 20, 2020

# Fix for Spree #1767
# If a payment fails, we want to make sure we keep the record of it failing
after_rollback :save_anyway
Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking out loud: It would've been much easier not to include this in the same DB transaction so it wouldn't have been rolled back.

@filipefurtad0 filipefurtad0 self-assigned this Sep 16, 2020
@filipefurtad0 filipefurtad0 added the pr-staged-uk staging.openfoodnetwork.org.uk label Sep 16, 2020
@filipefurtad0
Copy link
Contributor

filipefurtad0 commented Sep 16, 2020

Hi @luisramos0 ,

Before staging this PR, I checked the Payments tab, when editing an order before this PR, for an order with Stripe-SCA payment method:

image

Then, I ran the SQL command for that order, and got (first lines):

                                                                  details                                                                  
-------------------------------------------------------------------------------------------------------------------------------------------
 --- !ruby/object:ActiveMerchant::Billing::Response                                                                                       +
 params:                                                                                                                                  +
   id: pi_1HRyCNKuuB1fWySnE4Icc45j                                                                                                        +
   object: payment_intent                                                                                                                 +
   allowed_source_types:                                                                                                                  +
   - card                                                                                                                                 +
   amount: 16100                                                                                                                          +
   amount_capturable: 16100                                                                                                               +
   amount_received: 0                                                                                                                     +
   application: ca_9ByaSyyyXj5O73DWisU0KLluf0870Vro                                                                                       +
   application_fee_amount:                                                                                                                +
   canceled_at:                                                                                                                           +
   cancellation_reason:                                                                                                                   +
   capture_method: manual                                               



After that, I staged the PR and

  • verified the previously placed order looks as before (Payments tab and SQL output)

  • placed a new order, and checked how Payments look like, in the back office:

image

Then, I ran the SQL command, for that order:

                                                              details                                                                  
-------------------------------------------------------------------------------------------------------------------------------------------
--- !ruby/object:ActiveMerchant::Billing::Response                                                                                       +
params:                                                                                                                                  +
 id: pi_1HRydiKuuB1fWySnsNMjUcob                                                                                                        +
 object: payment_intent                                                                                                                 +
 allowed_source_types:                                                                                                                  +
 - card                                                                                                                                 +
 amount: 16100                                                                                                                          +
 amount_capturable: 16100                                                                                                               +
 amount_received: 0                                                                                                                     +
 application: ca_9ByaSyyyXj5O73DWisU0KLluf0870Vro                                                                                       +
 application_fee_amount:                                                                                                                +
 canceled_at:                                                                                                                           +
 cancellation_reason:                                                                                                                   +



All good!
Ready to go.

@filipefurtad0 filipefurtad0 removed the pr-staged-uk staging.openfoodnetwork.org.uk label Sep 16, 2020
@luisramos0 luisramos0 merged commit f566c21 into openfoodfoundation:master Sep 16, 2020
@luisramos0 luisramos0 deleted the basic_spree_core branch September 16, 2020 13:57
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.

None yet

4 participants