search

envato-archive / guide

Document your application with a living component library and styleguide
https://rubygems.org/gems/guide
MIT License
8 stars 3 forks source link

Updated controller to render 404 on partial node path #84

Closed geoffevason closed 4 years ago

geoffevason commented 6 years ago

If a partial path is visited, that currently triggers a NoMethod error and returns a 500

A valid url is: https://market.styleguide.envato.com/scenario/with_hosted_promo/html/for/structures/user/downloads/download is a valid url.

A partial path that fails is: https://market.styleguide.envato.com/scenario/with_hosted_promo/html/for/structures/user/downloads

The partial path should return a 404 instead of a 500 error.

This change updates the scenario to catch this error and render a 404. It also bumps the gem version.

geoffevason commented 6 years ago

@johnsyweb I had made both of those adjustments. Thanks.

jordanlewiz commented 6 years ago

Ping @lukearndt 👋

ppj commented 6 years ago

The reasoning and the code changes look 👌to me. I guess this is open-sourced so would be good to get @lukearndt's thought's on it too?

johnsyweb commented 5 years ago

Is this still needed?

geoffevason commented 5 years ago

I'll revisit this during hack week. I think it's still worthwhile as it caused a callout at least once.

On Thu, Apr 4, 2019 at 5:17 AM Zubin Henner notifications@github.com wrote:

@zubin commented on this pull request.

In Gemfile https://github.com/envato/guide/pull/84#discussion_r271870268 :

@@ -1,3 +1,5 @@ source 'https://rubygems.org'

+gem 'rails-controller-testing'

Should this go in guide.gemspec as a development dependency? Otherwise it will load in prod.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/envato/guide/pull/84#discussion_r271870268, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAhbVjr8EJTZoE_pk72abHpiloj119iks5vdPA1gaJpZM4VhCJJ .

--

Geoff Evason

geoff.evason@envato.com geoff.evason@envato.com

envato.com http://www.envato.com/

Envato Pty Ltd. 121 King Street, Melbourne, VIC 3000, Australia

Follow Envato on: Twitter https://twitter.com/envato | Linkedin http://www.linkedin.com/company/envato

This email and its attachments may contain confidential information. If you have received this email in error, I'd appreciate you letting me know and deleting it.

zubin commented 4 years ago

@geoffevason Are you planning to wrap this up soon? Would you like some help?

geoffevason commented 4 years ago

@zubin TBH I'm not sure when I'll get time to regain the context here and finish it. If you're willing to finish it off, please do. Otherwise we can close it.

zubin commented 4 years ago

@geoffevason No worries, will try to squeeze it in sometime soon.

johnsyweb commented 4 years ago

GitHub just reminded me that I was mentioned in this PR. What a blast from the past. Should we close it?

zubin commented 4 years ago

GitHub just reminded me that I was mentioned in this PR. What a blast from the past. Should we close it?

I'm not sure how important this issue is, or whether there are plans to stop using this gem, but I had a go at picking up this PR a while back but it was far from straightforward!

geoffevason commented 4 years ago

I'm so far removed from remembering the context here that I think it's probably not going to move forward. Closing.