Lost page identification on controller render

[superlou]

I have defined a link to a new model inside my navigation heirarchy:

events_sub_nav.item "new_event", "New event", new_event_path

This works great (mostly using simple-navigation for breadcrumbs currently) except when the form is submitted with invalid data. This causes a controller render rather than redirect_to and the browser path is now /events rather than /events/new. From what I've read, this is normal for rails, but seems to be preventing simple-navigation from identifying the page. Is there an option for defining the nav to catch this case?

[andi]

This is a known issue... It can be partially solved by using the :highlights_on option, i.e.

events_sub_nav.item "new_event", "New event", new_event_path, :highlights_on => /(\/events/new)|(\/events)/

But this - of course - does also highlight the item on a get request to /events (the index action). I currently can't see a clean way to solve this issue... Any ideas are appreciated..

[mjtko]

When this issue was logged last night, I briefly wondered if we could solve this using new_record? but haven't got any further than briefly wondering yet. :-)

[mjtko]

Hi superlou,

I've created a highlights-on-proc branch with an enhanced :highlights_on option that will now take a Proc as well as a Regexp.

I'd suggest trying something like this:

events_sub_nav.item "new_event", "New event", new_event_path, :highlights_on => lambda{@event.new_record?}

Let me know how it goes and I'll work on pulling the functionality into master and releasing a new version of simple-navigation incorporating the feature and full rspec coverage. :)

[mjtko]

BTW (and you might already know this) but assuming you are using Rails 3, modifying your Gemfile to contain a directive like the following is a good way of pointing to the development gem in place for testing with your app:

gem 'simple-navigation', '3.3.3', :path => '/Users/markt/code/simple-navigation'

[superlou]

Thanks for the quick responses!

I had actually included your branch via the following. Is this not recommended?

gem 'simple-navigation', '3.3.3',
    :git => 'https://github.com/andi/simple-navigation.git',
    :branch => 'highlights-on-proc'

For new items, it seems to be working with the slight modification to protect against nil objects:

:highlights_on => lambda{@event.new_record? if @event.present?}

The only concern I might have is what happens if you have a form with accepts_nested_parameters_for. I think that can be handled if you can use both a regex and lambda.

The last sticking point is similar, but occurs for edits (CRUD updates) that fail. I can't seem to find an equivalent to new_record? for update (not_updated? would be nice) but will keep looking.

[superlou]

Would it be possible to do something like the following to catch failed updates?

:highlights_on => [/events\/(\d)+\/edit/, lambda{@event.errors.count > 0 if @event.present?}]

It looks pretty inelegant to my newbie eye and it would be awesome to bake in functionality for new and edit so they just work. Let me know if there is anything I can do to help. I'm pretty new to ruby and rails and most of my testing has been done via Test::Unit + Shoulda unfortunately.

[mjtko]

Referencing the gem as a branch that way is fine - I just have had lesser success than some with bundler and git references in the past and tend to fall back to base principles (and, of course, this syntax is great for hacking purposes! :-)).

Glad to hear it's working for new items. FYI, another pattern to protect against nils could be:

lambda { @event && @event.new_record? }

Can you provide an example usage where accepts_nested_parameters_for fails with this test? I'm happy to incorporate an additional feature if it is necessary and can't be dealt with through a (relatively) simple Proc.

As for edits: perhaps you could make use of the changed? method?

lambda ( @event && @event.changed? }

Though this would need to be combined with a regex for the initial before changes. Hmmmm, will think a bit more about this.

[mjtko]

Ah, interleaved comments FTW. :)

Yes, I agree, that does look inelegant!

I also agree that it would be great to bake functionality in - but this is not a rails specific library and, as such, any bake-ins would need to be carefully constructed so as not to pollute core functionality with railsisms. :)

More than happy to take on board and/or collaborate any ideas you have though!

[superlou]

I like that pattern. Didn't realize boolean logic is happy to terminate early when failing a logic and.

Unfortunately, my model using accepts_nested_parameters_for is pretty busted right now and I don't think I understand the internals well enough to create a clean test case. I'll post back if I come up with something that looks good. I think the problem would be similar to the needing a proc and regex for the update case, since the nested model could make it look like you are at a different leaf than predicted. Might be a non-issue (I'll think about it some more).

I forgot about the isolation from rails, sorry. If it's a simple as a pattern for :highlights_on for new and for update, it would be cleaner to just make a helper.

:highlights_on => rails_new(@event)
:highlights_on => rails_update(@event)

or something of the like. There's probably some fancy meta-programming that could be done so the user doesn't have to remember to add the highlights manually. It doesn't seem worth creating a special rails fork of simple-navigation.

Sorry about the following Closed -> Reopened.

[mjtko]

Ok, taking a different approach to this problem, I've been playing about with defining this kind of helper Proc:

matcher = lambda do |controller,action|
  Array(controller).include?(controller_name) && Array(action).include?(action_name)
end

And using it like this:

events_sub_nav.item "new_event", "New event", new_event_path,
  :highlights_on => lambda { matcher.call('events',['new','create']) }
events_sub_nav.item "edit_event", "Edit event", edit_event_path(@event),
  :highlights_on => lambda { matcher.call('events',['edit','update']) }, :unless => lambda { @event.nil? }

The solution could be generalized further. For eg. in application_helper.rb:

  EDIT_ACTIONS = ['edit','update']
  CREATE_ACTIONS = ['new','create']

  def action_matcher(controller,action)
    controllers = Array(controller)
    actions = Array(action)
    lambda { controllers.include?(controller_name) && actions.include?(action_name) }
  end

  def edit_action_matcher(controller)
    action_matcher(controller,EDIT_ACTIONS)
  end

  def create_action_matcher(controller)
    action_matcher(controller,CREATE_ACTIONS)
  end

Then, in navigation.rb:

events_sub_nav.item "new_event", "New event", new_event_path,
  :highlights_on => create_action_matcher('events')
events_sub_nav.item "edit_event", "Edit event", edit_event_path(@event),
  :highlights_on => edit_action_matcher('events'), :unless => lambda { @event.nil? }

How does that grab you?

[superlou]

It doesn't seem to be working for me (also, small typo pluralizing actions). Just eyeballing it looks like it should work but I'll get into a bit more and report back.

[mjtko]

Ah, yeah, sorry about the (multiple!) typos, corrected in post now.

[superlou]

One more typo in action_matcher:

  EDIT_ACTIONS = ['edit','update']
  CREATE_ACTIONS = ['new','create']

  def action_matcher(controller,action)
    controllers = Array(controller)
    actions = Array(action)
    lambda { controllers.include?(controller_name) && actions.include?(action_name) }
  end

  def edit_action_matcher(controller)
    action_matcher(controller,EDIT_ACTIONS)
  end

  def create_action_matcher(controller)
    action_matcher(controller,CREATE_ACTIONS)
  end

Working except for an edit action on a nested resource.

p.s. I need to refresh the page more often. :)

[superlou]

Also, doesn't seem to be an issue related to nested resources. It occurs whenever there are 2 siblings at a level of the tree.

[mjtko]

Not quite sure what you mean by 2 siblings at a level of the tree. Can you provide an example or two?

Something you can try to use for debugging:

  def action_matcher(controller,action)
    controllers = Array(controller)
    actions = Array(action)
    lambda do 
      STDERR.puts "ctrlr: #{controller_name} act: #{action_name}"
      controllers.include?(controller_name) && actions.include?(action_name)
    end
  end

[superlou]

So, I have two Events (Event A and Event B), when I try to edit (or update) either my breadcrumb looks like:

Events ≫ Event A ≫ Edit Event A ≫ Event B ≫ Edit Event B

In the STDERR I see ctrlr: events act: edit repeated 8 times.

When I remove the highlights_on, I get a more normal result though it only works with edit (not update).

Events ≫ Event A ≫ Edit Event A

My structure is the following (using a normal list view at the root of the project):

Events
= Event A
== Activities
=== Main Concert
==== Edit Main Concert
=== Safety Dance
==== Edit Safety Dance
=== New activity
== Edit Event A
= Event B
== Activities
=== New activity
== Edit Event B
= New event

[mjtko]

Ah, I see, I guess it's matching on both edit actions within the navigation.

If you have an edit entry for each event, you'll need to be selective about matching for the id of the event too. At work right now, will try and work on a solution/example this evening (UK).

[mjtko]

Maybe getting a bit messy again, but something like this should work:

  EDIT_ACTIONS = ['edit','update']
  def edit_action_matcher(controller,&additional_condition)
    action_matcher(controller,EDIT_ACTIONS,&additional_condition)
  end

  def action_matcher(controller,action,&additional_condition)
    controllers = Array(controller)
    actions = Array(action)
    lambda do 
     controllers.include?(controller_name) && 
       actions.include?(action_name) && 
       ( additional_condition.nil? ? true : additional_condition.call )
    end
  end

And then:

events_sub_nav.item "edit_event_a", "Edit Event A", edit_event_path(event_a),
  :highlights_on => edit_action_matcher('events') { @event == event_a }
events_sub_nav.item "edit_event_b", "Edit Event B", edit_event_path(event_b),
  :highlights_on => edit_action_matcher('events') { @event == event_b }

I imagine you're already DRYing this stuff up in your navigation.rb so perhaps it's not too bad.

[mjtko]

PS. I've merged the highlights-on-proc branch into master now, and will release a new version of the gem shortly. Please let me know when you've finished with the branch and are using master or the latest gem so I can remove it - ta. :)

[mjtko]

FYI, I'm going to remove the branch tomorrow and close the issue - shout if you think otherwise. :)

[superlou]

Very sorry not to get back to you! Github stopped sending me the notification emails and I fell behind. I've switched to master so will be ok. Never quite got it working, but haven't gotten to try your most recent comments. Will hopefully get a chance tomorrow evening. If you want to close the issue, be my guest.

Again, sorry not to respond until now.

[mjtko]

Ok, will close this, please feel free to reopen if you have more ideas for improvements.