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

Jump to bottom

[SLE-9109] Packages online search #467

Merged
merged 31 commits into from Jan 24, 2020
Merged

[SLE-9109] Packages online search #467

merged 31 commits into from Jan 24, 2020

Conversation

imobachgs
Copy link
Contributor

@imobachgs imobachgs commented Jan 14, 2020 •
edited

This PR is an initial implementation of the online search feature SLE 9109. It is rather incomplete, but it is already able to search for packages and register modules as needed.

online-search-enable-module
Selecting a package from an unregistered module for installation

online-search-module-details
Displaying package details (including version and module where it belongs to)

online-search-menu-option
The menu option to start the online search

Todo

  • Register the base system
  • Do not initialize the sw management when called from the packager
  • Install the packages when called as a standalone client
  • Display dependencies when asking about enabling a module
  • Refresh module details after selecting a package for installation
  • Keep table order after selecting a package

@coveralls
Copy link

coveralls commented Jan 14, 2020 •
edited

Coverage Status

Coverage increased (+0.2%) to 79.025% when pulling eecb651 on jse-9109/online-search into 81fc218 on master.

@lslezak
Copy link
Member

lslezak commented Jan 15, 2020

I haven't checked the code yet, here are just some notes and hints for improvements 😉

UI/UX

  • It's not clear what can be entered in the search field, is it just a substring or regexps are supported? The full packager widget contains a Search Mode combo box with values like "Contains", "Begins with", or "Use Regular Expression". You can also search in description or in the package summary. I think it would be nice to use it also here if the SCC backend supports that...
  • About the search: Would it be possible to filter out e.g. the paid extensions? (The ones which require a separate registration key.)
  • Does the API return some more details about the package? E.g. summary or details? Sometimes it might be difficult to just decide from the package name.
  • When a new product to register requires a registration key the user should be informed in advance, before really registering it.
  • The package and product details are mixed together in the RichText details at the bottom. You can use e.g. <h3> headers with "Package" and "Product" label. Or at least there should be an empty line between the package and the product details.
  • Do we need that Toggle Package button at all? The standard packager widget does not use anything like that...
  • I'm not sure if we need a separate "Product Status" column. Maybe simply appending something like "(Not registered yet)" at the end would be better. E.g. display "Containers Module (Not registered yet)" in the "Product" column.

Technical Questions

  • Will this also later work with SMT/RMT? If not (or at least not in the near future) we should check which registration server is actually used and display an informative error for SMT/RMT.

Todo Notes

Return the list of packages that were selected for installation.

I think that you could simply select the packages to install after adding the respective modules, the packager widget would then correctly display them as selected to install.

lslezak
lslezak reviewed Jan 15, 2020
View reviewed changes
Copy link
Member

@lslezak lslezak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some notes, except that "OMG" note it looks good 😃

Thanks for the early PR!

src/lib/registration/package_search.rb Outdated Show resolved Hide resolved
src/lib/registration/remote_package.rb Show resolved Hide resolved
src/lib/registration/widgets/remote_package_details.rb Outdated Show resolved Hide resolved
This was referenced Jan 17, 2020
Merged
Merged
Merged
Merged
Merged
Merged
lslezak
lslezak reviewed Jan 22, 2020
View reviewed changes
Copy link
Member

@lslezak lslezak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor notes, looks good!

src/lib/registration/addon.rb Outdated Show resolved Hide resolved
src/lib/registration/addon.rb Outdated Show resolved Hide resolved
src/lib/registration/clients/online_search.rb Outdated Show resolved Hide resolved
src/lib/registration/dialogs/online_search.rb Show resolved Hide resolved
src/lib/registration/widgets/package_search.rb Outdated Show resolved Hide resolved
src/lib/registration/widgets/remote_package_details.rb Show resolved Hide resolved
src/lib/registration/widgets/remote_packages_table.rb Outdated Show resolved Hide resolved
src/lib/registration/widgets/remote_packages_table.rb Outdated Show resolved Hide resolved
src/lib/registration/widgets/remote_packages_table.rb Outdated Show resolved Hide resolved
test/registration/package_search_test.rb Outdated Show resolved Hide resolved
@imobachgs
Add some CWM related documentation macros
894eb02
@imobachgs
Add a RemotePackage class
a1f07f1
@imobachgs
Add a RemotePackage class
dd91a69
@imobachgs
Add a PackageSearch class for online searches
dcfe75e 
* It uses the SUSEConnect API to find out packages in all modules,
  including those which have not been activated yet.
@imobachgs
Add a packages search form
b9b9eb1
@imobachgs
Add a table to show a list of remote packages
a6988f6
@imobachgs
Add a widget to display remote packages details
274f3e5
@imobachgs
Add a widget to search for packages
cea258c
@imobachgs
Add an online_search client
c3d0b76
@imobachgs
Several usability fixes in the online search
7e8b229 
* Use just one column for information about the product.
* Drop the "Toggle Package" button (it can be selected using the enter
key).
* Make clear which is the expected input in the search box.
@imobachgs
Add missing textdomain call
96ac9c9
@imobachgs
Make RuboCop happy
eef685c
@imobachgs
Reduce PackageSearch#packages complexity
afb0fea
@imobachgs
Escape package and addon names in the details
4a2f87a
@imobachgs
Use a checkmark for installed packages
bb5b71e
@imobachgs
Do not touch the libzypp's lock in online searches
905a73b
@imobachgs
Update version dependency on yast2.rpm
46a5b51
@imobachgs
Improve online search table handling
57ff33e
@imobachgs
Do not initialize software management during online search
89007a4
@imobachgs imobachgs marked this pull request as ready for review January 23, 2020 12:25
lslezak
lslezak approved these changes Jan 23, 2020
View reviewed changes
Copy link
Member

@lslezak lslezak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@imobachgs imobachgs merged commit ba2b44e into master Jan 24, 2020
@imobachgs imobachgs deleted the jse-9109/online-search branch January 24, 2020 06:26
@yast-bot
Copy link
Contributor

❌ Public Jenkins job #66 failed

@yast-bot
Copy link
Contributor

❌ Internal Jenkins job #28 failed

@yast-bot
Copy link
Contributor

❌ Internal Jenkins job #29 failed

@skazi0 skazi0 mentioned this pull request Nov 10, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lslezak lslezak lslezak approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@imobachgs @coveralls @lslezak @yast-bot