Expand scope of report definitions that visible to a user #16716
Conversation
gtanzillo
force-pushed
the
custom-reports-tenant-level
branch
3 times, most recently
from
1cdb677
to
6d7a792
Compare
|
@carbonin I cleaned up the tests as best I could and took care of the rubocops. Please take another look when you get a chance. |
spec/models/miq_report_spec.rb
Outdated
| it "returns reports created by me or anyone in a group in my tenant" do | ||
| my_report | ||
| report_in_my_tenant | ||
| report_in_another_tenant |
There was a problem hiding this comment.
If we change these to be let! we can remove these calls to instantiate the reports.
spec/models/miq_report_spec.rb
Outdated
| expect(described_class.for_user(my_user)).to match_array([my_report, report_in_my_tenant]) | ||
| end | ||
|
|
||
| it "does not return reports created by a group in another tenant" do |
There was a problem hiding this comment.
I think you can just remove this spec.
By asserting that the array only contains my_report and report_in_my_tenant in the first spec you're also asserting that report_in_another_tenant was not returned even though you created it.
gtanzillo
force-pushed
the
custom-reports-tenant-level
branch
from
6d7a792
to
3fbe9f3
Compare
This change expands the existing scope of the current user's group to the current user's tenant allowing visibility to all reports created by users within the tenant. The scope was changed in ManageIQ#15472 but was too restrictive Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1526058
gtanzillo
force-pushed
the
custom-reports-tenant-level
branch
from
3fbe9f3
to
5978047
Compare
|
Checked commit gtanzillo@5978047 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0 |
| let!(:report_in_another_tenant) { FactoryGirl.create(:miq_report, :miq_group => group_in_other_tenant, :rpt_type => "Custom") } | ||
|
|
||
| it "returns reports created by me or anyone in a group in my tenant" do | ||
| User.current_user = my_user |
There was a problem hiding this comment.
Not sure what happened in the specs, but maybe it's not safe to set User.current_user as that sets a thread variable?
There was a problem hiding this comment.
Weird, I had that line there when the tests were passing. I'm gonna restart and hope it succeeds.
Expand scope of report definitions that visible to a user (cherry picked from commit eff2583) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1531301
|
Gaprindashvili backport details: |
|
This PR introduced a travis spec failure in ui-classic - fixed by ManageIQ/manageiq-ui-classic#3171 :) (backport to fine together with that PR please) |
Expand scope of report definitions that visible to a user (cherry picked from commit eff2583) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1532857
|
Fine backport details: |
…t-level Expand scope of report definitions that visible to a user (cherry picked from commit eff2583) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1532857
This change expands the existing scope of the current user's group to the current user's tenant
allowing visibility to all reports created by users within the tenant.
The scope was changed in #15472 but was too restrictive
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1526058