piotrmurach / loaf

Manages and displays breadcrumb trails in Rails app - lean & mean.
MIT License
407 stars 24 forks source link

adds nil guard and clear error messages to Crumb initialization. #15

Closed dmvt closed 6 years ago

dmvt commented 6 years ago

Sorry for the long hiatus after the last PR. Based on your feedback, I've added some error handling to Loaf::Crumb#initialize.

ref: https://github.com/piotrmurach/loaf/pull/11

This error:

  1) Customer signs in via the company sign in page having a customer account with the company with an instance param is taken to the member instance details page
     Failure/Error: - breadcrumb_trail crumb_length: 60 do |name, url, current|

     ActionView::Template::Error:
       arguments passed to url_for can't be handled. Please require routes or provide your own implementation
     # /Users/dan/grillwork/loaf/lib/loaf/view_extensions.rb:57:in `block in breadcrumb_trail'
     # /Users/dan/grillwork/loaf/lib/loaf/view_extensions.rb:55:in `each'
     # /Users/dan/grillwork/loaf/lib/loaf/view_extensions.rb:55:in `breadcrumb_trail'
     # ./app/views/layouts/member_area.html.slim:52:in `_app_views_layouts_member_area_html_slim__290762266190123875_70211764360280'
     # ./app/controllers/members/class_details_controller.rb:53:in `instance_details'
     # ./lib/subdomain/middleware.rb:42:in `call'
     # ./lib/auth/middleware.rb:8:in `call'
     # ./spec/features/auth/customer_signs_in_spec.rb:20:in `block (5 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # ArgumentError:
     #   arguments passed to url_for can't be handled. Please require routes or provide your own implementation
     #   /Users/dan/grillwork/loaf/lib/loaf/view_extensions.rb:57:in `block in breadcrumb_trail'

Becomes this error:

  1) Customer signs in via the company sign in page having a customer account with the company with an instance param is taken to the member instance details page
     Failure/Error: breadcrumb("... name here ...", nil)

     ArgumentError:
       second argument, `url`, cannot be nil
     # /Users/dan/grillwork/loaf/lib/loaf/crumb.rb:14:in `initialize'
     # /Users/dan/grillwork/loaf/lib/loaf/controller_extensions.rb:62:in `new'
     # /Users/dan/grillwork/loaf/lib/loaf/controller_extensions.rb:62:in `breadcrumb'
     # ./app/controllers/members/class_details_controller.rb:29:in `instance_details'
     # ./lib/subdomain/middleware.rb:42:in `call'
     # ./lib/auth/middleware.rb:8:in `call'
     # ./spec/features/auth/customer_signs_in_spec.rb:20:in `block (5 levels) in <top (required)>'

Not only is the error easier to understand, but the trace now includes the line on which the error occurred.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.03%) to 97.059% when pulling eb21a7ebe2ae740eb51da052ae5fdd86a1cbda01 on dmvt:master into 1962cc1c127980a8ae874936bf8dc1f554b705b5 on piotrmurach:master.

piotrmurach commented 6 years ago

Better late than never 😄 Thanks!

piotrmurach commented 6 years ago

Released v0.6.1 with the patch!

dmvt commented 6 years ago

:shipit:

Thanks!