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

ui search capability to include services #2832

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ArtjomsPorss
Copy link
Contributor

@ArtjomsPorss ArtjomsPorss commented Dec 17, 2024

Description

UI search capability to include services

  • added dropdown to pick Domain or Service search
  • added call to new search services endpoint
  • added display formatting of found services
  • added functional tests
  • updated jest snapshots

Contribution Checklist:

  • The pull request does not introduce any breaking changes
  • I have read the contribution guidelines.
  • Create an issue and link to the pull request.

@ArtjomsPorss ArtjomsPorss marked this pull request as ready for review December 17, 2024 17:00
@ArtjomsPorss ArtjomsPorss force-pushed the search-services branch 2 times, most recently from ac5da73 to dc3db16 Compare December 24, 2024 20:12
Copy link
Contributor

@chandrasekhar1996 chandrasekhar1996 left a comment

Choose a reason for hiding this comment

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

  1. We should indicate to the the user that the search results being shown is partial. (The search results is partial when serviceMatchCount field is set.) One way to do that can be to show it in Search results header below
Screenshot 2025-01-13 at 6 57 55 PM
  1. The backend API has the capability to filter results based on domain name as well. Can you confirm if this story/PR was also meant to add support for that in UI? IIRC users can give "athenz:zms" as search term then UI can make search call with domain name set to athenz and service to zms

req.clients.zms.searchServiceIdentities(params, function (err, data) {
if (!err && Array.isArray(data.list)) {
data.list.sort((a, b) => {
debug(a.name, b.name);
Copy link
Contributor

Choose a reason for hiding this comment

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

remove in subsequent PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

regarding point 2 - services search accepts partial name of the service, without the domain name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will make change regarding point 1

Copy link
Contributor

Choose a reason for hiding this comment

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

for point 2, the scenario I was thinking of is if searching for a service returns more than 100(default limit) results then user will never be able to use UI for search if the result is after top 100 even though the user may know the domain name which will help further narrow it down.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants