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

Launchpad: Search for another PR (GitHub) #3543

Closed
axosoft-ramint opened this issue Sep 6, 2024 · 10 comments · Fixed by #3684
Closed

Launchpad: Search for another PR (GitHub) #3543

axosoft-ramint opened this issue Sep 6, 2024 · 10 comments · Fixed by #3684
Assignees
Labels
area-launchpad Issues or features related to Launchpad feature New feature or request verified ✔ Verified
Milestone

Comments

@axosoft-ramint
Copy link
Contributor

axosoft-ramint commented Sep 6, 2024

Add ability to search for a PR not in the Launchpad list. UX needs to be talked through.

Normally our algorithm provides the Launchpad with some subset of all PRs. So, the full list of PRs accessible to the user is wider. The current search looks only for PRs selected for the Launchpad and we want user to be able to search among all other available PRs.

The current idea, but needs discussion and may be changed:

  • User can use the text input in the Launchpad quickpick, either in the main Launchpad list/step or through some other/new step, to search for a PR using its url. For example: https://github.com/gitkraken/vscode-gitlens/pull/3500. Or if that fails, we could just do a PR search with the integration using the user's search query.
  • If no item in the existing categorized list matches the search, we check other PRs in the repo (or repos) matching the user's text to see if we can find a matching PR, and show a Focus item (default to "other" category) for it.
  • The user can then open that item and take actions on it like any other Launchpad item.

Notes

  • Should first discuss design/UX with the team. For example: should there instead be a "search for a different PR" button or action item in the Launchpad list? Would that make it more discoverable?
  • Thinking of using integration.getPullRequest for the PR search when the user uses PR url, and integration.searchPullRequests for more general search, but the exact function/query may change depending on final design.
  • Note that if we use integration.getPullRequest or integration.searchPullRequests, they must be implemented for GitLab since Launchpad supports GitLab as well (currently it is only implemented for GitHub). This might be a good opportunity to add this functionality to the shared provider library, but for now we can implement them locally for GitLab as needed.

Specs and delivery

PR1. Search by URL GitHub

If I copy and paste a PR url directly into the input box on the main step, it should search for that PR

  1. ✅ filter the current list for the specified PR if it's there
  2. ✅ otherwise search for the specified PR by its ID
  3. ✅ make sure it's debounced properly
  4. Fix few search bugs:
  • ✅ Fix bug of not clearing the found item
  • ✅ Fix bug of incorrect local search by PR-id
  1. ✅ be friendly to incomplete URLs or URLs with additional parts

PR2. Add full search mode

Add an always visible item such as "Search everywhere". If it's clicked, switch to the mode that searches across all PRs the user has access to using the search terms.

  1. ✅ Add full search mode
  2. ✅ Bug: Local search cannot find items of collapsed groups.

Follow-ups

Developer's testing

Try different queries

  1. A title or a repo name of a PR that exists in the Launchpad
  2. A PR-number or a URL of a PR that exists in the Launchpad
  3. An URL of a PR that does not exists in the Launchpad
  4. Any query.

Supported expressions that get converted to a PR identity are reflected in the unit-test

Change query while searching

In order to debug this part I increase the debouncing timeout and add delays into the debounced procedure.

  1. While searching get back from API search to local filter:
  2. While searching get Back from API search to local search by URI
  • in 1-2, check that the current search is cancelled, its result is not applied
  1. While searching change the search query
  • the second result is applied
  • the first debounced promise is not cancelled, so it enters the finally method at the same time when the second query enters
  • inside of the first debounced request we see that the cancellation token require us to cancel, so we don't apply the result

Change query after the search

  1. After an API search get back to the local filter
  2. After an API search get back to the local search by URI
  • in 1-2, check that the additional search is removed from the result (e.g. it cannot be found by its title)
@axosoft-ramint axosoft-ramint self-assigned this Sep 6, 2024
@axosoft-ramint axosoft-ramint added this to the 16.0 milestone Sep 6, 2024
@axosoft-ramint axosoft-ramint added feature New feature or request area-launchpad Issues or features related to Launchpad labels Sep 6, 2024
sergeibbb added a commit that referenced this issue Sep 17, 2024
sergeibbb added a commit that referenced this issue Sep 17, 2024
@sergeibbb

This comment has been minimized.

@axosoft-ramint

This comment has been minimized.

@sergeibbb

This comment has been minimized.

@axosoft-ramint

This comment has been minimized.

sergeibbb added a commit that referenced this issue Oct 8, 2024
sergeibbb added a commit that referenced this issue Oct 8, 2024
sergeibbb added a commit that referenced this issue Oct 8, 2024
sergeibbb added a commit that referenced this issue Oct 10, 2024
sergeibbb added a commit that referenced this issue Oct 10, 2024
sergeibbb added a commit that referenced this issue Oct 10, 2024
sergeibbb added a commit that referenced this issue Oct 10, 2024
sergeibbb added a commit that referenced this issue Nov 6, 2024
sergeibbb added a commit that referenced this issue Nov 6, 2024
sergeibbb added a commit that referenced this issue Nov 6, 2024
sergeibbb added a commit that referenced this issue Nov 6, 2024
sergeibbb added a commit that referenced this issue Nov 6, 2024
sergeibbb added a commit that referenced this issue Nov 6, 2024
@sergeibbb sergeibbb added the needs-verification Request for verification label Nov 6, 2024
@sergeibbb sergeibbb changed the title Launchpad: Search for another PR Launchpad: Search for another PR (GitHub) Nov 6, 2024
@d13 d13 added the pending-release Resolved but not yet released to the stable edition label Nov 8, 2024
@sergiolms sergiolms added the verified ✔ Verified label Nov 13, 2024
@axosoft-ramint axosoft-ramint added needs-verification Request for verification and removed needs-verification Request for verification pending-release Resolved but not yet released to the stable edition labels Nov 14, 2024
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-launchpad Issues or features related to Launchpad feature New feature or request verified ✔ Verified
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants