Rescue failed container annotation attempts #13075
Conversation
enoodle
force-pushed
the
rescue_annotate_errors
branch
from
a725320
to
4b60dfa
Compare
|
LGTM 👍 |
|
LGTM 👍 |
| @@ -188,7 +188,7 @@ def check_compliance(model) | |||
|
|
|||
| def find_current_item(model) | |||
| if params[:id].nil? || model.find_by(:id => params[:id].to_i).nil? | |||
| add_flash(_("%{model} no longer exists") % {:table => ui_lookup(:model => model)}, :error) | |||
| add_flash(_("%{model} no longer exists") % {:table => ui_lookup(:model => model.to_s)}, :error) | |||
There was a problem hiding this comment.
This seems strange. The format hash should contain variables that appear in the formatted string. table isn't found there.
There was a problem hiding this comment.
I agree, I blindly fixed the similar problem in different places.
enoodle
force-pushed
the
rescue_annotate_errors
branch
from
4b60dfa
to
de3c186
Compare
|
@cben PTAL |
|
LGTM, thanks! |
|
We are going to need a test for this fix. |
enoodle
force-pushed
the
rescue_annotate_errors
branch
from
de3c186
to
9d76e5f
Compare
|
Checked commit enoodle@9d76e5f with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1 |
|
@moolitayer I added a test to |
|
@enoodle thanks! LGTM 👍 |
|
ping @simon3z |
|
@enoodle Cannot apply the following label because they are not recognized: bug euwe/yes |
|
@miq-bot add_label bug, euwe/yes |
|
@simon3z what about merging it to master? |
|
👍 ready cc @chessbyte @miq-bot assign chessbyte |
|
<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush. |
|
closed for its ui-classic counter-part. if needed a backport to euwe will be created after the ui-classic is merged. |
|
IIUC this was moved to ManageIQ/manageiq-ui-classic#34 |
When failing to annotate a container entity an error is thrown, this caught and will then cause
ui_lookupto be called with the module instead of its name which will cause another uncaught error in the controller that will be thrown.Updated all the calls in the file of to
ui_lookupto send the model's name instead of the model. Also refactored this specific error message.PS: The flash is not displayed correctly on error but this is out of scope for this patch.
BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1402349
cc @moolitayer @simon3z @yaacov @nimrodshn PTAL