Copilot commented on code in PR #4082:
URL: https://github.com/apache/streampipes/pull/4082#discussion_r2656646821
##########
ui/src/app/assets/components/asset-overview/asset-overview.component.ts:
##########
@@ -71,6 +71,7 @@ export class SpAssetOverviewComponent implements OnInit {
private breadcrumbService: SpBreadcrumbService,
private dialogService: DialogService,
private router: Router,
+ private route: ActivatedRoute,
Review Comment:
The ActivatedRoute dependency is injected but only used in debug console.log
statements. If the route is not needed for the fix, this dependency should be
removed. If it was intended to be used for reading query parameters to restore
filter state, that logic appears to be missing.
##########
ui/src/app/assets/components/asset-overview/asset-overview.component.ts:
##########
@@ -79,6 +80,11 @@ export class SpAssetOverviewComponent implements OnInit {
) {}
ngOnInit(): void {
+ console.log('[AssetOverview] ngOnInit');
+ console.log(
+ '[AssetOverview] query params',
+ this.route.snapshot.queryParams,
+ );
Review Comment:
Debug console.log statements should be removed before merging to production.
These appear to be leftover debugging code that adds unnecessary noise to the
console.
```suggestion
```
##########
ui/src/app/assets/components/asset-overview/asset-overview.component.ts:
##########
@@ -112,10 +118,22 @@ export class SpAssetOverviewComponent implements OnInit {
}
loadAssets(): void {
+ console.log(
+ '[AssetOverview] loading assets with filter',
+ this.currentFilterIds,
+ );
this.assetService.getAllAssets().subscribe(result => {
this.existingAssets = result as SpAssetModel[];
this.dataSource.sort = this.sort;
this.dataSource.data = this.existingAssets;
+
+ // Apply filter if one exists after loading assets
+ if (
+ this.currentFilterIds &&
+ this.hasActiveFilter(this.currentFilterIds)
+ ) {
Review Comment:
The condition on line 132 checks if currentFilterIds is truthy, which makes
the hasActiveFilter check on line 133 partially redundant. The hasActiveFilter
method already checks for null/undefined. Consider simplifying to just use
hasActiveFilter(this.currentFilterIds) or remove the redundant null check from
hasActiveFilter.
```suggestion
if (this.hasActiveFilter(this.currentFilterIds)) {
```
##########
ui/src/app/assets/components/asset-overview/asset-overview.component.ts:
##########
@@ -200,6 +218,10 @@ export class SpAssetOverviewComponent implements OnInit {
});
}
+ private hasActiveFilter(filter: Set<string>): boolean {
+ return filter !== null && filter !== undefined && filter.size > 0;
Review Comment:
The null and undefined checks are redundant when checking the size property.
In TypeScript/JavaScript, if filter is null or undefined, accessing filter.size
would throw an error. Consider using optional chaining: return filter?.size > 0
?? false; This is more concise and handles both null and undefined cases safely.
```suggestion
return (filter?.size ?? 0) > 0;
```
##########
ui/src/app/assets/components/asset-overview/asset-overview.component.ts:
##########
@@ -112,10 +118,22 @@ export class SpAssetOverviewComponent implements OnInit {
}
loadAssets(): void {
+ console.log(
+ '[AssetOverview] loading assets with filter',
+ this.currentFilterIds,
+ );
Review Comment:
Debug console.log statement should be removed before merging to production.
This appears to be leftover debugging code that adds unnecessary noise to the
console.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
