Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix wrong flash messages after widget import #2411

Merged
merged 3 commits into from
Oct 20, 2017

Conversation

ZitaNemeckova
Copy link
Contributor

Cloud Intel -> Reports -> Import/Export -> Widgets -> Choose file (yml with exported widgets) -> Upload -> no widget is checked -> Commit

Before:
screen shot 2017-10-16 at 5 10 07 pm

No widget imported.

After:
screen shot 2017-10-16 at 4 50 00 pm

Cloud Intel -> Reports -> Import/Export -> Widgets -> Choose file (yml with exported widgets) -> Upload -> check few widgets -> Commit

Before:
screen shot 2017-10-16 at 5 10 07 pm

After:
screen shot 2017-10-16 at 5 03 54 pm

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

Bug is present in Fine.

@miq-bot add_label bug, fine/yes

add_flash(_("1 widget imported successfully"), :success)
else
add_flash(_("%{number} widgets imported successfully") % {:number => number}, :success)
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole if can be done as:

n_("%{number} widget imported successfully", "%{number} widgets imported successfully", number) % {:number => number}

@miq-bot
Copy link
Member

miq-bot commented Oct 18, 2017

Checked commits ZitaNemeckova/manageiq-ui-classic@78d375e~...155463d with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks fine. 🍪

@ZitaNemeckova
Copy link
Contributor Author

@mzazrivec Fixed. Thanks :)

@mzazrivec mzazrivec self-assigned this Oct 19, 2017
@mzazrivec
Copy link
Contributor

The problem here is that this fix will work only as long as queue worker isn't running. If it's running,
the widget import (with or without fix from this PR) will fail with the following error:

widget-import

This seems to be caused by the fact, that the imported file is deleted by the queue worker
before we're able to do anything with its content (see store_for_import from WidgetImportService).

I'm a bit on the edge what to do about this PR. @himdel @martinpovolny what do you guys think?

@himdel
Copy link
Contributor

himdel commented Oct 19, 2017

@mzazrivec I think we should treat it as 2 distinct bugs..

What store_for_import does is call MiqQueue.put with :deliver_on => 1.day.from_now,.

So.. there should be plenty of time :).

But apparently, the attribute is ignored because the task gets run immediately - hence I think this is 2 bugs and the second one is a backend bug => ManageIQ/manageiq#16238.

@ZitaNemeckova
Copy link
Contributor Author

@mzazrivec ManageIQ/manageiq#16238 for that error you mentioned.

@Fryguy
Copy link
Member

Fryguy commented Oct 19, 2017

If you are seeing this happening with simulate_queue_worker, then that is by design (see my comment in ManageIQ/manageiq#16238). If you are seeing that with a "real" worker, then we have a real bug.

@mzazrivec mzazrivec added this to the Sprint 72 Ending Oct 30, 2017 milestone Oct 20, 2017
@mzazrivec mzazrivec merged commit a26b01e into ManageIQ:master Oct 20, 2017
simaishi pushed a commit that referenced this pull request Nov 20, 2017
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit 7c8eb3faec2d647c62782fe71f80fd7a5bcae488
Author: Milan Zázrivec <[email protected]>
Date:   Fri Oct 20 13:18:34 2017 +0200

    Merge pull request #2411 from ZitaNemeckova/fix_widget_import
    
    Fix wrong flash messages after widget import
    (cherry picked from commit a26b01e4ad6d65b34aab64b4e4d084e7b18aafd7)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1504775

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants