Skip to content

chore(user): set default BusinessWritePermission for system admins in…#614

Merged
iwanghc merged 1 commit into
mainfrom
dms/feat-813
May 12, 2026
Merged

chore(user): set default BusinessWritePermission for system admins in…#614
iwanghc merged 1 commit into
mainfrom
dms/feat-813

Conversation

@iwanghc
Copy link
Copy Markdown
Collaborator

@iwanghc iwanghc commented May 12, 2026

User description

… AddUser function.

关联的 issue

https://github.com/actiontech/dms-ee/issues/813

描述你的变更

添加用户时,如果为系统管理员权限且未设置业务写参数,默认为开启

确认项(pr提交后操作)

Tip

请在指定复审人之前,确认并完成以下事项,完成后✅


  • 我已完成自测
  • 我已记录完整日志方便进行诊断
  • 我已在关联的issue里补充了实现方案
  • 我已在关联的issue里补充了测试影响面
  • 我已确认了变更的兼容性,如果不兼容则在issue里标记 not_compatible
  • 我已确认了是否要更新文档,如果要更新则在issue里标记 need_update_doc


Description

  • 更新用户结构体注释描述

  • 修改新增用户参数中默认业务写权限处理逻辑

  • 修正更新用户时非管理员的业务写权限默认值

  • 在服务层根据权限列表设置系统管理员默认业务写权限


Diagram Walkthrough

flowchart LR
  A["修改用户结构体和参数"]
  B["更新更新用户逻辑"]
  C["新增服务层判断逻辑"]
  A -- "同步业务写权限逻辑" --> B
  B -- "传递正确默认值" --> C
Loading

File Walkthrough

Relevant files
Bug fix
user.go
调整用户结构体及更新逻辑                                                                                         

internal/dms/biz/user.go

  • 修改业务写权限的注释说明
  • 移除 nil 默认值提示
  • 调整更新用户时非管理员默认权限设置为 false
+4/-4     
user.go
增加服务层默认权限判断                                                                                           

internal/dms/service/user.go

  • 新增对 nil 权限的默认判断逻辑
  • 根据权限列表设置系统管理员默认业务写权限
  • 修改传递给业务逻辑的参数
+12/-1   

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 12, 2026

PR Reviewer Guide 🔍

(Review updated until commit 6f289f5)

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

拼写错误

在更新用户逻辑中,注释描述将业务写权限设置为 false 时错误地拼写为“flase”。建议修正该描述以避免混淆。

	// User is not system administrator: always reset BWP to flase
	user.BusinessWritePermission = false
}

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 12, 2026

PR Code Suggestions ✨

Latest suggestions up to 6f289f5
Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
修正注释拼写

请将注释中的 "flase" 更正为 "false",以确保注释与实际代码逻辑一致。注释错误可能导致团队成员误解代码意图,从而增加维护难度。

internal/dms/biz/user.go [888-889]

-// User is not system administrator: always reset BWP to flase
+// User is not system administrator: always reset BWP to false
 user.BusinessWritePermission = false
Suggestion importance[1-10]: 3

__

Why: The suggestion only fixes a minor typo in the comment ("flase" to "false") without impacting functionality, so it has a low impact.

Low

Previous suggestions

Suggestions up to commit 930640b
CategorySuggestion                                                                                                                                    Impact
Possible issue
明确默认权限值

目前如果用户不具备系统管理员权限,businessWritePermission 保持 nil,可能会导致下游对 nil 值的不当处理而引发
panic。建议在遍历结束后,如果 businessWritePermission 依然为 nil,则明确赋值为 false。

internal/dms/service/user.go [100-109]

 businessWritePermission := req.User.BusinessWritePermission
 if businessWritePermission == nil {
 	for _, uid := range req.User.OpPermissionUids {
 		if uid == pkgConst.UIDOfOpPermissionGlobalManagement {
 			t := true
 			businessWritePermission = &t
 			break
 		}
 	}
+	if businessWritePermission == nil {
+		f := false
+		businessWritePermission = &f
+	}
 }
Suggestion importance[1-10]: 8

__

Why: The suggestion enhances safety by explicitly assigning a default false value to businessWritePermission if it remains nil, which can prevent potential nil pointer issues downstream. This error handling improvement is relevant and appropriately addresses a critical edge case.

Medium

@github-actions
Copy link
Copy Markdown

Persistent review updated to latest commit 6f289f5

@iwanghc iwanghc merged commit 76e4847 into main May 12, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants