-
Notifications
You must be signed in to change notification settings - Fork 65
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
fixed in method for discovery rule and updated it's entities #1480
fixed in method for discovery rule and updated it's entities #1480
Conversation
75247da
to
ed29751
Compare
PRT Result
|
trigger: test-robottelo |
PRT Result
|
ed29751
to
c7123de
Compare
The description of the PR differs from the code change here. Can you please either update the description or make the code changes to match it. |
9104abd
to
314a17d
Compare
trigger: test-robottelo |
PRT Result
|
airgun/entities/discoveryrule.py
Outdated
:return: Read the page and provide the values in a dict, | ||
""" | ||
view = self.navigate_to(self, 'All') | ||
return view.page_info.read() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am in more favor to modify def read_all()
to return view.read()
. It would read entire page in that case and not just the table.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried using that option, but the issue is that even when we remove the table object from read_all()
, it still attempts to read all the values present in DiscoveryRulesView()
, such as title and table. Since the table has been deleted, it throws an error indicating that the table element is not present. Instead of removing it, I left it as is, so this doesn’t impact 2 out of the 4 test cases. For the cases where the rule is being deleted, I created a new read_after_del()
method, which reads only the page_info object instead of the entire object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than creating a new method, I simply updated the existing read_all()
method and removed the read_after_del()
method that I had previously added
314a17d
to
6141a67
Compare
trigger: test-robottelo |
PRT Result
|
airgun/entities/discoveryrule.py
Outdated
@@ -50,13 +50,16 @@ def read(self, entity_name, widget_names=None): | |||
return view.read(widget_names=widget_names) | |||
|
|||
def read_all(self): | |||
"""Reads the whole discovery rules table. | |||
"""Reads the entire discovery rules table or page after a rule is deleted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"""Reads the entire discovery rules table or page after a rule is deleted | |
"""Reads the entire discovery rules page |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated the comment.
airgun/entities/discoveryrule.py
Outdated
@@ -50,13 +50,16 @@ def read(self, entity_name, widget_names=None): | |||
return view.read(widget_names=widget_names) | |||
|
|||
def read_all(self): | |||
"""Reads the whole discovery rules table. | |||
"""Reads the entire discovery rules table or page after a rule is deleted | |||
|
|||
:return: list of table rows, each row is dict, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please update what it returns?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Also, please update :return: according to new implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes have been made.
210a971
to
d5f43e0
Compare
d5f43e0
to
93a5a11
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack
trigger: test-robottelo |
PRT Result
|
(cherry picked from commit e7ec818)
(cherry picked from commit e7ec818)
…1529) (cherry picked from commit e7ec818) Co-authored-by: amolpati30 <[email protected]>
Updated the views with Table() widget instead SatTable(), to read all the table values.
Added entities to read the page after deleting discovery rules table.
Dependent PR: SatelliteQE/robottelo#15783