openfoodfoundation / openfoodnetwork

Connect suppliers, distributors and consumers to trade local produce.
https://www.openfoodnetwork.org
GNU Affero General Public License v3.0
1.12k stars 724 forks source link

[Spree 2.1] Replace various quickfixes in the upgrade branch #4777

Closed luisramos0 closed 4 years ago

luisramos0 commented 4 years ago

This issue/epic represents all the quickfixes we have currently in the upgrade branch that must be analysed, fixed, removed before the branch can go live.

TODO:

luisramos0 commented 4 years ago

Adding an extra point for consideration: why is gem 'i18n-js', '~> 3.5.1' downgraded to 3.2.1 in 3-0-stable. Needs to be investigated.

Matt-Yorkley commented 4 years ago

I think some of these bits might be dead code essentially, or things that we don't need to replace.

luisramos0 commented 4 years ago
RachL commented 4 years ago

@luisramos0 @Matt-Yorkley yes we have hubs in France using it for their POS. I discovered this when I wanted to disable invoices... Apparently it's a hack some of them even paid for back then 🙈 It's not something maintainable on the long run for sure, but right now I don't see how I can stop them to use it...

luisramos0 commented 4 years ago

thanks Rachel! It doesnt look like an hack, looks like a valid implementation of a specific feature. And it looks maintainable as well (the fact that is in a separate repo is really good). There is a cost involved in mantaining it for sure and we are paying for it now for example, I believe this is what you meant, I just wanted to clarify :+1:

RachL commented 4 years ago

Yes this is what I've meant. I use the "hack" word because from my understanding it is using somehow the invoices to generate the receipts. Yet the receipts can have specific requirements so it's a bit hacky. And adds even more load to the fact that invoices are not legal in some countries. Anyway. A topic for later :see_no_evil:

Matt-Yorkley commented 4 years ago

I'm not sure we have any tests that cover this QZ integration...? :see_no_evil:

luisramos0 commented 4 years ago

I see one in the build:

 6) 
    As an administrator
    I want to manage orders
 as an enterprise manager viewing the edit page can print an order's ticket
     Got 0 failures and 2 other errors:

     6.1) Failure/Error:
            accept_alert do
              print_data = page.evaluate_script('printData');
              elements_in_print_data =
                [
                  order.distributor.name,
                  order.distributor.address.address_part1,
                  order.distributor.address.address_part2,
                  order.distributor.contact.email,
                  order.number,
                  order.line_items.map { |line_item|

          Capybara::ModalNotFound:
            Unable to find modal dialog
          # ./spec/features/admin/orders_spec.rb:338:in `block (5 levels) in <top (required)>'
          # ./spec/features/admin/orders_spec.rb:337:in `block (4 levels) in <top (required)>'
          # ------------------
          # --- Caused by: ---
          # Selenium::WebDriver::Error::TimeoutError:
          #   timed out after 30 seconds (no such alert
          #     (Session info: headless chrome=72.0.3626.96)
          #     (Driver info: chromedriver=72.0.3626.69 (3c16f8a135abc0d4da2dff33804db79b849a7c38),platform=Linux 4.4.0-116-generic x86_64))
          #   ./spec/features/admin/orders_spec.rb:338:in `block (5 levels) in <top (required)>'

     6.2) Failure/Error: @app.call(env)

          ActionController::RoutingError:
            No route matches [GET] "/javascripts/qz/ticket-popup.js"
          # ./lib/open_food_network/rack_request_blocker.rb:36:in `call'
          # ------------------
          # --- Caused by: ---
          # Capybara::CapybaraError:
          #   Your application server raised an error - It has been raised in your test code because Capybara.raise_server_errors == true
          #   ./spec/spec_helper.rb:115:in `block (2 levels) in <top (required)>'
Matt-Yorkley commented 4 years ago

Okay, I'm taking a look at ofn-qz now. The gem looks like it might be a modified fork, but I can't find any sign of an upstream/original anywhere... maybe @ltrls has some advice? :pray:

The gem seems to mostly contain a few bits of javascript and not much else. Maybe we can just hack the restrictions in the gemspec and see if it works as-is with Rails 4?

luisramos0 commented 4 years ago

yes, I think its just a bit of js code, together with some js dependencies. Your plan sounds good :+1:

Sent from my iPhone

On 28 Feb 2020, at 14:02, Matt-Yorkley notifications@github.com wrote:

Okay, I'm taking a look at ofn-qz now. The gem looks like a modified version from somewhere, but I can't find any sign of an upstream/original anywhere... maybe @ltrls has some advice? 🙏

The gem seems to mostly contain a few bits of javascript and not much else. Maybe we can just hack the restrictions in the gemspec and see if it works as-is with Rails 4?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

Matt-Yorkley commented 4 years ago

Looks like it works :tada:

luisramos0 commented 4 years ago

I added spring and newrelix_rpm back in #5044

luisramos0 commented 4 years ago

I moved the price_adjustments item to a separate issue #5045

luisramos0 commented 4 years ago

I moved the problem with FoundationRailsHelper to a separate issue #5046

luisramos0 commented 4 years ago

this issue is now in code review and can be close after #5010 and #5044 are merged.

luisramos0 commented 4 years ago

:tada: