Skip to content

Commit

Permalink
Remove redundant internal event in the User Permissions module (#573)
Browse files Browse the repository at this point in the history
<!-- Thank you for submitting a Pull Request. If you're new to
contributing to BCApps please read our pull request guideline below
* https://github.com/microsoft/BCApps/Contributing.md
-->
#### Problem:
`NavUserAccountHelper.IsPermissionSetAssigned` doesn't work with
buffered inserts, so when user records are inserted without a `Commit`
(e. g. in tests), it throws the "User does not exist" exception. So far
this problem has been solved with an internal event that skips the call
alltogether in tests.

#### Solution:
Call `User.Get` before calling
`NavUserAccountHelper.IsPermissionSetAssigned` to make sure buffered
inserts are flushed.

#### Work Item(s) <!-- Add the issue number here after the #. The issue
needs to be open and approved. Submitting PRs with no linked issues or
unapproved issues is highly discouraged. -->
Fixes
[AB#497548](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/497548)
  • Loading branch information
IhorHandziuk authored Feb 13, 2024
1 parent 6139686 commit fa23e93
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -236,14 +236,15 @@ codeunit 153 "User Permissions Impl."

procedure HasUserPermissionSetAssigned(UserSecurityId: Guid; Company: Text; RoleId: Code[20]; ItemScope: Option; AppId: Guid): Boolean
var
User: Record User;
NavUserAccountHelper: DotNet NavUserAccountHelper;
Skip: Boolean;
begin
if HasUserPermissionSetDirectlyAssigned(UserSecurityId, Company, RoleId, ItemScope, AppId) then
exit(true);

OnHasUserPermissionSetAssigned(Skip);
if Skip then
// NavUserAccountHelper doesn't work with bulk (buffered) inserts.
// Calling a Get flushes the buffer.
if not User.Get(UserSecurityId) then
exit(false);

if NavUserAccountHelper.IsPermissionSetAssigned(UserSecurityId, '', RoleId, AppId, ItemScope) then
Expand Down Expand Up @@ -301,13 +302,4 @@ codeunit 153 "User Permissions Impl."
local procedure OnCanManageUsersOnTenant(UserSID: Guid; var Result: Boolean)
begin
end;

/// <summary>
/// Allows the subscriber library to skip calls to NavUserAccountHelper in tests.
/// </summary>
/// <param name="Skip">Skip calls to NavUserAccountHelper.IsPermissionSetAssigned.</param>
[InternalEvent(false)]
local procedure OnHasUserPermissionSetAssigned(var Skip: Boolean)
begin
end;
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,5 @@ codeunit 130019 "Test User Permissions Subs."
if CanManageUserSecIDs.Contains(UserSID) then
Result := true;
end;

/// <summary>
/// Skip calls to NavUserAccountHelper.IsPermissionSetAssigned, as it may fail in tests.
/// </summary>
/// <param name="Skip">Skip calls NavUserAccountHelper.IsPermissionSetAssigned.</param>
[EventSubscriber(ObjectType::Codeunit, Codeunit::"User Permissions Impl.", 'OnHasUserPermissionSetAssigned', '', false, false)]
local procedure OnHasUserPermissionSetAssigned(var Skip: Boolean)
begin
Skip := true;
end;
}

0 comments on commit fa23e93

Please sign in to comment.