diff --git a/app/assets/stylesheets/application_redesign.css.scss b/app/assets/stylesheets/application_redesign.css.scss index 13c6d4966a..8d39de1629 100644 --- a/app/assets/stylesheets/application_redesign.css.scss +++ b/app/assets/stylesheets/application_redesign.css.scss @@ -168,6 +168,7 @@ z-index levels @import 'redesign/style_session'; @import 'redesign/tables'; @import 'redesign/pages'; +@import 'redesign/error'; // Checkout @import 'redesign/pages/page-checkout'; diff --git a/app/assets/stylesheets/redesign/_error.scss b/app/assets/stylesheets/redesign/_error.scss new file mode 100644 index 0000000000..5fa9ffca7f --- /dev/null +++ b/app/assets/stylesheets/redesign/_error.scss @@ -0,0 +1,26 @@ +// Error Pages (Errors 404, 500 etc) +.error-page { + margin: 100px auto; + + h1 { + font-size: 3.8em; + margin-top: 0; + } + + h2 { + font-size: 3.2rem; + margin: 50px 0; + } + + p { + font-size: 2rem; + font-weight: 300; + line-height: 150%; + margin: 0; + } + + a { + @extend .link-underline; + } + +} diff --git a/app/assets/stylesheets/redesign/generic/_helpers.scss b/app/assets/stylesheets/redesign/generic/_helpers.scss index 774959717c..cd96132d5c 100644 --- a/app/assets/stylesheets/redesign/generic/_helpers.scss +++ b/app/assets/stylesheets/redesign/generic/_helpers.scss @@ -202,6 +202,22 @@ } +.thin-line { + + &--bottom::after { + content: ""; + padding-bottom: 50px; + border-bottom: 1px solid $c-neutral-black; + width: 90px; + display: block; + } + + &--alignCenter::after { + margin: 0 auto; + } + +} + @media (min-width: $bootstrap-md-width) { .no-margin-md { diff --git a/app/assets/stylesheets/redesign/pages/_why-us.scss b/app/assets/stylesheets/redesign/pages/_why-us.scss index 30d4d6e2b4..1ba2cca66f 100644 --- a/app/assets/stylesheets/redesign/pages/_why-us.scss +++ b/app/assets/stylesheets/redesign/pages/_why-us.scss @@ -33,12 +33,8 @@ } .heading-thin-line-bottom::after { - content: ""; - padding-bottom: 40px; + @extend .thin-line--bottom; margin-bottom: 40px; - border-bottom: 1px solid $c-neutral-black; - width: 90px; - display: block; } @media (max-width: $bootstrap-md-width - 1px) { @@ -63,7 +59,7 @@ } } - + } } diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 96ac84e399..2bbf1ca2c8 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -12,6 +12,7 @@ class ApplicationController < ActionController::Base include Concerns::UserCampaignable include Concerns::AutomaticDiscount include Concerns::Moodboards + include Concerns::Errors # Marketing related concerns include Marketing::Gtm::Controller::Container diff --git a/app/controllers/concerns/errors.rb b/app/controllers/concerns/errors.rb new file mode 100644 index 0000000000..ffade8792c --- /dev/null +++ b/app/controllers/concerns/errors.rb @@ -0,0 +1,49 @@ +module Concerns + module Errors + extend ActiveSupport::Concern + + MYSTERIOUS_ROUTES = [ 'undefined', '/au/undefined', '1000668' ].freeze + + included do + rescue_from ActiveRecord::RecordInvalid, with: -> { render_error(code: 422) } + rescue_from ActiveRecord::RecordNotFound, with: -> { render_error(code: 404) } + rescue_from ActionController::UnknownController, with: -> { render_error(code: 404) } + rescue_from ActionController::RoutingError, with: -> { render_error(code: 404) } + rescue_from AbstractController::ActionNotFound, with: -> { render_error(code: 404) } + end + + def non_matching_request + path = params.fetch(:path, '') + format = params.fetch(:format, '') + + if path.match(/\.php$/) || format.eql?('php') + head :not_acceptable, layout: false + + elsif MYSTERIOUS_ROUTES.any? { |v| v.match(path) } + data = request.env.select {|key,_| key.upcase == key } + NewRelic::Agent.record_custom_event('UndefinedURL', data) + render text: 'Not Found', status: :not_found + + else + # NOTE: Alexey Bobyrev 14 Jan 2017 + # Raise error here to make it visible for application#rescue_from + # Ref: https://github.com/rails/rails/issues/671 + raise ActionController::RoutingError.new(path) + end + end + + protected + + def render_error(code:) + if Rails.application.config.consider_all_requests_local + raise + else + respond_to do |format| + format.html { render "errors/#{code}", status: code } + format.all { render nothing: true, status: code } + end + end + end + + end +end diff --git a/app/controllers/errors/invalid_format_controller.rb b/app/controllers/errors/invalid_format_controller.rb deleted file mode 100644 index 743ac229fe..0000000000 --- a/app/controllers/errors/invalid_format_controller.rb +++ /dev/null @@ -1,9 +0,0 @@ -module Errors - class InvalidFormatController < ActionController::Base - layout false - - def capture_php - head :not_acceptable - end - end -end diff --git a/app/controllers/errors/mysterious_route_controller.rb b/app/controllers/errors/mysterious_route_controller.rb deleted file mode 100644 index 3de9e5ecc9..0000000000 --- a/app/controllers/errors/mysterious_route_controller.rb +++ /dev/null @@ -1,14 +0,0 @@ -module Errors - class MysteriousRouteController < ActionController::Base - layout false - - # Captures and logs hits to weird URLs we keep seeing in the logs. - # '/undefined' '/au/undefined' - def undefined - data = request.env.select {|key,_| key.upcase == key } - - NewRelic::Agent.record_custom_event("UndefinedURL", data) - render :text => 'Not Found', :status => :not_found - end - end -end diff --git a/app/controllers/products/collections_controller.rb b/app/controllers/products/collections_controller.rb index 238bfc93f3..e25dc1f284 100644 --- a/app/controllers/products/collections_controller.rb +++ b/app/controllers/products/collections_controller.rb @@ -114,7 +114,7 @@ def collection_template if page.page_is_lookbook? || @collection_options page.template_path else - {file: 'public/404', layout: false, status: :not_found} + raise ActiveRecord::RecordNotFound end end diff --git a/app/controllers/spree/omniauth_callbacks_controller.rb b/app/controllers/spree/omniauth_callbacks_controller.rb index 306476432c..1fb01baecb 100644 --- a/app/controllers/spree/omniauth_callbacks_controller.rb +++ b/app/controllers/spree/omniauth_callbacks_controller.rb @@ -140,7 +140,7 @@ def failure end def passthru - render :file => "#{Rails.root}/public/404", :formats => [:html], :status => 404, :layout => false + raise ActiveRecord::RecordNotFound end def auth_hash diff --git a/app/views/errors/404.html.slim b/app/views/errors/404.html.slim new file mode 100644 index 0000000000..c3e8480e15 --- /dev/null +++ b/app/views/errors/404.html.slim @@ -0,0 +1,15 @@ +- @title = "The page you were looking for doesn't exist (404)" + += render 'errors/analytics_javascript', locals: { code: 404, message: 'not found'} + +.container + .col-xs-12 + .error-page.text-center + h1 + | Looking for something? + h2 + | Sorry, this page doesn't exist. + p + | Try our #{link_to "homepage", root_path} to see what's new and start customizing. + p.thin-line--bottom.thin-line--alignCenter + | Still can't find what you're looking for? #{link_to "E-mail us", 'mailto:team@fameandpartners.com'} at team@fameandpartners.com. diff --git a/app/views/errors/422.html.slim b/app/views/errors/422.html.slim new file mode 100644 index 0000000000..ad0cc64e2a --- /dev/null +++ b/app/views/errors/422.html.slim @@ -0,0 +1,15 @@ +- @title = "The change you wanted was rejected (422)" + += render 'errors/analytics_javascript', locals: { code: 422, message: 'unprocessable entity'} + +.container + .col-xs-12 + .error-page.text-center + h1 + | The change you wanted was rejected. + h2 + | Maybe you tried to change something you didn't have access to. + p + | Try our #{link_to "homepage", root_path} to see what's new and start customizing. + p.thin-line--bottom.thin-line--alignCenter + | Still can't find what you're looking for? #{link_to "E-mail us", 'mailto:team@fameandpartners.com'} at team@fameandpartners.com. diff --git a/app/views/errors/_analytics_javascript.html.slim b/app/views/errors/_analytics_javascript.html.slim new file mode 100644 index 0000000000..4723f31529 --- /dev/null +++ b/app/views/errors/_analytics_javascript.html.slim @@ -0,0 +1,18 @@ +- code = local_assigns[:code] +- message = local_assigns[:message] + +javascript: + (function (i, s, o, g, r, a, m) { + i['GoogleAnalyticsObject'] = r; + i[r] = i[r] || function () { + (i[r].q = i[r].q || []).push(arguments) + }, i[r].l = 1 * new Date(); + a = s.createElement(o), + m = s.getElementsByTagName(o)[0]; + a.async = 1; + a.src = g; + m.parentNode.insertBefore(a, m) + })(window, document, 'script', '//www.google-analytics.com/analytics.js', 'ga'); + ga('create', 'UA-41247818-1', 'auto'); + ga('send', 'pageview'); + ga('send', 'event', "#{code}", "#{message}"); diff --git a/config/routes.rb b/config/routes.rb index 381c177e97..95c07173f9 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,5 +1,4 @@ FameAndPartners::Application.routes.draw do - ############################ # Devise Omniauth Workaround ############################ @@ -503,14 +502,6 @@ end end - ################# - # Mysterious URLs - ################# - - get '/undefined', to: 'errors/mysterious_route#undefined' - get '/au/undefined', to: 'errors/mysterious_route#undefined' - get '/1000668', to: 'errors/mysterious_route#undefined' - ######### # Widgets ######### @@ -624,6 +615,12 @@ mount Revolution::Engine => '/' mount WeddingAtelier::Engine, at: '/wedding-atelier' +end - match '*path', to: 'errors/invalid_format#capture_php', constraints: lambda { |request| request.path[/\.php$/] } +# NOTE: Alexey Bobyrev 14 Feb 2017 +# Method append used here to handle all request directly right after defined ones (including engines) +FameAndPartners::Application.routes.append do + # NOTE: Alexey Bobyrev 14 Jan 2017 + # Any other routes are handled here (as ActionDispatch prevents RoutingError from hitting ApplicationController#rescue_action) + match '*path', to: 'application#non_matching_request', as: 'routing_error' end diff --git a/public/404.html b/public/404.html deleted file mode 100644 index aa201306d2..0000000000 --- a/public/404.html +++ /dev/null @@ -1,57 +0,0 @@ - - - - The page you were looking for doesn't exist (404) - - - - - -
- -

So sorry we can't find that page.

-

In the meantime, here is a $20 voucher to make up for your time - just use this code @checkout: OOPS20

-

- Go to the homepage -

-

- If you would like some further assistance, feel free to email: -

-

- team@fameandpartners.com -

-
- - - diff --git a/public/422.html b/public/422.html deleted file mode 100644 index 83660ab187..0000000000 --- a/public/422.html +++ /dev/null @@ -1,26 +0,0 @@ - - - - The change you wanted was rejected (422) - - - - - -
-

The change you wanted was rejected.

-

Maybe you tried to change something you didn't have access to.

-
- - diff --git a/spec/controllers/mysterious_route_controller_spec.rb b/spec/controllers/mysterious_route_controller_spec.rb deleted file mode 100644 index 67a71a202d..0000000000 --- a/spec/controllers/mysterious_route_controller_spec.rb +++ /dev/null @@ -1,27 +0,0 @@ -require 'rails_helper' - -RSpec.describe Errors::MysteriousRouteController, type: :controller do - describe "GET #undefined" do - it "returns not found" do - get :undefined - expect(response).to have_http_status(:not_found) - end - end -end - -RSpec.describe Errors::MysteriousRouteController, type: :routing do - - let(:controller) { { controller: 'errors/mysterious_route', action: 'undefined'} } - - it "/undefined" do - expect(get: "/undefined").to route_to(controller) - end - - it "/au/undefined" do - expect(get: "/au/undefined").to route_to(controller) - end - - it "/1000668'" do - expect(get: '/1000668').to route_to(controller) - end -end diff --git a/spec/controllers/products/collections_controller_spec.rb b/spec/controllers/products/collections_controller_spec.rb index 750067d079..73385eadbe 100644 --- a/spec/controllers/products/collections_controller_spec.rb +++ b/spec/controllers/products/collections_controller_spec.rb @@ -43,11 +43,12 @@ describe 'returns a 404 status' do before(:each) do allow(controller).to receive(:parse_permalink).and_return(nil) + allow(Rails.application.config).to receive(:consider_all_requests_local).and_return(false) end it 'when querying a inexistent permalink' do get :show, permalink: 'nothing' - expect(response).to render_template(file: 'public/404', layout: false) + expect(response).to render_template(file: 'errors/404', layout: 'redesign/application') expect(response).to have_http_status(:not_found) end end diff --git a/spec/routing/not_found_routes_spec.rb b/spec/routing/not_found_routes_spec.rb new file mode 100644 index 0000000000..4899fa0c4c --- /dev/null +++ b/spec/routing/not_found_routes_spec.rb @@ -0,0 +1,34 @@ +require 'spec_helper' + +describe 'Not found requests', type: :request do + + describe 'routes to application#non_matching_request' do + before(:each) do + allow(Rails.application.config).to receive(:consider_all_requests_local).and_return(false) + end + + it 'with *.php' do + get '/abc.php' + + expect(response).to render_template(nil) + expect(response).to have_http_status(406) + end + + it 'with any non-existent page' do + get '/abc' + + expect(response.body).to match("this page doesn't exist") + expect(response.body).to match('MY ACCOUNT') + expect(response.body).to match('Fame & Partners. All rights reserved.') + expect(response).to have_http_status(404) + end + + it 'with an undefined page' do + get '/undefined' + + expect(response.body).to match('Not Found') + expect(response).to have_http_status(404) + end + end + +end diff --git a/spec/routing/route_php_spec.rb b/spec/routing/route_php_spec.rb deleted file mode 100644 index 576d3100d4..0000000000 --- a/spec/routing/route_php_spec.rb +++ /dev/null @@ -1,16 +0,0 @@ -require 'spec_helper' - -describe 'Route `.php` requests', type: :routing do - let(:controller) { {controller: 'errors/invalid_format', action: 'capture_php', path: 'abc', format: 'php'} } - - it 'routes to errors/invalid_format#capture_php' do - expect(get: '/abc.php').to route_to(controller) - expect(post: '/abc.php').to route_to(controller) - end - - it 'should not route' do - expect(get: '/abc').not_to be_routable - expect(post: '/abc').not_to be_routable - expect(get: '/abc.js').not_to be_routable - end -end