ruffle1986 commented on a change in pull request #1356: METRON-2029: Configure
Table should have filter
URL: https://github.com/apache/metron/pull/1356#discussion_r265190796
##########
File path:
metron-interface/metron-alerts/src/app/alerts/configure-table/configure-table.component.ts
##########
@@ -91,6 +95,46 @@ export class ConfigureTableComponent implements OnInit {
});
}
+ ngAfterViewInit() {
+ fromEvent(this.filterColResults.nativeElement, 'keyup')
+ .pipe(debounceTime(250))
+ .subscribe(e => {
+ this.filterColumns(e['target'].value);
+ });
+ }
+
+ filterColumns(val) {
+ const words = val.trim().split(' ');
+ this.filteredColumns = this.allColumns.filter(col => {
+ if (col.displayName) {
+ if (this.colMissingFilterKeyword(words, col, true)) {
+ return false;
+ } else {
+ return true;
+ }
+ } else {
+ if (this.colMissingFilterKeyword(words, col)) {
+ return false;
+ } else {
+ return true;
+ }
+ }
+ });
+ }
+
+ colMissingFilterKeyword(words, col, displayName = false) {
+ if (displayName) {
+ return words.map(word =>
col.displayName.toLowerCase().includes(word.toLowerCase())).includes(false);
+ } else {
+ return words.map(word =>
col.columnMetadata.name.toLowerCase().includes(word.toLowerCase())).includes(false);
+ }
+ }
Review comment:
Well done. The only thing is (for me at least) that it took me a while to
understand this part of the code.
So the code above basically checks whether there's any word in the words
array that is missing from the col name. If so, it returns true which means
yes, `col(is)MissingFilterKeyword`. Amiright?
My suggestion is that we should make it more naturally readable.
First, I'd start the function with a verb to immediately make it clear that
this function returns with a bool. Since we don't use type annotation here, and
the function begins with a `map`, it's really difficult to notice it
immediately.
Function name starting with a verb:
`isColMissingFilterKeyword`
Secondly, at first glance, I thought this function returned an array because
of the map function at the beginning of the line. In my opinion, it's more
natural to read, but again, that is just my opinion.
```
colMissingFilterKeyword(words, col, displayName = false) {
if (displayName) {
return !words.every(word =>
col.displayName.toLowerCase().includes(word.toLowerCase()))
} else {
return !words.every(word =>
col.columnMetadata.name.toLowerCase().includes(word.toLowerCase()))
}
}
```
In the example above it almost says "Not every word is included in
col.displayName or col.columnMetadata.name"
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
