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

task/WP-164 Implement Workspace Search #886

Merged
merged 6 commits into from
Oct 19, 2023

Conversation

shayanaijaz
Copy link
Contributor

@shayanaijaz shayanaijaz commented Oct 11, 2023

Overview

Implements workspace search that matches the new tapis v3 implementation of workspaces/projects

Related

Changes

  • Simplified IndexedProject document wrapper to more closely resemble the actual response that is sent to the frontend
  • Added new shared task that indexes the projects
  • Added elasticsearch query for when a query_string parameter is sent in a get projects request
  • Fixed a bug that was causing file listing search within shared workspaces to not work

Testing

  1. Setup search index using instructions from the Core Portal README
  2. Go to shared workspace and test out the search functionality. The title field for a workspace is search on at the moment
  3. Go into a shared workspace and test out the file search functionality and ensure that works as well

UI

No UI changes

Notes

TODO:

  • Add searching on project id field
  • Add unit test for new added feature Added task WP-341 for updating workspace tests

@codecov
Copy link

codecov bot commented Oct 11, 2023

Codecov Report

Merging #886 (55be3f7) into main (c522d1c) will decrease coverage by 0.12%.
The diff coverage is 32.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #886      +/-   ##
==========================================
- Coverage   63.44%   63.33%   -0.12%     
==========================================
  Files         427      427              
  Lines       12215    12245      +30     
  Branches     2510     2514       +4     
==========================================
+ Hits         7750     7755       +5     
- Misses       4259     4284      +25     
  Partials      206      206              
Flag Coverage Δ
javascript 69.67% <ø> (ø)
unittests 57.01% <32.50%> (-0.20%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
server/portal/libs/agave/operations.py 65.00% <100.00%> (ø)
server/portal/libs/elasticsearch/docs/base.py 98.55% <100.00%> (-0.03%) ⬇️
server/portal/apps/search/tasks.py 40.74% <75.00%> (+1.52%) ⬆️
server/portal/libs/elasticsearch/utils.py 76.00% <9.09%> (-11.50%) ⬇️
server/portal/apps/projects/views.py 35.84% <15.78%> (-3.48%) ⬇️

@shayanaijaz shayanaijaz marked this pull request as ready for review October 11, 2023 18:43
Copy link
Contributor

@edmondsgarrett edmondsgarrett left a comment

Choose a reason for hiding this comment

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

Workspace + workspace listing searches both working, code looks good, LGTM! Nice work

Copy link
Contributor

@edmondsgarrett edmondsgarrett left a comment

Choose a reason for hiding this comment

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

As for the question on any other fields to search by, only other one that could be useful is the project ID, if for some reason the project id was used instead of a long project name? (Seen PIs do this on DesignSafe)

Copy link
Collaborator

@chandra-tacc chandra-tacc left a comment

Choose a reason for hiding this comment

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

LGTM.

Also, tested:

  1. exact match
  2. term match
  3. case insensitive term match
  4. no match
  5. clear search.

Copy link
Collaborator

@chandra-tacc chandra-tacc left a comment

Choose a reason for hiding this comment

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

Just FYI on merging this PR: There is an issue with data files search within the shared workspace and Shayan is looking into the fix.

Copy link
Collaborator

@chandra-tacc chandra-tacc left a comment

Choose a reason for hiding this comment

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

looks good with recent commit.

  1. Title and ID search test works
  2. Searching for files at multiple levels returns results.

@chandra-tacc chandra-tacc merged commit f12180a into main Oct 19, 2023
4 of 6 checks passed
@chandra-tacc chandra-tacc deleted the task/WP-164--workspace-search branch October 19, 2023 16:34
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.

3 participants