removed assets related to spree store: dead code #2540

luisramos0 · 2018-08-11T22:40:20Z

What? Why?

I found this in the context of deleting dead code here #2539
Deleting dead UI code. The OFN frontoffice should not depend on any Spree JS code any longer.

What should we test?

The frontoffice (OFN shop) should work and look the same way.

Release notes

Removed unused Spree UI code.

Changelog Category: Removed

mkllnk

I actually don't know if this is used or not. I can't find any other require statement. Let's test it. :-D

Do you happen to know why we need to add stuff like shared/* here? Shouldn't that already be included in admin/all or darkswarm/all?

I'm also just seeing the all_split2 again which we don't need any more.

luisramos0 · 2018-08-18T16:18:50Z

re /shared, I dont know.
I have checked and most stuff in shared is called and included in the all.js file of admin or darkswarm.
jquery 1.8 and jquery timeago are only used in tests.

re split2, I have removed it, there's no reference to it in the code base.

I don't understand why we have /app/assets/javascripts/shared and /vendor/assets/javascript.
It would be good to merge these two under /app/assets/javascripts/shared.

mkllnk · 2018-08-20T02:04:56Z

Why not in vendor/assets? Seems to be the Rails convention: https://guides.rubyonrails.org/asset_pipeline.html#asset-organization

daniellemoorhead · 2018-08-20T06:58:41Z

@Matt-Yorkley could you take a look at this and approve pls? That way we can try and merge it and get it out of the way :)

daniellemoorhead · 2018-08-20T06:59:22Z

@luisramos0 can I assume this one doesn't need to be tested by Myriam or Rachel or Sally?

I put the 'pr-no-test' label on it, it can be removed if it does actually need to be tested :)

mkllnk · 2018-08-20T07:48:00Z

It should still be tested. We think that we are removing unused code, but sometimes we make mistakes.

luisramos0 · 2018-08-20T08:35:59Z

yes, needs to be tested. we need to validate the checkout process.

Matt-Yorkley · 2018-08-21T13:26:28Z

Looks good, I hope we can get rid of the split2 file without any problems.

luisramos0 · 2018-08-21T14:43:31Z

all right, I moved it to test ready. needs staging.

daniellemoorhead · 2018-08-22T05:46:25Z

I think we really need the french staging server so we have 3 european options going at the same time. There are so many things to test, it will help with that. @luisramos0 if you can try and figure out why it's not working (ping @myriamboure FYI) and get access to it I think this will be a great help in clearing out the testing backlog.

luisramos0 · 2018-08-27T12:17:53Z

ok, stagingFR is working now. I'll learn how to stage and stage this one there today or tomorrow.

mkllnk · 2018-08-31T07:07:03Z

Staged on https://staging1.openfood.com.au/.

mkllnk · 2018-09-03T01:52:52Z

After deploying this, we got the following exception on staging:

 ActionView::Template::Error · darkswarm/all_split2.css isn't precompiled

gems/actionpack-3.2.22.5/lib/sprockets/helpers/rails_helper.rb:142:in `digest_for': darkswarm/all_split2.css isn't precompiled (ActionView::Template::Error)
    from gems/actionpack-3.2.22.5/lib/sprockets/helpers/rails_helper.rb:151:in `rewrite_asset_path'
    from gems/actionpack-3.2.22.5/lib/action_view/asset_paths.rb:27:in `compute_public_path'
    from gems/actionpack-3.2.22.5/lib/sprockets/helpers/rails_helper.rb:56:in `asset_path'
    from gems/actionpack-3.2.22.5/lib/sprockets/helpers/rails_helper.rb:49:in `block in stylesheet_link_tag'
    from gems/actionpack-3.2.22.5/lib/sprockets/helpers/rails_helper.rb:43:in `collect'
    from gems/actionpack-3.2.22.5/lib/sprockets/helpers/rails_helper.rb:43:in `stylesheet_link_tag'
    from gems/css_splitter-0.4.5/app/helpers/css_splitter/application_helper.rb:18:in `block in split_stylesheet_link_tag'
    from gems/css_splitter-0.4.5/app/helpers/css_splitter/application_helper.rb:7:in `map'
    from gems/css_splitter-0.4.5/app/helpers/css_splitter/application_helper.rb:7:in `split_stylesheet_link_tag'
    from app/views/layouts/darkswarm.html.haml:20:in `_ab5644e7ce6d8a4a7df3eb77aaf8a69f'
    from gems/actionpack-3.2.22.5/lib/action_view/template.rb:145:in `block in render'
    from gems/skylight-1.6.1/lib/skylight/vendor/active_support/notifications.rb:161:in `instrument'
    from gems/actionpack-3.2.22.5/lib/action_view/template.rb:143:in `render'
    from bundler/gems/deface-1110a1336252/lib/deface/action_view_extensions.rb:41:in `render'
    from gems/actionpack-3.2.22.5/lib/action_view/renderer/template_renderer.rb:60:in `render_with_layout'
    from gems/skylight-1.6.1/lib/skylight/probes/action_view.rb:30:in `block in render_with_layout'
    from gems/actionpack-3.2.22.5/lib/action_view/renderer/abstract_renderer.rb:38:in `block in instrument'
    from gems/skylight-1.6.1/lib/skylight/vendor/active_support/notifications.rb:159:in `block in instrument'
    from gems/skylight-1.6.1/lib/skylight/vendor/active_support/notifications/instrumenter.rb:20:in `instrument'
    from gems/skylight-1.6.1/lib/skylight/vendor/active_support/notifications.rb:159:in `instrument'
    from gems/actionpack-3.2.22.5/lib/action_view/renderer/abstract_renderer.rb:38:in `instrument'
    from gems/skylight-1.6.1/lib/skylight/probes/action_view.rb:29:in `render_with_layout'
    from gems/actionpack-3.2.22.5/lib/action_view/renderer/template_renderer.rb:46:in `render_template'
    from gems/actionpack-3.2.22.5/lib/action_view/renderer/template_renderer.rb:18:in `render'
    from gems/actionpack-3.2.22.5/lib/action_view/renderer/renderer.rb:43:in `render_template'
    from gems/actionpack-3.2.22.5/lib/action_view/renderer/renderer.rb:24:in `render'
    from gems/actionpack-3.2.22.5/lib/abstract_controller/rendering.rb:111:in `_render_template'
    from gems/actionpack-3.2.22.5/lib/action_controller/metal/streaming.rb:225:in `_render_template'
    from gems/actionpack-3.2.22.5/lib/abstract_controller/rendering.rb:104:in `render_to_body'
    from gems/actionpack-3.2.22.5/lib/action_controller/metal/renderers.rb:28:in `render_to_body'
    from gems/actionpack-3.2.22.5/lib/action_controller/metal/compatibility.rb:50:in `render_to_body'
    from gems/actionpack-3.2.22.5/lib/abstract_controller/rendering.rb:89:in `render'
    from gems/actionpack-3.2.22.5/lib/action_controller/metal/rendering.rb:16:in `render'
    from gems/actionpack-3.2.22.5/lib/action_controller/metal/instrumentation.rb:40:in `block (2 levels) in render'
    from gems/activesupport-3.2.22.5/lib/active_support/core_ext/benchmark.rb:5:in `block in ms'
    from /home/openfoodweb/.rbenv/versions/2.1.5/lib/ruby/2.1.0/benchmark.rb:294:in `realtime'
    from gems/activesupport-3.2.22.5/lib/active_support/core_ext/benchmark.rb:5:in `ms'
    from gems/actionpack-3.2.22.5/lib/action_controller/metal/instrumentation.rb:40:in `block in render'
    from gems/actionpack-3.2.22.5/lib/action_controller/metal/instrumentation.rb:83:in `cleanup_view_runtime'

Caused by: gems/actionpack-3.2.22.5/lib/sprockets/helpers/rails_helper.rb:142:in `digest_for': darkswarm/all_split2.css isn't precompiled (Sprockets::Helpers::RailsHelper::AssetPaths::AssetNotPrecompiledError)
    from gems/actionpack-3.2.22.5/lib/sprockets/helpers/rails_helper.rb:151:in `rewrite_asset_path'
    from gems/actionpack-3.2.22.5/lib/action_view/asset_paths.rb:27:in `compute_public_path'
    from gems/actionpack-3.2.22.5/lib/sprockets/helpers/rails_helper.rb:56:in `asset_path'
    from gems/actionpack-3.2.22.5/lib/sprockets/helpers/rails_helper.rb:49:in `block in stylesheet_link_tag'
    from gems/actionpack-3.2.22.5/lib/sprockets/helpers/rails_helper.rb:43:in `collect'
    from gems/actionpack-3.2.22.5/lib/sprockets/helpers/rails_helper.rb:43:in `stylesheet_link_tag'
    from gems/css_splitter-0.4.5/app/helpers/css_splitter/application_helper.rb:18:in `block in split_stylesheet_link_tag'
    from gems/css_splitter-0.4.5/app/helpers/css_splitter/application_helper.rb:7:in `map'
    from gems/css_splitter-0.4.5/app/helpers/css_splitter/application_helper.rb:7:in `split_stylesheet_link_tag'
    from app/views/layouts/darkswarm.html.haml:20:in `_ab5644e7ce6d8a4a7df3eb77aaf8a69f'
    from gems/actionpack-3.2.22.5/lib/action_view/template.rb:145:in `block in render'
    from gems/skylight-1.6.1/lib/skylight/vendor/active_support/notifications.rb:161:in `instrument'
    from gems/actionpack-3.2.22.5/lib/action_view/template.rb:143:in `render'
    from bundler/gems/deface-1110a1336252/lib/deface/action_view_extensions.rb:41:in `render'
    from gems/actionpack-3.2.22.5/lib/action_view/renderer/template_renderer.rb:60:in `render_with_layout'
    from gems/skylight-1.6.1/lib/skylight/probes/action_view.rb:30:in `block in render_with_layout'
    from gems/actionpack-3.2.22.5/lib/action_view/renderer/abstract_renderer.rb:38:in `block in instrument'
    from gems/skylight-1.6.1/lib/skylight/vendor/active_support/notifications.rb:159:in `block in instrument'
    from gems/skylight-1.6.1/lib/skylight/vendor/active_support/notifications/instrumenter.rb:20:in `instrument'
    from gems/skylight-1.6.1/lib/skylight/vendor/active_support/notifications.rb:159:in `instrument'
    from gems/actionpack-3.2.22.5/lib/action_view/renderer/abstract_renderer.rb:38:in `instrument'
    from gems/skylight-1.6.1/lib/skylight/probes/action_view.rb:29:in `render_with_layout'
    from gems/actionpack-3.2.22.5/lib/action_view/renderer/template_renderer.rb:46:in `render_template'
    from gems/actionpack-3.2.22.5/lib/action_view/renderer/template_renderer.rb:18:in `render'
    from gems/actionpack-3.2.22.5/lib/action_view/renderer/renderer.rb:43:in `render_template'
    from gems/actionpack-3.2.22.5/lib/action_view/renderer/renderer.rb:24:in `render'
    from gems/actionpack-3.2.22.5/lib/abstract_controller/rendering.rb:111:in `_render_template'
    from gems/actionpack-3.2.22.5/lib/action_controller/metal/streaming.rb:225:in `_render_template'
    from gems/actionpack-3.2.22.5/lib/abstract_controller/rendering.rb:104:in `render_to_body'
    from gems/actionpack-3.2.22.5/lib/action_controller/metal/renderers.rb:28:in `render_to_body'
    from gems/actionpack-3.2.22.5/lib/action_controller/metal/compatibility.rb:50:in `render_to_body'
    from gems/actionpack-3.2.22.5/lib/abstract_controller/rendering.rb:89:in `render'
    from gems/actionpack-3.2.22.5/lib/action_controller/metal/rendering.rb:16:in `render'
    from gems/actionpack-3.2.22.5/lib/action_controller/metal/instrumentation.rb:40:in `block (2 levels) in render'
    from gems/activesupport-3.2.22.5/lib/active_support/core_ext/benchmark.rb:5:in `block in ms'
    from /home/openfoodweb/.rbenv/versions/2.1.5/lib/ruby/2.1.0/benchmark.rb:294:in `realtime'
    from gems/activesupport-3.2.22.5/lib/active_support/core_ext/benchmark.rb:5:in `ms'
    from gems/actionpack-3.2.22.5/lib/action_controller/metal/instrumentation.rb:40:in `block in render'
    from gems/actionpack-3.2.22.5/lib/action_controller/metal/instrumentation.rb:83:in `cleanup_view_runtime'

luisramos0 · 2018-09-03T08:52:53Z

yeah...
I have put the split2 thing back in application.rb preconpile list.
If we do something with it, we can do it in PR #2637
I think we should go forward with this PR without it.

luisramos0 · 2018-09-06T20:12:47Z

This needs to be staged again.

luisramos0 · 2018-09-13T10:35:40Z

Back to staging, this time on https://staging.openfoodfrance.org/

RachL · 2018-09-17T09:32:59Z

Front office looks as usual on both desktop and mobile. Guest and login checkout worked good as well.
Testing notes:
https://docs.google.com/document/d/1_tMLQZ2DHIlmbkU-tWlSgAo_bhbDAEYGRn94JBxXeIk/edit#

luisramos0 self-assigned this Aug 11, 2018

HugsDaniel approved these changes Aug 14, 2018

View reviewed changes

mkllnk approved these changes Aug 17, 2018

View reviewed changes

luisramos0 force-pushed the delete_dead_store branch from 703b7e9 to 270eefb Compare August 18, 2018 16:16

daniellemoorhead requested a review from Matt-Yorkley August 20, 2018 06:58

daniellemoorhead added the pr-no-test label Aug 20, 2018

luisramos0 removed the pr-no-test label Aug 20, 2018

Matt-Yorkley approved these changes Aug 21, 2018

View reviewed changes

luisramos0 mentioned this pull request Aug 23, 2018

Duplicate CSS definitions from use of css_splitter #1361

Closed

mkllnk added the pr-staged-au staging.openfoodnetwork.org.au label Aug 31, 2018

removed assets related to spree store: dead code

14d526e

luisramos0 force-pushed the delete_dead_store branch from 270eefb to 14d526e Compare September 3, 2018 08:50

sigmundpetersen removed the pr-staged-au staging.openfoodnetwork.org.au label Sep 3, 2018

mkllnk approved these changes Sep 4, 2018

View reviewed changes

daniellemoorhead added the tech debt label Sep 7, 2018

luisramos0 added the pr-staged-fr staging.coopcircuits.fr label Sep 13, 2018

RachL self-assigned this Sep 17, 2018

RachL removed the pr-staged-fr staging.coopcircuits.fr label Sep 17, 2018

sauloperez merged commit 0520f21 into openfoodfoundation:master Sep 17, 2018

luisramos0 deleted the delete_dead_store branch September 17, 2018 17:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

removed assets related to spree store: dead code #2540

removed assets related to spree store: dead code #2540

luisramos0 commented Aug 11, 2018 •

edited

Loading

mkllnk left a comment

luisramos0 commented Aug 18, 2018 •

edited

Loading

mkllnk commented Aug 20, 2018

daniellemoorhead commented Aug 20, 2018

daniellemoorhead commented Aug 20, 2018 •

edited

Loading

mkllnk commented Aug 20, 2018

luisramos0 commented Aug 20, 2018

Matt-Yorkley commented Aug 21, 2018

luisramos0 commented Aug 21, 2018

daniellemoorhead commented Aug 22, 2018

luisramos0 commented Aug 27, 2018 •

edited

Loading

mkllnk commented Aug 31, 2018

mkllnk commented Sep 3, 2018

luisramos0 commented Sep 3, 2018

luisramos0 commented Sep 6, 2018

luisramos0 commented Sep 13, 2018

RachL commented Sep 17, 2018

removed assets related to spree store: dead code #2540

removed assets related to spree store: dead code #2540

Conversation

luisramos0 commented Aug 11, 2018 • edited Loading

What? Why?

What should we test?

Release notes

mkllnk left a comment

Choose a reason for hiding this comment

luisramos0 commented Aug 18, 2018 • edited Loading

mkllnk commented Aug 20, 2018

daniellemoorhead commented Aug 20, 2018

daniellemoorhead commented Aug 20, 2018 • edited Loading

mkllnk commented Aug 20, 2018

luisramos0 commented Aug 20, 2018

Matt-Yorkley commented Aug 21, 2018

luisramos0 commented Aug 21, 2018

daniellemoorhead commented Aug 22, 2018

luisramos0 commented Aug 27, 2018 • edited Loading

mkllnk commented Aug 31, 2018

mkllnk commented Sep 3, 2018

luisramos0 commented Sep 3, 2018

luisramos0 commented Sep 6, 2018

luisramos0 commented Sep 13, 2018

RachL commented Sep 17, 2018

luisramos0 commented Aug 11, 2018 •

edited

Loading

luisramos0 commented Aug 18, 2018 •

edited

Loading

daniellemoorhead commented Aug 20, 2018 •

edited

Loading

luisramos0 commented Aug 27, 2018 •

edited

Loading