Make redirects after actions the way user would expect

[miha-plesko]

When user deleted an object/container, she was always redirected to the list of all containers, but this brings bad UX. Fixed by redirecting user to the parent page of the element that she just deleted - just like she would have expected.

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1444078

@miq-bot assign @AparnaKarve
@miq-bot add_label bug

[AparnaKarve]

@miha-plesko While this works perfectly when the Cloud Object Store Containers are accessed from the Storage Manager Summary screen because we have a valid previous_breadcrumb_url, it does not work when I try to delete an item directly from cloud_object_store_container/show_list.

----] F, [2017-04-24T14:38:50.130901 #40537:3fce76c292a8] FATAL -- : Error caught: [NoMethodError] undefined method `[]' for nil:NilClass
/Users/aparnakarve/rh/master/manageiq/plugins/manageiq-ui-classic/app/controllers/application_controller.rb:2061:in `previous_breadcrumb_url'

Note that there are no breadcrumbs on this (cloud_object_store_container/show_list) screen.

[miha-plesko]

You are right, I don't know how this could have slipped through my testing. Fixed.

[miha-plesko]

@AparnaKarve thank you for testing. I've fixed the annoying problem that you've found by rewriting the if statement so that one can easily read it now. It bases on observation that variable @display is set to:

1. @display = nil when we're on list page (Containers or Objects)
2. @display = "main" when we're on details page (Container or Object)
3. @display = "cloud_object_store_objects" when we're on Object list page for specific Container

I'm looking forward to your response.

[miha-plesko]

I'm not sure why Travis is failing, it doesn't seem related.

[AparnaKarve]

Travis failures will be addressed with this: #1154

[AparnaKarve]

@miha-plesko Overall, LGTM.
However, I was wondering if we could do something about the CC errors this time around...

We have a mixin called GenericFormMixin that currently does something similar for the cancel action. I think we could also add a delete_action method there, like this --

def delete_action
  if @display == "main"
    session[:flash_msgs] = @flash_array.dup if @flash_array
    javascript_redirect(previous_breadcrumb_url)
  else
    render_flash unless @flash_array.nil? || performed?
  end
end

And then

delete_action if params[:pressed].ends_with?("delete")

in the controller to call delete_action

I think this should DRY things up and hopefully clear up those CC mass = 43 errors.

[miq-bot]

Checked commits miha-plesko/manageiq-ui-classic@bf62d8d~...4646a1c with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
5 files checked, 0 offenses detected
Everything looks good. 👍

[miha-plesko]

I've pushed additional commit as an answer to Aparna's comments. I had to complicate things with additional IF statement otherwise clear operation didn't get its flash rendered, but the solution is still DRY and resolves CC issue.

@mzazrivec looking forward to your review.

[mmojzis]

This PR is fixing this BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1444078

[simaishi]

Fine backport details:

$ git log -1
commit c170618a2b92f94f7e8a8031bfb61161db2c8171
Author: Milan Zázrivec <mzazrivec@redhat.com>
Date:   Wed Apr 26 10:39:07 2017 +0200

    Merge pull request #1130 from miha-plesko/redirect_delete_object
    
    Make redirects after actions the way user would expect
    (cherry picked from commit 3febb8ea6c033f1eab6bc0f3850510879abadcf2)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1458899