[NAE-2243] Dynamic date resolution in Elastic query #309
Conversation
- add dynamicQuery switch to case search request
WalkthroughA new optional boolean field Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
projects/netgrif-components-core/src/lib/filter/models/case-search-request-body.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Matrix Test (24)
- GitHub Check: Matrix Test (20)
- GitHub Check: Matrix Test (22)
- GitHub Check: Test with SonarCloud
🔇 Additional comments (1)
projects/netgrif-components-core/src/lib/filter/models/case-search-request-body.ts (1)
55-58: Verify security implications of dynamic query execution.Enabling Groovy-based dynamic code execution in queries raises security concerns. Ensure the backend properly validates and sanitizes inputs to prevent code injection attacks, especially if any part of the query can be influenced by user input.
Please confirm:
- What input validation exists on the backend for queries when
dynamicQueryis enabled?- Are there any restrictions on which Groovy expressions can be executed?
- Is there documentation on safe usage patterns for this feature?
| /** | ||
| * A switch for resolving query the old way or by Groovy compiler for dynamic code in queries. | ||
| */ | ||
| dynamicQuery?: boolean; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Improve documentation clarity.
The JSDoc comment could be more specific and helpful for developers. Consider clarifying:
- What happens when
dynamicQueryistruevsfalsevsundefined(not provided) - What "the old way" means (e.g., "static query resolution" or "template-based resolution")
- What the Groovy-based dynamic code execution enables
- The default behavior when this field is omitted
Example improvement:
/**
- * A switch for resolving query the old way or by Groovy compiler for dynamic code in queries.
+ * Controls query resolution mode for the {@link query} field.
+ *
+ * - `true`: Enables Groovy-based dynamic code evaluation in queries, allowing runtime date resolution and expressions
+ * - `false` or `undefined`: Uses legacy template-based query resolution (default)
+ *
+ * @default false
*/
dynamicQuery?: boolean;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * A switch for resolving query the old way or by Groovy compiler for dynamic code in queries. | |
| */ | |
| dynamicQuery?: boolean; | |
| /** | |
| * Controls query resolution mode for the {@link query} field. | |
| * | |
| * - `true`: Enables Groovy-based dynamic code evaluation in queries, allowing runtime date resolution and expressions | |
| * - `false` or `undefined`: Uses legacy template-based query resolution (default) | |
| * | |
| * @default false | |
| */ | |
| dynamicQuery?: boolean; |
🤖 Prompt for AI Agents
In
projects/netgrif-components-core/src/lib/filter/models/case-search-request-body.ts
around lines 55 to 58, the JSDoc for dynamicQuery is vague; update it to
explicitly state behavior for true, false, and undefined, define what "the old
way" means (e.g., static/template-based resolution), describe what enabling
Groovy-based dynamic execution allows (evaluating dynamic expressions or scripts
in queries), and document the default when omitted; change the comment to
clearly list: true = use Groovy dynamic evaluation, false = use
static/template-based resolution, undefined = use the default behavior (specify
which default your system uses), and note any security/validation considerations
or side effects of enabling Groovy evaluation.
|
Description
Fixes NAE-2243
Dependencies
none
Third party dependencies
Blocking Pull requests
There are no dependencies on other PR
How Has Been This Tested?
manually
Test Configuration
Checklist:
Summary by CodeRabbit