Skip to content

Commit

Permalink
[Feature] [WEBSITE-1239] 404 page redesign (#2253)
Browse files Browse the repository at this point in the history
* 404 page redesign

* Fix css format

* Fix link destination

* Remove old 404 file

* Add new controller and helper

* Add new 404 template

* Add new route to the 404 page

* Tell rails to use tempaltes

* Error pages sass

* Catch errors with custom error pages yielded w/ application layout

* update styling and put content inside .container

* Optimize calls + raise exception for development mode

* Return back static 500 page

* Make rails app available to catch RoutingError (raised by ActionDispatch)

Ref: rails/rails#671

* Fix not found routes spec

* Fix unmactched routes catch

* Fix specs
  • Loading branch information
albertocorrea authored and Gustavo Robles committed Mar 7, 2017
1 parent 2a2768f commit 6b7fde3
Show file tree
Hide file tree
Showing 20 changed files with 188 additions and 168 deletions.
1 change: 1 addition & 0 deletions app/assets/stylesheets/application_redesign.css.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
26 changes: 26 additions & 0 deletions app/assets/stylesheets/redesign/_error.scss
Original file line number Diff line number Diff line change
@@ -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;
}

}
16 changes: 16 additions & 0 deletions app/assets/stylesheets/redesign/generic/_helpers.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 2 additions & 6 deletions app/assets/stylesheets/redesign/pages/_why-us.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -63,7 +59,7 @@
}

}

}

}
1 change: 1 addition & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
49 changes: 49 additions & 0 deletions app/controllers/concerns/errors.rb
Original file line number Diff line number Diff line change
@@ -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
9 changes: 0 additions & 9 deletions app/controllers/errors/invalid_format_controller.rb

This file was deleted.

14 changes: 0 additions & 14 deletions app/controllers/errors/mysterious_route_controller.rb

This file was deleted.

2 changes: 1 addition & 1 deletion app/controllers/products/collections_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/spree/omniauth_callbacks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions app/views/errors/404.html.slim
Original file line number Diff line number Diff line change
@@ -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
| <em>Sorry,</em> 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.
15 changes: 15 additions & 0 deletions app/views/errors/422.html.slim
Original file line number Diff line number Diff line change
@@ -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.
18 changes: 18 additions & 0 deletions app/views/errors/_analytics_javascript.html.slim
Original file line number Diff line number Diff line change
@@ -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}");
17 changes: 7 additions & 10 deletions config/routes.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
FameAndPartners::Application.routes.draw do

############################
# Devise Omniauth Workaround
############################
Expand Down Expand Up @@ -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
#########
Expand Down Expand Up @@ -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
57 changes: 0 additions & 57 deletions public/404.html

This file was deleted.

26 changes: 0 additions & 26 deletions public/422.html

This file was deleted.

27 changes: 0 additions & 27 deletions spec/controllers/mysterious_route_controller_spec.rb

This file was deleted.

3 changes: 2 additions & 1 deletion spec/controllers/products/collections_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 6b7fde3

Please sign in to comment.