Added support for boundary filters in kibana dashboard #1729
Conversation
📝 WalkthroughWalkthroughThe changes introduce a new function, Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
micro-ui/web/micro-ui-internals/packages/modules/utilities/src/pages/employee/IFrameInterface/index.js(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
micro-ui/web/micro-ui-internals/packages/modules/utilities/src/pages/employee/IFrameInterface/index.js (1)
Pattern **/*.js: check
| function addBoundaryFilters(url, filters) { | ||
| const { province, district } = filters || {}; | ||
|
|
||
| // Determine filter type based on district and province | ||
| const filter = district | ||
| ? `(query:(match_phrase:(Data.boundaryHierarchy.district.keyword:'${district}')))` | ||
| : province | ||
| ? `(query:(match_phrase:(Data.boundaryHierarchy.province.keyword:'${province}')))` | ||
| : null; | ||
|
|
||
| // If there's a filter to apply | ||
| if (filter) { | ||
| // Match existing filters in the URL | ||
| const existingFilters = /filters:\!\((.*?)\)/.exec(url); | ||
|
|
||
| // Replace existing filters or append the new filter | ||
| const updatedUrl = existingFilters | ||
| ? url.replace(existingFilters[0], `filters:!(${filter})`).replace(/ /g, '%20') | ||
| : url; | ||
|
|
||
| setUrl(updatedUrl); | ||
| } else { | ||
| // No filter to add, keep original URL | ||
| setUrl(url); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider architectural improvements for boundary filter feature
To enhance maintainability and reliability:
- Add JSDoc documentation for the boundary filter feature
- Implement error tracking for filter-related issues
- Add unit tests for the new functionality
Would you like me to:
- Generate JSDoc documentation for the boundary filter feature?
- Create a test suite for the new functionality?
- Implement error tracking integration?
Also applies to: 203-203
Add input validation and security measures to addBoundaryFilters
The function needs several security and robustness improvements:
- Add input validation for the URL parameter
- Sanitize district/province values before using in query
- Use non-greedy regex matching
- Add error handling for malformed URLs
- Use proper escaping for query values
Consider applying these improvements:
function addBoundaryFilters(url, filters) {
+ if (!url || typeof url !== 'string') {
+ console.error('Invalid URL provided to addBoundaryFilters');
+ return url;
+ }
+
const { province, district } = filters || {};
+
+ // Sanitize input values
+ const sanitizeValue = (value) => {
+ if (!value) return null;
+ return encodeURIComponent(value.replace(/['"\\]/g, ''));
+ };
+
+ const sanitizedDistrict = sanitizeValue(district);
+ const sanitizedProvince = sanitizeValue(province);
// Determine filter type based on district and province
- const filter = district
- ? `(query:(match_phrase:(Data.boundaryHierarchy.district.keyword:'${district}')))`
- : province
- ? `(query:(match_phrase:(Data.boundaryHierarchy.province.keyword:'${province}')))`
+ const filter = sanitizedDistrict
+ ? `(query:(match_phrase:(Data.boundaryHierarchy.district.keyword:'${sanitizedDistrict}')))`
+ : sanitizedProvince
+ ? `(query:(match_phrase:(Data.boundaryHierarchy.province.keyword:'${sanitizedProvince}')))`
: null;
if (filter) {
try {
- const existingFilters = /filters:\!\((.*?)\)/.exec(url);
+ const existingFilters = /filters:\!\((.*?)\)/?.exec(url);
const updatedUrl = existingFilters
? url.replace(existingFilters[0], `filters:!(${filter})`).replace(/ /g, '%20')
: url;
setUrl(updatedUrl);
+ } catch (error) {
+ console.error('Error processing URL in addBoundaryFilters:', error);
+ setUrl(url);
+ }
} else {
setUrl(url);
}
}📝 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.
| function addBoundaryFilters(url, filters) { | |
| const { province, district } = filters || {}; | |
| // Determine filter type based on district and province | |
| const filter = district | |
| ? `(query:(match_phrase:(Data.boundaryHierarchy.district.keyword:'${district}')))` | |
| : province | |
| ? `(query:(match_phrase:(Data.boundaryHierarchy.province.keyword:'${province}')))` | |
| : null; | |
| // If there's a filter to apply | |
| if (filter) { | |
| // Match existing filters in the URL | |
| const existingFilters = /filters:\!\((.*?)\)/.exec(url); | |
| // Replace existing filters or append the new filter | |
| const updatedUrl = existingFilters | |
| ? url.replace(existingFilters[0], `filters:!(${filter})`).replace(/ /g, '%20') | |
| : url; | |
| setUrl(updatedUrl); | |
| } else { | |
| // No filter to add, keep original URL | |
| setUrl(url); | |
| } | |
| } | |
| function addBoundaryFilters(url, filters) { | |
| if (!url || typeof url !== 'string') { | |
| console.error('Invalid URL provided to addBoundaryFilters'); | |
| return url; | |
| } | |
| const { province, district } = filters || {}; | |
| // Sanitize input values | |
| const sanitizeValue = (value) => { | |
| if (!value) return null; | |
| return encodeURIComponent(value.replace(/['"\\]/g, '')); | |
| }; | |
| const sanitizedDistrict = sanitizeValue(district); | |
| const sanitizedProvince = sanitizeValue(province); | |
| // Determine filter type based on district and province | |
| const filter = sanitizedDistrict | |
| ? `(query:(match_phrase:(Data.boundaryHierarchy.district.keyword:'${sanitizedDistrict}')))` | |
| : sanitizedProvince | |
| ? `(query:(match_phrase:(Data.boundaryHierarchy.province.keyword:'${sanitizedProvince}')))` | |
| : null; | |
| if (filter) { | |
| try { | |
| const existingFilters = /filters:\!\((.*?)\)/?.exec(url); | |
| const updatedUrl = existingFilters | |
| ? url.replace(existingFilters[0], `filters:!(${filter})`).replace(/ /g, '%20') | |
| : url; | |
| setUrl(updatedUrl); | |
| } catch (error) { | |
| console.error('Error processing URL in addBoundaryFilters:', error); | |
| setUrl(url); | |
| } | |
| } else { | |
| setUrl(url); | |
| } | |
| } |
| @@ -173,6 +200,7 @@ | |||
| : ""; | |||
| const title = pageObject?.["title"] || ""; | |||
| let url = `${domain}${contextPath}`; | |||
| addBoundaryFilters(url, filters?.filters); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and state management for boundary filter integration
The current integration has potential issues:
- Multiple
setUrlcalls could lead to race conditions - Missing error boundary
- Unclear effect dependencies
Consider refactoring the URL construction logic:
- addBoundaryFilters(url, filters?.filters);
+ try {
+ // Combine URL modifications to avoid multiple state updates
+ let processedUrl = url;
+ if (filters?.filters) {
+ processedUrl = addBoundaryFilters(processedUrl, filters.filters);
+ }
+ if (pageObject?.authToken?.enable) {
+ processedUrl = addAuthToken(processedUrl, pageObject.authToken);
+ }
+ setUrl(processedUrl);
+ } catch (error) {
+ console.error('Error processing URL with filters:', error);
+ setUrl(url);
+ }Also, consider extracting the URL processing logic into a separate useCallback hook to better manage dependencies and prevent unnecessary re-renders.
Committable suggestion skipped: line range outside the PR's diff.
Choose the appropriate template for your PR:
Summary by CodeRabbit
New Features
IFrameInterfacecomponent based on user-defined boundary filters (province and district).Bug Fixes