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

[Spree Upgrade] Merging master into 2-0-stable (4th run in Mar2019) #3668

Merged
merged 57 commits into from
Apr 9, 2019

Conversation

luisramos0
Copy link
Contributor

@luisramos0 luisramos0 commented Mar 29, 2019

Merging master into 2-0-stable.
Same as previous #3640

This is the result of:

git checkout master

git pull upstream master

git checkout 2-0-stable
git pull upstream 2-0-stable
git checkout -b 2-0-stable-Mar29
git merge master

####Conflicts:

CONFLICT (content): Merge conflict in spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb
keep master and replace factory line_item with line_item_with_shipment in a few places
git add spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb
I am having trouble with the new specs from #3646, @kristinalim @mkllnk could you please have a look?

CONFLICT (content): Merge conflict in app/controllers/spree/admin/variants_controller_decorator.rb
keep HEAD
git add app/controllers/spree/admin/variants_controller_decorator.rb
remove delete_and_refresh_cache from variant_decorator
git add app/models/spree/variant_decorator.rb
remove delete_and_refresh_cache tests from variant spec
git add spec/models/spree/variant_spec.rb
thanks @sauloperez for directions here, it made the merge very easy. I thought it was working but the build is broken here as well in app/models/spree/variant_decorator.rb @sauloperez could you please have a look?

CONFLICT (content): Merge conflict in Gemfile.lock
keep 2-0 but update stripe to 4.11
git add Gemfile.lock

CONFLICT (content): Merge conflict in Gemfile
keep 2-0 but update stripe to 4.11
git add Gemfile

git commit
git push origin 2-0-stable-Mar29

What should we test?

On Zenhub, this PR is connected with and blocks #3625.

According to PR #3647, #3625 needs to be solved already in this merge PR.

there are also these PRs that are now brought to v2 (I am not sure we need to retest): #3662, #3646 and #3644

Matt-Yorkley and others added 30 commits February 16, 2019 01:05
Bumps [stripe](https://github.com/stripe/stripe-ruby) from 4.9.0 to 4.10.0.
- [Release notes](https://github.com/stripe/stripe-ruby/releases)
- [Changelog](https://github.com/stripe/stripe-ruby/blob/master/CHANGELOG.md)
- [Commits](stripe/stripe-ruby@v4.9.0...v4.10.0)

Signed-off-by: dependabot[bot] <support@dependabot.com>
Bumps [oj](https://github.com/ohler55/oj) from 3.7.10 to 3.7.11.
- [Release notes](https://github.com/ohler55/oj/releases)
- [Changelog](https://github.com/ohler55/oj/blob/master/CHANGELOG.md)
- [Commits](ohler55/oj@v3.7.10...v3.7.11)

Signed-off-by: dependabot[bot] <support@dependabot.com>
These tests prove that
openfoodfoundation#3629 is
indeed is a bug because we don't refresh the cache when deleting
a variant.
Note that, as explained in
https://apidock.com/rails/v3.2.13/ActiveRecord/Relation/delete, `delete` does
not trigger callbacks and so it skips the products cache logic.

If we still want to avoid instantiating the AR object, we need to explicitly
call that logic for the cache to be up-to-date.
Probably a copy&paste error. The PackingReport had exactly the same
spec.
This conversion is done by Transpec 3.4.0 with the following command:
    transpec spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb

* 8 conversions
    from: == expected
      to: eq(expected)

* 8 conversions
    from: obj.should
      to: expect(obj).to

* 1 conversion
    from: obj.stub(:message)
      to: allow(obj).to receive(:message)

For more details: https://github.com/yujinakayama/transpec#supported-conversions
This groups the Customer Totals report by variant too (among others,
including by product), and then changes the report to use the variant
SKU not the product SKU.

This adds a spec for customer totals grouping and details, which
replaces the test specific to the SKU column.
Line items belong to a single order.
…product_sku_in_report

3528 Scope Customer Totals report also by variant (not just product) and use variant SKU
@luisramos0
Copy link
Contributor Author

thank you very much @kristinalim 🙏

luisramos0 and others added 9 commits April 1, 2019 15:37
…method

 Add shipping method name to orders detail report
I saw the following error on the 2-0-stable branch:

  translation missing: en.spree.line_item_adjustments

In a pending PR I change all three translations in the view file to use
lazy lookup. This commit backports the addition to the locale so that it
can be translated via Transifex before we release v2.
…rides-in-inventory-report

Test inventory report to use variant overrides
…mand_error

Fix nil values in on_hand column
…2-translation

Add missing translation for order form in v2
@sauloperez sauloperez self-assigned this Apr 5, 2019
In v2 we no longer rely on `Variant#delete` but `Variant#destroy` so
stubbed calls needed fixing.
@sauloperez
Copy link
Contributor

Fixed! Sorry for my late notice and thanks for the offering @kristinalim

@luisramos0
Copy link
Contributor Author

ah, awesome, thanks a lot team!!!

@luisramos0
Copy link
Contributor Author

This was one week old already so I run another git merge master into this branch.
Conflicts and broken specs fixed in 3 additional commits, have a look.

@kristinalim
Copy link
Member

This PR moves with #3625 in Zenhub. I added the section "What should we test?" above:

On Zenhub, this PR is connected with and blocks #3625.

According to PR #3647, #3625 needs to be solved already in this merge PR.

@luisramos0
Copy link
Contributor Author

yeah, I have added 3 more PRs that are brought to v2 with this.
Anyway, this is last big merge from master 🎉

@luisramos0 luisramos0 merged commit 4478d51 into openfoodfoundation:2-0-stable Apr 9, 2019
@luisramos0
Copy link
Contributor Author

apologies for bypassing the process, I decided to merge this now so we can test everything together (#3604 for example) now.
if there are problems with this we can create new issues.

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.

8 participants