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

OR Multiple _typeFilters #54

Merged
merged 2 commits into from
Dec 16, 2024
Merged

OR Multiple _typeFilters #54

merged 2 commits into from
Dec 16, 2024

Conversation

elsaperelli
Copy link
Collaborator

@elsaperelli elsaperelli commented Dec 11, 2024

Summary

This PR ensures that our implementation of the _typeFilter $export Query Parameter aligns with the Bulk Data Access IG. Two or more _typeFilter Query Parameters are OR'd together when provided in an export request.

New behavior

  • _typeFilter is now aligned with the IG: "When more than one _typeFilter parameter is provided with a query for the same resource type, the server SHALL include resources of that resource type that meet the criteria in any of the parameters (a logical "or")."

Code changes

  • src/service/export.service.js - checks if _typeFilter is already an array
  • src/util/exportToNDJson.js - checks if _typeFilter is already an array, added a TODO comment for addressing ANDs within a _typeFilter query (we have a separate backlog task for this)
  • test/util/exportToNDJson.test.js - added unit test for _typeFilter array

Testing guidance

  • npm run db:reset
  • npm run upload-bundles
  • npm start
  • Try out different _typeFilter queries in Insomnia and make sure they work as expected! Example request collection for Insomnia is attached.

typeFilterUpdate.json

Copy link

github-actions bot commented Dec 11, 2024

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements 75.63% 574/759
🟡 Branches
63.33% (+0.14% 🔼)
209/330
🟡 Functions 75.83% 91/120
🟡 Lines 75.87% 566/746
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / export.service.js
85.91%
80% (-0.68% 🔻)
94.12% 85.52%

Test suite run success

96 tests passing in 8 suites.

Report generated by 🧪jest coverage report action from 9bbd0e0

@lmd59 lmd59 self-requested a review December 12, 2024 14:22
@lmd59 lmd59 self-assigned this Dec 12, 2024
@@ -78,7 +78,7 @@ const exportToNDJson = async jobOptions => {
const valueSetQueries = {};
if (typeFilter) {
// subqueries may be joined together with a comma for a logical "or"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// subqueries may be joined together with a comma for a logical "or"
// subqueries may be joined together with a comma or as an array for a logical "or"

Copy link
Contributor

Choose a reason for hiding this comment

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

(or however you think it would be appropriate to update this comment)

Copy link
Contributor

@lmd59 lmd59 left a comment

Choose a reason for hiding this comment

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

Great updates! Added one very small recommendation for a comment update.

@elsaperelli elsaperelli requested a review from lmd59 December 16, 2024 13:56
@elsaperelli elsaperelli merged commit 535e176 into main Dec 16, 2024
4 checks passed
@elsaperelli elsaperelli deleted the argo-typefilter branch December 16, 2024 14:05
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