Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {I18nFieldValue} from '../../data-fields/i18n-field/models/i18n-field-val
import {LanguageService} from '../../translate/language.service';
import {LoadingEmitter} from '../../utility/loading-emitter';
import {PathService} from "../service/path.service";
import {GroupNavigationConstants} from "../model/group-navigation-constants";


@Component({
Expand Down Expand Up @@ -257,9 +258,18 @@ export abstract class AbstractDashboardComponent {

public navigate(itemCase: Case) {
if (this.getItemInternal(itemCase)) {
const itemPath = this._doubleDrawerNavigationService.getItemRoutingPath(this.dashboardItemsMapping[itemCase.stringId]);
this._pathService.activePath = itemPath;
this._router.navigate([itemPath]);
const menuItemCase = this.dashboardItemsMapping[itemCase.stringId];
if (!menuItemCase) {
this._log.warn(`No mapped menu item for dashboard item ${itemCase.stringId}`);
return;
}
const itemRoute = this._doubleDrawerNavigationService.getItemRoutingPath(menuItemCase);
this._pathService.activePath = this.getFieldValue(menuItemCase, GroupNavigationConstants.ITEM_FIELD_ID_NODE_PATH);
const nodePath = this.getFieldValue(menuItemCase, GroupNavigationConstants.ITEM_FIELD_ID_NODE_PATH);
if (nodePath) {
this._pathService.activePath = nodePath;
}
this._router.navigate([itemRoute]);
} else {
window.open(this.getItemURL(itemCase), "_blank");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,11 @@ export abstract class AbstractNavigationDoubleDrawerComponent implements OnInit,

this._userService.user$.pipe(filter(u => !!u && u.id !== ''), take(1)).subscribe(() => {
this._currentPathSubscription = this._pathService.activePath$.subscribe(path => {
this.currentPath = path;
if (path !== this.currentPath) {
this.currentPath = path;
} else {
this.openAvailableView();
}
Comment on lines +129 to +133
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Scope and de-duplicate auto-open to avoid surprise navigation and loops

Else-branch now calls openAvailableView() whenever activePath$ re-emits the same value. This bypasses the earlier canApplyAutoSelect() guard (used below on rightItems$) and can:

  • Trigger auto-navigation on routes where it wasn’t intended.
  • Repeatedly call openAvailableView() if the same path re-emits (possible re-entrancy, flicker).

Guard by (a) scoping to canApplyAutoSelect(), (b) ensuring right items are available, and (c) making it one-shot per path.

Apply this diff within the subscription:

-                if (path !== this.currentPath) {
-                    this.currentPath = path;
-                } else {
-                    this.openAvailableView();
-                }
+                if (path !== this.currentPath) {
+                    this.currentPath = path;
+                    this._autoOpenForPath = undefined; // reset one-shot flag
+                } else if (this.canApplyAutoSelect()
+                    && ((this.rightItems?.length ?? 0) > 0)
+                    && this._autoOpenForPath !== path) {
+                    this._autoOpenForPath = path;
+                    this.openAvailableView();
+                }

Add this helper field to the class (near other protected fields):

private _autoOpenForPath?: string;

Verification suggestions:

  • Confirm openAvailableView() is idempotent when invoked multiple times for the same path.
  • Validate that auto-open only occurs on the configured URL and only once per path.
    Do you want me to add a focused unit test that asserts a single openAvailableView() invocation when activePath$ re-emits the same value?

Also applies to: 136-143


🏁 Script executed:

#!/bin/bash
set -e
# Locate openAvailableView definition
rg -n 'openAvailableView' -C3 -g '*.ts'
# Locate canApplyAutoSelect definition and usage
rg -n 'canApplyAutoSelect' -C3 -g '*.ts'
# Inspect subscription logic around currentPath
rg -n -C3 'if \(path !== this\.currentPath\)' projects/netgrif-components-core/src/lib/navigation/navigation-double-drawer/abstract-navigation-double-drawer.ts
# Inspect use of rightItems
rg -n 'rightItems' -C3 projects/netgrif-components-core/src/lib/navigation/navigation-double-drawer/abstract-navigation-double-drawer.ts

Length of output: 8795


🏁 Script executed:

#!/bin/bash
# Show class declaration and field definitions in abstract-navigation-double-drawer.ts
sed -n '1,200p' projects/netgrif-components-core/src/lib/navigation/navigation-double-drawer/abstract-navigation-double-drawer.ts

# Locate any plain `rightItems` property declarations in this file
grep -R "rightItems\s*[:=]" -n projects/netgrif-components-core/src/lib/navigation/navigation-double-drawer/abstract-navigation-double-drawer.ts

Length of output: 8392


Guard and de-duplicate auto-open in AbstractNavigationDoubleDrawerComponent

Add a one-shot flag and apply the guard in the activePath$ subscription to prevent unintended loops and bypassing of canApplyAutoSelect():

  1. Near the other protected fields, add:

    private _autoOpenForPath?: string;
  2. In the activePath$ subscription (lines 129–133), replace:

    - if (path !== this.currentPath) {
    -     this.currentPath = path;
    - } else {
    -     this.openAvailableView();
    - }

    with:

    if (path !== this.currentPath) {
        this.currentPath = path;
        this._autoOpenForPath = undefined;
    } else if (
        this.canApplyAutoSelect()
        && ((this.rightItems?.length ?? 0) > 0)
        && this._autoOpenForPath !== path
    ) {
        this._autoOpenForPath = path;
        this.openAvailableView();
    }
  3. Apply a similar one-shot guard to the rightItems$ subscription (lines 136–143) if auto-open should only ever fire once per path.

Would you like a focused unit test that asserts openAvailableView() is invoked exactly once when activePath$ re-emits the same value?

🤖 Prompt for AI Agents
In
projects/netgrif-components-core/src/lib/navigation/navigation-double-drawer/abstract-navigation-double-drawer.ts
around lines 129 to 133, the activePath$ subscription can trigger
openAvailableView repeatedly and bypass canApplyAutoSelect(); add a one-shot
guard field (private _autoOpenForPath?: string) near the other protected fields,
then update the activePath$ handler so that when path !== this.currentPath you
set this.currentPath = path and clear _autoOpenForPath, else only call
openAvailableView if canApplyAutoSelect() is true, there are rightItems, and
_autoOpenForPath !== path, and before calling openAvailableView set
_autoOpenForPath = path; also consider applying the same one-shot guard to the
rightItems$ subscription so auto-open fires at most once per path.

});

if (this.canApplyAutoSelect()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export class DoubleDrawerNavigationService implements OnDestroy {
protected _redirectService: RedirectService,
protected _pathService: PathService,
protected _userService: UserService,
) {
) {
this._leftItems$ = new BehaviorSubject([]);
this._rightItems$ = new BehaviorSubject([]);
this._moreItems$ = new BehaviorSubject([]);
Expand Down Expand Up @@ -439,6 +439,8 @@ export class DoubleDrawerNavigationService implements OnDestroy {
this.resolveCustomViewsInRightSide();
this._rightLoading$.off();
this.itemLoaded.emit({menu: 'right', items: this.rightItems});

this.openAvailableView();
}, error => {
Comment on lines +442 to 444
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Avoid double auto-open triggers.

openAvailableView() is called here and also via the subscription in onItemClick. This can cause duplicate navigations.

Minimal change: rely on the post-load call and remove the subscription.

-                this._rightLoading$.pipe(
-                    filter(isRightLoading => isRightLoading === false),
-                    take(1)
-                ).subscribe(() => {
-                    this.openAvailableView();
-                })
+                // Auto-open is handled in loadRightSide() after items finish loading.

Run to find other call sites to confirm no remaining duplicates:

Also applies to: 269-275


🏁 Script executed:

#!/bin/bash
rg -n "openAvailableView\\(" -C2

Length of output: 3958


🏁 Script executed:

#!/bin/bash
sed -n '250,340p' projects/netgrif-components-core/src/lib/navigation/navigation-double-drawer/service/double-drawer-navigation.service.ts

Length of output: 4357


🏁 Script executed:

#!/bin/bash
sed -n '120,160p' projects/netgrif-components-core/src/lib/navigation/navigation-double-drawer/abstract-navigation-double-drawer.ts

Length of output: 1550


🏁 Script executed:

#!/bin/bash
sed -n '400,480p' projects/netgrif-components-core/src/lib/navigation/navigation-double-drawer/service/double-drawer-navigation.service.ts

Length of output: 3484


🏁 Script executed:

#!/bin/bash
rg -n "protected loadLeftSide" -C3 projects/netgrif-components-core/src/lib/navigation/navigation-double-drawer/service/double-drawer-navigation.service.ts

Length of output: 237


🏁 Script executed:

#!/bin/bash
sed -n '1,120p' projects/netgrif-components-core/src/lib/navigation/navigation-double-drawer/service/double-drawer-navigation.service.ts

Length of output: 5236


Remove duplicate auto-open in onItemClick
The _rightLoading$.pipe(...).subscribe(() => this.openAvailableView()) block in onItemClick fires in addition to the openAvailableView() call at the end of loadRightSide(), causing double navigations. Delete that subscription and rely on the post-load auto-open.

-                this._rightLoading$.pipe(
-                    filter(isRightLoading => isRightLoading === false),
-                    take(1)
-                ).subscribe(() => {
-                    this.openAvailableView();
-                })
+                // auto-open is handled in loadRightSide() after items finish loading
📝 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.

Suggested change
this.openAvailableView();
}, error => {
// auto-open is handled in loadRightSide() after items finish loading
this.openAvailableView();
}, error => {
🤖 Prompt for AI Agents
In
projects/netgrif-components-core/src/lib/navigation/navigation-double-drawer/service/double-drawer-navigation.service.ts
around lines 442-444, there is a redundant subscription inside onItemClick:
remove the _rightLoading$.pipe(...).subscribe(() => this.openAvailableView())
block so it no longer calls openAvailableView() from this subscription; rely on
the existing openAvailableView() invocation that occurs after loadRightSide()
completes to perform the post-load auto-open, and ensure no other code path
reintroduces an immediate open to avoid double navigation.

this._log.error(error);
this._rightItems$.next([]);
Expand Down Expand Up @@ -492,7 +494,7 @@ export class DoubleDrawerNavigationService implements OnDestroy {
const resolvedBannedRoles = DoubleDrawerUtils.resolveAccessRoles(itemCase, GroupNavigationConstants.ITEM_FIELD_ID_BANNED_ROLES);
if (!!resolvedRoles) item.access['role'] = resolvedRoles;
if (!!resolvedBannedRoles) item.access['bannedRole'] = resolvedBannedRoles;
if (!this._accessService.canAccessView(item, item.routingPath)) return;
if (!this._accessService.canAccessView(item, item.routing?.path)) return;
return item;
}

Expand Down Expand Up @@ -545,7 +547,8 @@ export class DoubleDrawerNavigationService implements OnDestroy {
public getItemRoutingPath(itemCase: Case) {
const taskId = DoubleDrawerUtils.findTaskIdInCase(itemCase, SETTINGS_TRANSITION_ID);
const url = this._dynamicRoutingService.route;
return `/${url}/${taskId}`;
const prefix = url.startsWith('/') ? '' : '/';
return `${prefix}${url}/${taskId}`;
}

private processLeftItems(cases: Case[], orderedChildCaseIds: string[]): NavigationItem[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {Component, Input, OnDestroy, OnInit, Optional} from '@angular/core';
import {MatExpansionPanel} from '@angular/material/expansion';
import {AbstractPanelWithHeaderBindingComponent} from '../abstract/panel-with-header-binding';
import {HeaderColumn} from '../../header/models/header-column';
import {Observable, Subscription} from 'rxjs';
import {Subscription} from 'rxjs';
import {LoggerService} from '../../logger/services/logger.service';
import {toMoment} from '../../resources/types/nae-date-type';
import {DATE_TIME_FORMAT_STRING} from '../../moment/time-formats';
Expand All @@ -18,7 +18,6 @@ import {PetriNetResourceService} from '../../resources/engine-endpoint/petri-net
import {ProgressType, ProviderProgress} from '../../resources/resource-provider.service';
import {OverflowService} from '../../header/services/overflow.service';


export interface WorkflowPanelContent {
netIdentifier: TextField;
title: TextField;
Expand Down Expand Up @@ -160,15 +159,19 @@ export abstract class AbstractWorkflowPanelComponent extends AbstractPanelWithHe
case WorkflowMetaField.INITIALS:
return {value: this.workflow.initials, icon: '', type: 'meta'};
case WorkflowMetaField.TITLE:
return {value: this.workflow.title, icon: '', type: 'meta'};
return {value: this.workflow.title || this.workflow.identifier, icon: '', type: 'meta'};
case WorkflowMetaField.NET_ID:
return {value: this.workflow.stringId, icon: '', type: 'meta'};
case WorkflowMetaField.VERSION:
return {value: this.workflow.version, icon: '', type: 'meta'};
case WorkflowMetaField.AUTHOR:
return {value: this.workflow.author.fullName, icon: 'account_circle', type: 'meta'};
case WorkflowMetaField.CREATION_DATE:
return {value: toMoment(this.workflow.createdDate).format(DATE_TIME_FORMAT_STRING), icon: 'event', type: 'meta'};
return {
value: toMoment(this.workflow.createdDate).format(DATE_TIME_FORMAT_STRING),
icon: 'event',
type: 'meta'
};
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export class CaseProcess extends NoConfigurationAutocompleteCategory<string> {

this._allowedNetsSub = this._optionalDependencies.allowedNetsService.allowedNets$.subscribe(allowedNets => {
this._optionsMap.clear();
Comment on lines 43 to 44
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Guard against duplicate subscriptions

If createOptions can be called multiple times, unsubscribe before resubscribing to avoid leaks/duplicate updates.

-        this._allowedNetsSub = this._optionalDependencies.allowedNetsService.allowedNets$.subscribe(allowedNets => {
+        if (this._allowedNetsSub && !this._allowedNetsSub.closed) {
+            this._allowedNetsSub.unsubscribe();
+        }
+        this._allowedNetsSub = this._optionalDependencies.allowedNetsService.allowedNets$.subscribe(allowedNets => {
📝 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.

Suggested change
this._allowedNetsSub = this._optionalDependencies.allowedNetsService.allowedNets$.subscribe(allowedNets => {
this._optionsMap.clear();
// Unsubscribe existing subscription to avoid duplicate handlers/leaks
if (this._allowedNetsSub && !this._allowedNetsSub.closed) {
this._allowedNetsSub.unsubscribe();
}
this._allowedNetsSub = this._optionalDependencies.allowedNetsService.allowedNets$.subscribe(allowedNets => {
this._optionsMap.clear();
// …rest of your handling logic…
});
🤖 Prompt for AI Agents
In
projects/netgrif-components-core/src/lib/search/models/category/case/case-process.ts
around lines 44-45, the subscription to allowedNets$ is created unconditionally
which can cause duplicate subscriptions/leaks if createOptions is called
multiple times; before assigning this._allowedNetsSub, check if an existing
this._allowedNetsSub exists and unsubscribe it (or call
this._allowedNetsSub?.unsubscribe()), then create the new subscription so only
one active subscription remains; alternatively implement a takeUntil/subject
teardown pattern if preferred.

this._uniqueOptionsMap.clear();
allowedNets.forEach(petriNet => {
if (this.isUniqueOption(petriNet.title, petriNet.identifier)) {
this.addToMap(petriNet.title, petriNet.identifier);
Expand Down
Loading