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

Add elemMatch support to mongo helper #3749

Merged
merged 4 commits into from Apr 12, 2022
Merged

Add elemMatch support to mongo helper #3749

merged 4 commits into from Apr 12, 2022

Conversation

bufke
Copy link
Contributor

@bufke bufke commented Mar 31, 2022

Description

Adds $elemMatch support to form data api filtering

Implementation

  • Add $elemMatch support to mongo helper. Considering what we already allow, this is probably safe. However any modification of a query param to mongo workflow merits a higher level of security review as we could potentially allow injection attacks 🚨
  • Handle mongo operation failure on data api with fallback.

Use Cases this solves

  • I want to filter submissions to only show results for a specific question inside of a repeating group (which is an array).
  • I maybe want to do this for a future gallery update Feature: New Gallery #1349

Other thoughts

  • What do you think of my handling of two semi related but not 100% related issues in one PR? It felt small enough to group
  • Thoughts on my doing some very very mild refactoring while in the area? Technically removing an unused import isn't 100% guaranteed to be safe in case something is importing *. But we only do that in settings.
  • I decided to catch the OperationFailure, check if it was some error I know about, and if not raise a warning and give a more generic message. I didn't want to display any messages right from mongo in case there is some way to force mongo to give out unwanted data via exception message.
  • I'm only 80% sure we need elemMatch for future gallery update, but it seems good to support it as a standard alone addition to the data api.

Related issues

Internal bug tracking: KPI-BACKEND-232K

@bufke bufke marked this pull request as ready for review March 31, 2022 18:22
@bufke bufke requested a review from noliveleger March 31, 2022 18:22
}
response = self.client.get(self.submission_list_url, data)
self.assertEqual(response.status_code, status.HTTP_200_OK)
if isinstance(response.data, list):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is pretty bad but the alternative seems to be writing two 95% identical copies of the test for each api version which I think is worse.

@noliveleger noliveleger merged commit b1b8af7 into beta Apr 12, 2022
@noliveleger noliveleger deleted the data-api-elemmatch branch April 12, 2022 15:43
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.

None yet

2 participants