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

Add both custom reports and custom report menu items to report menu #3190

Merged
merged 1 commit into from Jan 8, 2018

Conversation

skateman
Copy link
Member

@skateman skateman commented Jan 8, 2018

The code is nearly unreadable and honestly, I don't see the bigger picture here and have no idea how this get_reports_menu should behave. The fix is an ugly hack and it might break everything in cases which I don't even know if exist at all. Writing tests around something I don't understand is IMHO meaningless and it might cause even more problems.

If you go to Cloud Intel -> Reports (menu) -> Reports (accordion), you will see both the custom menus created under the Edit Report Menus (accordion) for the current user's group and the custom reports created (if any). Originally the manually edited report menus were not displayed, that has been fixed by #3098, that broke custom reports.

Because of #3187 there will be a merge conflict for gaprindashvili so I will create a separate PR for that.

@h-kataria could you please take a look at this? It might need some extra testing on each place where this is being used.

@miq-bot add_label bug, cloud intel/reports, gaprindashvili/yes
@miq-bot assign @h-kataria

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

@miq-bot
Copy link
Member

miq-bot commented Jan 8, 2018

@skateman Cannot apply the following label because they are not recognized: cloud intel/reports

@miq-bot
Copy link
Member

miq-bot commented Jan 8, 2018

Checked commit skateman@d18c53b with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 0 offenses detected
Everything looks fine. 🍪

@h-kataria
Copy link
Contributor

verified fix in UI, Custom reports folder is visible when logged in as admin. Created a new user for example "test" user, then edited reports menu for the "test" user's group, after assigning user's group some custom reports i was able to see them in Reports accordion after i logged in as
"test" user.

@h-kataria h-kataria added this to the Sprint 77 Ending Jan 15, 2018 milestone Jan 8, 2018
@h-kataria h-kataria merged commit e80fbe3 into ManageIQ:master Jan 8, 2018
@skateman skateman deleted the fix-report-menu branch January 8, 2018 17:09
@simaishi
Copy link
Contributor

Backported to Gaprindashvili via #3224

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.

None yet

4 participants