Doesn't count with selection of belongsto filter if any object doesn't exist in the filter sentence #18651

lpichler · 2019-04-11T18:37:06Z

let's have this structure

PROV1 (proder)
 -> DATAC1 (datacenter)
         -> Host1  (host)
               -> Cluster1 (cluster)
               -> Cluster2 (cluster)
               -> Cluster3 (cluster)

belonsto filter stored in a group of user

/belongsto/ExtManagementSystem|PROV1/EmsFolder|Datacenters/EmsFolder|DATAC1/EmsFolder|host/EmsCluster|Host1/EmsCluster|Cluster1

it will display just Cluster1

but when you delete Cluster1

this belongsto filter will be unchanged in the group

and RBAC will display skip last element of belongsto filter (/EmsCluster|Cluster1) and
then RBAC will start behave like filter is just (without cluster:

/belongsto/ExtManagementSystem|PROV1/EmsFolder|Datacenters/EmsFolder|DATAC1/EmsFolder|host/EmsCluster|Host1

so it will display everything under host

concretely it will display

Cluster2 (cluster)
and
Cluster3 (cluster)

this fix will display everything under Cluster1 and it means nothing when Cluster1 doesn't exist.
so and there is no way how remove this old filter from UI - so I guess, it is not good

but pretending that there is no filter will display everything what it unintended for user...

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

Links

https://bugzilla.redhat.com/show_bug.cgi?id=1693183
ManageIQ/manageiq-ui-classic#5511 - UI part

lpichler · 2019-04-16T14:10:04Z

@miq-bot assign @gtanzillo

@miq-bot add_label bug, rbac

gtanzillo · 2019-04-16T14:25:01Z

spec/lib/rbac/filterer_spec.rb

+        end
+
+        let(:dc) do
+          datacenter = FactoryBot.create(:ems_folder, :name => "Datacenter1")


Shouldn't his be datacenter = FactoryBot.create(:datacenter, :name => "Datacenter1")?

thanks @gtanzillo

we are using for datacenter it EmsFolder - it is inherited from "belongsto filter sentence":

/belongsto/ExtManagementSystem|vCenter/EmsFolder|Datacenters/EmsFolder|EMEA CloudLab/EmsFolder|host/EmsCluster|Infrastructure

and there is used EmsFolder for datacenter.

kbrock

This looks good (even w/o my question/suggestion)

Is this ready to go?

kbrock · 2019-04-29T18:27:56Z

app/models/miq_filter.rb

@@ -58,7 +58,7 @@ def self.belongsto2object_list(tag)
        obj.children.grep(tag_part_klass).detect { |c| c.name == name }
      end

-      return result unless obj
+      return [] unless obj


Not sure about this. tired of returning [] all over the place. Does it make sense to return none instead?

Suggested change

return [] unless obj

return none unless obj

none is something in ruby ?

NameError (undefined local variable or method none' for main:Object)`

@kbrock anyway we need [] because it means - that no object will displayed and other code count with [] in filterer - but I agree [] is confusing - so we need to bigger refactoring here - and I would leave it on other PR.

Thanks. leave it as is. none is an Active Record thing. e.g.: Vm.none

lpichler · 2019-04-30T17:21:32Z

Is this ready to go?

yes.

there is UI PR ManageIQ/manageiq-ui-classic#5511 which reflects those changes.
and I also added method for the IU PR.

thanks

kbrock · 2019-05-02T16:33:48Z

spec/lib/rbac/filterer_spec.rb

+        end
+
+        let(:hfolder) do
+          hfolder = FactoryBot.create(:ems_folder, :name => "host")


ASIDE: have you tried:

FactoryBot.create(:ems_folder, :name => "host", :parent => dc)

or if that doesn't work,

FactoryBot.create(:ems_folder, :name => "host").tap { |hfolder| hfolder.parent = dc }

that's right 👍 I used the tap version. thanks!

if not - presence of filter will ensure to displaying nothing and add method for human displaying of belongsto filter in MiqFilter

miq-bot · 2019-05-03T09:44:31Z

Checked commit lpichler@76767c5 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

kbrock · 2019-05-03T22:01:21Z

thanks for merge GT - sorry @lpichler for the delay

simaishi · 2019-05-15T16:10:49Z

@lpichler hammer/yes?

lpichler · 2019-05-16T10:15:53Z

@simaishi yes it can be also in hammer.
@miq-bot add_label hammer/yes

…y_object_doenst_exists Doesn't count with selection of belongsto filter if any object doesn't exist in the filter sentence (cherry picked from commit fdc1b6d) https://bugzilla.redhat.com/show_bug.cgi?id=1710998

simaishi · 2019-05-20T18:05:27Z

Hammer backport details:

$ git log -1
commit 29f4d73b74fd7a0f742309a6c69002141b24b4b9
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Fri May 3 16:55:37 2019 -0400

    Merge pull request #18651 from lpichler/skip_belongsto_filter_when_any_object_doenst_exists
    
    Doesn't count with selection of belongsto filter if any object doesn't exist in the filter sentence
    
    (cherry picked from commit fdc1b6d6245f68c7c4d55ee4b140089622df73ac)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1710998

lpichler changed the title ~~[WIP] don't count with selection of belongsto filters if any object doesn't exist in the filter sentence~~ [WIP] Doesn't count with selection of belongsto filters if any object doesn't exist in the filter sentence Apr 11, 2019

lpichler changed the title ~~[WIP] Doesn't count with selection of belongsto filters if any object doesn't exist in the filter sentence~~ [WIP] Doesn't count with selection of belongsto filter if any object doesn't exist in the filter sentence Apr 11, 2019

miq-bot added the wip label Apr 11, 2019

lpichler changed the title ~~[WIP] Doesn't count with selection of belongsto filter if any object doesn't exist in the filter sentence~~ Doesn't count with selection of belongsto filter if any object doesn't exist in the filter sentence Apr 16, 2019

miq-bot assigned gtanzillo Apr 16, 2019

miq-bot added bug core/rbac and removed wip labels Apr 16, 2019

gtanzillo reviewed Apr 16, 2019

View reviewed changes

lpichler changed the title ~~Doesn't count with selection of belongsto filter if any object doesn't exist in the filter sentence~~ [WIP] Doesn't count with selection of belongsto filter if any object doesn't exist in the filter sentence Apr 16, 2019

miq-bot added the wip label Apr 16, 2019

lpichler changed the title ~~[WIP] Doesn't count with selection of belongsto filter if any object doesn't exist in the filter sentence~~ Doesn't count with selection of belongsto filter if any object doesn't exist in the filter sentence Apr 26, 2019

miq-bot removed the wip label Apr 26, 2019

kbrock reviewed Apr 29, 2019

View reviewed changes

lpichler mentioned this pull request Apr 30, 2019

Inform user about outdates belongsto filters ManageIQ/manageiq-ui-classic#5511

Merged

1 task

lpichler force-pushed the skip_belongsto_filter_when_any_object_doenst_exists branch 2 times, most recently from c61de29 to 628e1e0 Compare April 30, 2019 17:15

kbrock reviewed May 2, 2019

View reviewed changes

lpichler force-pushed the skip_belongsto_filter_when_any_object_doenst_exists branch from 628e1e0 to dfbcb32 Compare May 3, 2019 09:32

Check if object in belongsto filter exist - miq_filter.rb

76767c5

if not - presence of filter will ensure to displaying nothing and add method for human displaying of belongsto filter in MiqFilter

lpichler force-pushed the skip_belongsto_filter_when_any_object_doenst_exists branch from dfbcb32 to 76767c5 Compare May 3, 2019 09:35

gtanzillo added this to the Sprint 111 Ending May 13, 2019 milestone May 3, 2019

gtanzillo merged commit fdc1b6d into ManageIQ:master May 3, 2019

lpichler deleted the skip_belongsto_filter_when_any_object_doenst_exists branch May 7, 2019 08:28

miq-bot added the hammer/yes label May 16, 2019

simaishi added hammer/backported and removed hammer/yes labels May 20, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Doesn't count with selection of belongsto filter if any object doesn't exist in the filter sentence #18651

Doesn't count with selection of belongsto filter if any object doesn't exist in the filter sentence #18651

lpichler commented Apr 11, 2019 •

edited

Loading

lpichler commented Apr 16, 2019

gtanzillo Apr 16, 2019

lpichler Apr 26, 2019

kbrock left a comment

kbrock Apr 29, 2019

lpichler Apr 30, 2019

kbrock May 2, 2019

lpichler commented Apr 30, 2019

kbrock May 2, 2019

lpichler May 3, 2019

miq-bot commented May 3, 2019

kbrock commented May 3, 2019

simaishi commented May 15, 2019

lpichler commented May 16, 2019

simaishi commented May 20, 2019

Doesn't count with selection of belongsto filter if any object doesn't exist in the filter sentence #18651

Doesn't count with selection of belongsto filter if any object doesn't exist in the filter sentence #18651

Conversation

lpichler commented Apr 11, 2019 • edited Loading

Links

lpichler commented Apr 16, 2019

gtanzillo Apr 16, 2019

Choose a reason for hiding this comment

lpichler Apr 26, 2019

Choose a reason for hiding this comment

kbrock left a comment

Choose a reason for hiding this comment

kbrock Apr 29, 2019

Choose a reason for hiding this comment

lpichler Apr 30, 2019

Choose a reason for hiding this comment

kbrock May 2, 2019

Choose a reason for hiding this comment

lpichler commented Apr 30, 2019

kbrock May 2, 2019

Choose a reason for hiding this comment

lpichler May 3, 2019

Choose a reason for hiding this comment

miq-bot commented May 3, 2019

kbrock commented May 3, 2019

simaishi commented May 15, 2019

lpichler commented May 16, 2019

simaishi commented May 20, 2019

lpichler commented Apr 11, 2019 •

edited

Loading