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 (one month of commits in master) #2863

Merged
merged 246 commits into from
Oct 16, 2018

Conversation

luisramos0
Copy link
Contributor

@luisramos0 luisramos0 commented Oct 12, 2018

Same as last time #2639

What? Why?
Merging master into 2-0-stable.

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-oct
git merge master

resolved conflicts:
git rm spec/models/spree/order_updater_spec.rb
git add spec/models/order_updater_spec.rb
git rm app/models/spree/order_updater_decorator.rb
git add app/models/order_updater.rb
.... @sauloperez can you please verify order_updater and spec? difficult to merge with a file move and changes on both sides...

git add spec/features/admin/product_import_spec.rb # added new spec and made it pending
git add db/seeds.rb. # replicate af1ac33 on mail_configuration
git add db/schema.rb. # drop shipping_method_name and keep cost_price and tax_category_id on table line_items
git rm app/models/spree/inventory_unit_decorator.rb
git add app/controllers/spree/admin/shipping_methods_controller_decorator.rb # keep 2-0-stable version
git add Gemfile.lock. # keep factory_bot (4.10.0) and kaminari (0.14.1), etc…

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

I double checked the list of commits on GH and it looks good.

kristinalim and others added 30 commits August 26, 2018 03:58
Devs keep using `#send` although that method does not preserve
private/protected visibility. Watching after this turned out to be quite
time-consuming while doing code review.

Currently, the Style/Send cop doesn't enforce `#public_send` however
(that's what we want). It simply discourages the use of #send. See
rubocop/rubocop#2081 (comment)
for details. So a new entry on the Code Conventions doc has been added
to overcome this limitation:
https://github.com/openfoodfoundation/openfoodnetwork/wiki/Code-Conventions#prefer-public_send-over-send
…ons-rename_entreprise_keys_to_enterprise

Rename "entreprise" in i18n keys to "enterprise"
When there is enough content in the modal, the height of the modal plus
its top margin could exceed the height of the viewport.

Considering a top position of 10%, a max height of 80% renders a tall
modal vertically centered, with 10% remaining space at the bottom.
Occasionally, the page scrolls up while the modal is being opened. This
was causing the final position of the modal to be at the wrong location
relative to the viewport.

This was happening because of a race condition between the animation
that slides the modal from above the viewport to the middle, and focus()
which the modal does:

https://github.com/yalabot/angular-foundation/blob/0.8.0/src/modal/modal.js#L109

The final vertical position of the modal is at 10%, so the animation
which translates the modal -25% vertically was starting -15% above the
viewport. The focus() was then causing vertical scroll.

This lowers the starting point of the animation, so there will no longer
be scrolling.

Additionally, the animation would only happen on large screens. The CSS
property "top" is 0 for smaller screens.
Matt-Yorkley and others added 11 commits October 10, 2018 18:32
This covers creation and update of a product.
As it is this is impossible to follow.
Now it's imposible to understand what is really going on. Feels more
like assembler than Ruby.
Variants whose product's variant_unit is weight or volume require
a unit_value.
This adds a data migration to fix those cases. It defaults to showing
1 unit of the specified weight. That is, if the user chose Kg, it'll
display 1 as unit.

Note that migration logs the process in a log/migrate.log file separate
from the regular Rails log/production.log file.

When you run the migration you'll see something like:

  Fixing variants missing unit_value...

  Processing variant 12...
  Succesfully fixed variant 12

  Done!

This helps auditing the changes applied and debug any possible failure
scenarios. You can delete it once all is ok.
@luisramos0 luisramos0 self-assigned this Oct 12, 2018
Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

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

This is too big. I think we have to merge more often. Should we do it a weekly task?

I had a quick sanity check of the commits I authored. I have no idea how I could spot merge errors in other commits without merging it myself and going through the conflicts. I just have to trust you here (and I do).

…_engine

[OFN Domains] Breaking OFN into domains - POC cookies inside an engine
@luisramos0
Copy link
Contributor Author

yes, agree. we have to do this more often!

The only two places where there could be an error on my side are:

  • gemlock (there are quite a few conflicts)
  • order_updater

I'll repeat this process today after fixing the rubocop issue. So I'll have an opportunity to double check every single step.

@luisramos0
Copy link
Contributor Author

Ive redone this today, imo, ready to go.

Copy link
Contributor

@sauloperez sauloperez 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 to me

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.