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

Bug/WP-361: Jobs Search - restrict search to a specific portal #896

Merged
merged 2 commits into from
Nov 7, 2023

Conversation

chandra-tacc
Copy link
Collaborator

@chandra-tacc chandra-tacc commented Nov 6, 2023

Overview

The job search is returning jobs from other portal sharing same tenant.
The search query conditions for portal name and query strings is enforcing portal condition correctly.

Related

Changes

Fix the query to force condition logic, add the missing parentheses (portal_match) AND (name_match OR archiveSystemDir_match OR appid_match OR archiveSystemId_match)

  • For future, instead of manually constructing the query, query building utils can be used to build the query and avoid issues like this

Testing

  1. Without this PR, check the issue exists

  2. In the code change the portal name to 'Frontera'.

  3. With dev tools open, run a query like https://cep.test/workbench/history/jobs?query_string=jup. See the network call and check the portal name in the job list. It will show names like 'CEP'.
    Screenshot 2023-11-06 at 12 28 49 PM

  4. Apply this PR.

  5. Run search https://cep.test/workbench/history/jobs?query_string=jup again. See the results, no results will be returned.
    Screenshot 2023-11-06 at 12 29 28 PM

Outside the portal code, tested the query in test client, and checked with and without the fix:

query_string = 'jup'
limit = 100
offset = 0
portal_name = 'Frontera'

sql_queries = [
    f"(tags IN ('portalName: {portal_name}')) AND",
    f"((name like '%{query_string}%') OR",
    f"(archiveSystemDir like '%{query_string}%') OR",
    f"(appId like '%{query_string}%') OR",
    f"(archiveSystemId like '%{query_string}%'))",
]
data = client.jobs.getJobSearchListByPostSqlStr(
        limit=limit,
        startAfter=offset,
        orderBy='lastUpdated(desc),name(asc)',
        request_body={
            "search": sql_queries
        },
        select="allAttributes"
    )
print(len(data))

UI

Notes

Copy link

codecov bot commented Nov 6, 2023

Codecov Report

Merging #896 (50a190d) into main (e935225) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #896   +/-   ##
=======================================
  Coverage   63.41%   63.41%           
=======================================
  Files         432      432           
  Lines       12389    12389           
  Branches     2579     2579           
=======================================
  Hits         7856     7856           
  Misses       4323     4323           
  Partials      210      210           
Flag Coverage Δ
javascript 69.76% <ø> (ø)
unittests 56.94% <ø> (ø)

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

Files Coverage Δ
server/portal/apps/workspace/api/views.py 40.76% <ø> (ø)

@chandra-tacc chandra-tacc marked this pull request as ready for review November 6, 2023 18:52
Copy link
Contributor

@shayanaijaz shayanaijaz left a comment

Choose a reason for hiding this comment

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

LGTM

@chandra-tacc chandra-tacc merged commit ab77b1e into main Nov 7, 2023
6 checks passed
@chandra-tacc chandra-tacc deleted the bug/WP-361-job-search branch November 7, 2023 22:27
@chandra-tacc chandra-tacc changed the title Bug/WP-361: Fix query string construction Bug/WP-361: Job Search - fix query string construction Nov 8, 2023
@chandra-tacc chandra-tacc changed the title Bug/WP-361: Job Search - fix query string construction Bug/WP-361: Job Search - restrict search to a specific portal Nov 8, 2023
@chandra-tacc chandra-tacc changed the title Bug/WP-361: Job Search - restrict search to a specific portal Bug/WP-361: Jobs Search - restrict search to a specific portal Nov 8, 2023
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