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

Feature/437 document text search #197

Merged
merged 26 commits into from
Jan 3, 2025

Conversation

maxdiebold-erg
Copy link
Collaborator

Related Issues:

Main Changes:

  • Added a new "Action Documents" profile. This is made up of two ATTAINS-provided tables and one derived text search vector table, allowing for queries against full documents.
    • The user-facing "profile" is a view of the relevant fields from the three tables.
  • Added a "preview" button to the UI that is only shown for the Action Documents profile (for now). This displays the results in a sortable table, and is limited to 500 rows.
  • Updated the api-public file with the new profile endpoints.

Steps To Test:

  1. Go to http://localhost:3000/attains/actionDocuments.
  2. Type some text into the SEARCH TEXT / KEYWORD field, and click "Preview". Confirm results are shown in a table, or an empty result message is shown.
  3. Close the modal, then click "Download". Confirm the result count matches the preview. Proceed with the download, and inspect the downloaded file. It should contain a superset of the previewed columns.
  4. Run some queries against the API, and verify they function as expected.
  5. Check the API documentation (http://localhost:3000/api-documentation) for correctness.

TODO:

  • Move as much to config as possible, especially the field specifiers in the PreviewModal component.

Comment on lines +1122 to +1124
router.get('/health/etlDomainValues', async function (req, res) {
await checkDomainValuesHealth(req, res);
});

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
a file system access
, but is not rate-limited.
Copy link
Collaborator

@cschwinderg cschwinderg left a comment

Choose a reason for hiding this comment

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

This is awesome and works well!

I left some small suggestions/comments below. I think we need to add some cypress tests around the document search. I'm ok with the tests being a separate PR/ticket.

app/client/src/components/previewModal.tsx Outdated Show resolved Hide resolved
app/client/src/components/previewModal.tsx Outdated Show resolved Hide resolved
app/client/src/components/previewModal.tsx Show resolved Hide resolved
app/server/.env.example Show resolved Hide resolved
@cschwinderg
Copy link
Collaborator

I just started looking at the changes and noticed that the querying of the original profiles doesn't work any more. The filters aren't filtering anything and the download only has the objectId column.

@cschwinderg
Copy link
Collaborator

It looks like the issue is just with the actions profile.

@maxdiebold-erg
Copy link
Collaborator Author

I made some changes to the client-side profiles.json config yesterday, and somehow missed the Actions profile. It should be fixed now.

@cschwinderg
Copy link
Collaborator

cschwinderg commented Jan 3, 2025

The Any option in the Reporting Cycle field is no longer in the list. Other than that, I think the changes look good.

Copy link

sonarqubecloud bot commented Jan 3, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Copy link
Collaborator

@cschwinderg cschwinderg left a comment

Choose a reason for hiding this comment

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

Thanks for making those changes.

@cschwinderg cschwinderg merged commit b20d045 into develop Jan 3, 2025
5 of 7 checks passed
@cschwinderg cschwinderg deleted the feature/437_document-text-search branch January 3, 2025 14:36
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