-
Notifications
You must be signed in to change notification settings - Fork 186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: 数据范围实现(model use trait实现) #561
base: master-department
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthrough此更改引入了一整套数据权限控制机制。新增的 DataScope 类通过静态方法 Changes
Sequence Diagram(s)sequenceDiagram
participant M as 模型 (DataScopeTrait)
participant S as DataScopeScope
participant D as DataScope
participant B as 构造器
M->>S: boot() 应用全局作用域
S->>B: 调用 apply() 处理查询
S->>D: 调用 query() 检查数据权限
D->>B: 检查表字段及用户权限,应用过滤
B-->>D: 返回修改后的构造器
D-->>S: 返回过滤结果
S-->>M: 返回最终查询构造器
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (8)
app/Model/Scopes/DataScopeScope.php (2)
15-23
: 移除不必要的 use 声明以保持代码整洁
在第 15-23 行声明的命名空间导入中,有多处未在本文件中实际使用(例如App\Http\CurrentUser
、App\Model\DataPermission\Policy
、App\Model\Enums\DataPermission\PolicyType
、App\Model\Permission\Department
、Hyperf\Database\Schema\Schema
等)。建议删除这些未使用的use
语句,以减少维护成本并提高可读性。
38-38
: 文件末尾缺少换行符
根据流水线提示(No newline at end of file),请在文件末尾添加换行符,避免某些工具或系统出现兼容性问题。app/Model/Traits/DataScopeTrait.php (2)
17-17
: 将 dataScope 属性设为受保护或者私有
将$dataScope
声明为 public 会允许任意外部修改该值,容易破坏数据范围逻辑。建议将其设为 protected 或 private,并通过专用方法进行访问,以满足封装性需求。
36-36
: 文件末尾缺少换行符
管线提示文件末尾缺换行符,为保持规范请补充对应换行。app/Model/DataScope.php (4)
26-26
: 注释中的字段拼写与代码不一致
第 26 行注释将 “created_by” 写成 “creatd_by”,建议修正以保持统一,避免与实际字段名不符导致混淆。
117-117
: 对自定义函数进行安全性检查
目前只检查function_exists($funcName)
,如果funcName
由用户输入而未限制范围,存在一定安全风险。可考虑增加白名单或更严格的函数名验证,防止执行不可信的函数。
164-172
: 递归获取子部门可能存在性能隐患
对于部门层级较深的情况,特别是数据量大时,递归调用容易造成性能下降和堆栈开销。可考虑迭代方式或在表中维护层级路径(path),以更高效地查询子部门并减少递归调用。
173-173
: 文件末尾缺少换行符
同样需要在文件末尾添加换行符,符合常规编码规范并避免某些工具链或系统环境出现问题。
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/Model/DataScope.php
(1 hunks)app/Model/Scopes/DataScopeScope.php
(1 hunks)app/Model/Traits/DataScopeTrait.php
(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: MineAdmin Unit and Ci Test
app/Model/Scopes/DataScopeScope.php
[warning] 1-1: No newline at end of file
app/Model/Traits/DataScopeTrait.php
[warning] 12-12: No newline at end of file
app/Model/DataScope.php
[warning] 24-24: No newline at end of file
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build Code coverage report (ubuntu-latest, 8.1, v5.1.3)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
app/Model/Scopes/DataScopeScope.php (1)
30-37
: 确认 getDataScope 方法的返回类型
当前仅通过if ($model->getDataScope())
判断布尔值,建议在方法或注释中说明返回值的含义(如 true/false 对应哪些数据范围场景),并为其编写单元测试以确保开启/关闭数据范围时逻辑一致。app/Model/Traits/DataScopeTrait.php (1)
19-24
: 确认父类是否存在同名的 boot 方法
由于在本 trait 中定义了protected function boot()
,需要确保不会与父类或其他 trait 同名方法冲突。若有重名风险,考虑改用bootDataScopeTrait
或其他机制防止覆盖。也可添加测试确保全局作用域的正确添加。
// 这里是一个简单的处理方案,如果有自定义函数,我们暂时将其作为一个单独的Or条件 | ||
// 真实场景中可能需要更复杂的合并逻辑 | ||
return $builder->where(function ($query) use ($table, $createdByIds, $deptIds, $customQuery) { | ||
// 应用其他范围条件 | ||
if (!$deptIds->isEmpty()) { | ||
$query->orWhereIn("{$table}.dept_id", $deptIds->unique()->values()->toArray()); | ||
} | ||
if (!$createdByIds->isEmpty()) { | ||
$query->orWhereIn("{$table}.created_by", $createdByIds->unique()->values()->toArray()); | ||
} | ||
|
||
// 添加自定义查询的条件(简化处理,实际可能需要更复杂的逻辑) | ||
$query->orWhereRaw('EXISTS (' . $customQuery->toSql() . ')', $customQuery->getBindings()); | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
自定义查询合并方式过于简化,可能影响数据范围准确性
在这里通过 orWhereRaw('EXISTS (...)')
与自定义查询合并,实际场景或复杂度更高时,可能导致数据范围不一致或性能问题。建议采用更完善的处理方案并进行验证,以确保不同策略能合理组合。
Summary by CodeRabbit