mcgilman commented on code in PR #10910:
URL: https://github.com/apache/nifi/pull/10910#discussion_r3574038045
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/users/state/user-listing/user-listing.effects.ts:
##########
@@ -263,17 +276,24 @@ export class UserListingEffects {
ofType(UserListingActions.createUserSuccess),
map((action) => action.response),
filter((response) => response.userGroupUpdate != null),
- mergeMap((createUserResponse) =>
- this.actions$.pipe(
- ofType(UserListingActions.updateUserGroupSuccess),
+ mergeMap((createUserResponse) => {
+ const expectedCount =
createUserResponse.userGroupUpdate?.userGroups?.length ?? 0;
+ if (expectedCount === 0) {
+ return of(UserListingActions.createUserComplete({
response: createUserResponse }));
+ }
Review Comment:
This `expectedCount === 0` early return causes a duplicate completion.
`createUserSuccess$` already emits `createUserComplete` for the empty case
(`userGroupUpdates.length === 0`), and this effect fires on the same
`createUserSuccess` action, so creating a user with no groups completes twice —
double `closeAll()`, `selectTenant()`, and `loadTenants()`. This is a common
path, since `createUser` always carries a non-null `userGroupUpdate`
(`userGroups` is `[]` when nothing is selected). At base this effect emitted
nothing when no group updates were pending, so `createUserSuccess$` was the
sole completer.
Return `EMPTY` here and let `createUserSuccess$` own the zero-group
completion:
```suggestion
if (expectedCount === 0) {
return EMPTY;
}
```
`EMPTY` needs adding to the `rxjs` import. A test that runs a no-group
`createUserSuccess` through both effects and asserts a single
`createUserComplete` would guard this — the current isolated specs don't run
the two together.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/users/state/user-listing/user-listing.effects.ts:
##########
@@ -263,17 +276,24 @@ export class UserListingEffects {
ofType(UserListingActions.createUserSuccess),
map((action) => action.response),
filter((response) => response.userGroupUpdate != null),
- mergeMap((createUserResponse) =>
- this.actions$.pipe(
- ofType(UserListingActions.updateUserGroupSuccess),
+ mergeMap((createUserResponse) => {
+ const expectedCount =
createUserResponse.userGroupUpdate?.userGroups?.length ?? 0;
+ if (expectedCount === 0) {
+ return of(UserListingActions.createUserComplete({
response: createUserResponse }));
+ }
+ return this.actions$.pipe(
+ ofType(UserListingActions.updateUserGroupSuccess,
UserListingActions.updateUserGroupError),
filter(
- (updateSuccess) =>
+ (action) =>
// @ts-ignore
Review Comment:
The `@ts-ignore` on the count lines is resolved. This `filter` predicate
(and the equivalent one at line 575) still carries one for the `'response' in
action ? action.response.requestId : action.requestId` discriminant. Since
`updateUserGroupSuccess`/`updateUserGroupError` are a clean discriminated pair,
this can be typed directly and the suppression dropped. Minor.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/users/state/user-listing/user-listing.effects.ts:
##########
@@ -539,17 +559,28 @@ export class UserListingEffects {
ofType(UserListingActions.updateUserSuccess),
map((action) => action.response),
filter((response) => response.userGroupUpdate != null),
- mergeMap((updateUserResponse) =>
- this.actions$.pipe(
- ofType(UserListingActions.updateUserGroupSuccess),
+ mergeMap((updateUserResponse) => {
+ const addedCount =
updateUserResponse.userGroupUpdate?.userGroupsAdded?.length ?? 0;
+ const removedCount =
updateUserResponse.userGroupUpdate?.userGroupsRemoved?.length ?? 0;
+ const expectedCount = addedCount + removedCount;
+
+ if (expectedCount === 0) {
+ return of(UserListingActions.updateUserComplete());
+ }
Review Comment:
Same duplicate-completion issue as the create path: `updateUserSuccess$`
already returns `updateUserComplete` for the empty case. This branch isn't
reachable from the UI today (`updateUser` only carries a `userGroupUpdate` when
there's at least one add/remove), but returning `EMPTY` keeps the two await
effects symmetric and avoids a duplicate if that invariant changes:
```suggestion
if (expectedCount === 0) {
return EMPTY;
}
```
--
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]
