Extend flatpak view and entity with remote details page - #1924
Merged
Conversation
vsedmik
added
No-CherryPick
Contributor
Reviewer's GuideAdds a dedicated Flatpak remote details page and CRUD modals, enriches the Flatpak Remotes view, and updates entity methods to leverage new workflows with retries and extended timeouts. Class diagram for Flatpak Remotes views and modalsclassDiagram
class FlatpakRemotesView {
+create_btn
+table
+is_displayed()
}
class FlatpakRemoteDetailsView {
+title
+url
+subtitle
+decription
+table
+pagination
+is_displayed()
}
class CreateFlatpakRemoteModal {
+title
+name
+url
+username
+password
+create_btn
+cancel_btn
+is_displayed()
}
class EditFlatpakRemoteModal {
+title
+name
+url
+username
+password
+update_btn
+cancel_btn
+is_displayed()
}
class MirrorFlatpakRemoteModal {
+title
+searchbar
+mirror_btn
+cancel_btn
+is_displayed()
}
class FlatpakRemoteDeleteModal {
+title
+delete_btn
+cancel_btl
+is_displayed()
}
FlatpakRemotesView <|-- FlatpakRemoteDetailsView
FlatpakRemotesView <|-- CreateFlatpakRemoteModal
FlatpakRemotesView <|-- EditFlatpakRemoteModal
FlatpakRemotesView <|-- MirrorFlatpakRemoteModal
FlatpakRemotesView <|-- FlatpakRemoteDeleteModal
Class diagram for FlatpakRemotesEntity methodsclassDiagram
class FlatpakRemotesEntity {
+search(value)
+read(widget_names)
+read_table()
+read_remote_details(name, repo_search)
+create(values)
+edit(entity_name, values)
+delete(entity_name)
+scan(entity_name)
+mirror(remote, repo, product)
}
FlatpakRemotesEntity <|-- ShowAllFlatpakRemotes : navigator
ShowAllFlatpakRemotes : +step()
ShowAllFlatpakRemotes : VIEW = FlatpakRemotesView
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
vsedmik
force-pushed
the
fp-remote-details
branch
4 times, most recently
from
August 4, 2025 14:17
d2311a3 to
411eace
Compare
vsedmik
marked this pull request as ready for review
August 4, 2025 18:32
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey @vsedmik - I've reviewed your changes - here's some feedback:
- Consider extracting the repeated timeout values (e.g. '20s') and common wait/display logic into constants or helper methods to reduce duplication.
- There are a couple of typos in attribute names (e.g. 'decription' instead of 'description' and 'cancel_btl' instead of 'cancel_btn') that could lead to runtime errors.
- The
is_displayedimplementation is duplicated across multiple modal and view classes—consider abstracting that check into a shared mixin or base class.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the repeated timeout values (e.g. '20s') and common wait/display logic into constants or helper methods to reduce duplication.
- There are a couple of typos in attribute names (e.g. 'decription' instead of 'description' and 'cancel_btl' instead of 'cancel_btn') that could lead to runtime errors.
- The `is_displayed` implementation is duplicated across multiple modal and view classes—consider abstracting that check into a shared mixin or base class.
## Individual Comments
### Comment 1
<location> `airgun/views/flatpak.py:58` </location>
<code_context>
+ title = OUIATitle('flatpak-remote-title')
+ url = OUIAText('url-text-value')
+ subtitle = OUIATitle('flatpak-remote-subtitle')
+ decription = OUIAText('flatpak-remote-description')
+
+ table = PF5OUIATable(
</code_context>
<issue_to_address>
Typo in attribute name 'decription'.
Please rename 'decription' to 'description' for consistency.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
decription = OUIAText('flatpak-remote-description')
=======
description = OUIAText('flatpak-remote-description')
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `airgun/views/flatpak.py:144` </location>
<code_context>
+ title = Text("//span[normalize-space(.)='Delete Flatpak remote?']")
+
+ delete_btn = PF5Button('Delete')
+ cancel_btl = PF5Button('Cancel')
+
+ @property
</code_context>
<issue_to_address>
Typo in attribute name 'cancel_btl'.
Consider renaming 'cancel_btl' to 'cancel_btn' for consistency with existing naming conventions.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
delete_btn = PF5Button('Delete')
cancel_btl = PF5Button('Cancel')
=======
delete_btn = PF5Button('Delete')
cancel_btn = PF5Button('Cancel')
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `airgun/entities/flatpak.py:148` </location>
<code_context>
+ view.table.row(name=repo)['Mirror'].widget.click()
+ view = MirrorFlatpakRemoteModal(self.browser)
+ view.wait_displayed(timeout='20s')
+ view.searchbar.fill(product.name)
+ self.browser.plugin.ensure_page_safe()
+ view.mirror_btn.click()
</code_context>
<issue_to_address>
Assumes 'product' has a 'name' attribute.
If 'product' is not an object with a 'name' attribute (e.g., a string), this will cause an AttributeError. Please validate or document the expected type for 'product'.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
view.table.row(name=repo)['Mirror'].widget.click()
view = MirrorFlatpakRemoteModal(self.browser)
view.wait_displayed(timeout='20s')
view.searchbar.fill(product.name)
self.browser.plugin.ensure_page_safe()
view.mirror_btn.click()
=======
view.table.row(name=repo)['Mirror'].widget.click()
view = MirrorFlatpakRemoteModal(self.browser)
view.wait_displayed(timeout='20s')
# Expecting 'product' to be an object with a 'name' attribute (str).
if not hasattr(product, 'name'):
raise TypeError(f"'product' must have a 'name' attribute, got {type(product).__name__}")
view.searchbar.fill(product.name)
self.browser.plugin.ensure_page_safe()
view.mirror_btn.click()
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
sambible
approved these changes
Aug 4, 2025
sambible
left a comment
sambible
left a comment
Contributor
There was a problem hiding this comment.
Nothing major to add here, looks good. I'd commit those name tweaks the bot suggested.
LGTM
| ) | ||
|
|
||
|
|
||
| class FlatpakRemoteDetailsView(BaseLoggedInView, SearchableViewMixinPF4): |
Contributor
There was a problem hiding this comment.
We don't need to do this here, but we should very much look into making a new PF5 Searchable View Mixin, or at least creating a dummy one so that we don't have to continue to have PF4 branded classes imported into our new views.
Non blocking here, though, since this doesn't exist currently.
Contributor
Author
There was a problem hiding this comment.
Agree we should implement PF5 Searchable View Mixin, somewhat I expected it to happen during the PF5 refactor and I didn't want block myself on this.
Anyway, it should be a different themed PR, I think. We would need to update a bunch of other views with the new PF5SVM in it.
Contributor
There was a problem hiding this comment.
Agree, we should not block on this. I will create an issue for this.
LadislavVasina1
self-requested a review
August 5, 2025 07:31
LadislavVasina1
added
the
PRT-Passed
Contributor
|
PRT Passed in SatelliteQE/robottelo#18985 |
LadislavVasina1
requested changes
Aug 5, 2025
LadislavVasina1
left a comment
LadislavVasina1
left a comment
Contributor
There was a problem hiding this comment.
Small change requested, otherwise great addition @vsedmik, GJ!
LadislavVasina1
approved these changes
Aug 5, 2025
vsedmik
force-pushed
the
fp-remote-details
branch
from
August 6, 2025 12:22
f1a920f to
025081f
Compare
Contributor
Author
|
Squashed and rebased. |
vsedmik
force-pushed
the
fp-remote-details
branch
from
August 6, 2025 20:48
025081f to
42a5a5a
Compare
LadislavVasina1
approved these changes
Aug 11, 2025
vijaysawant
approved these changes
Aug 11, 2025
jnagare-redhat
pushed a commit
to jnagare-redhat/airgun
that referenced
this pull request
Aug 13, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR just extends the Flatpak view and entity with the brand new Flatpak remote details page.
Requires:
Katello/katello#11424
Katello/katello#11452
Katello/katello#11461
Summary by Sourcery
Extend Flatpak module with a dedicated remote details page, full CRUD and mirror capabilities, and improved view and navigation reliability.
New Features:
Enhancements: