Skip to content
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.

Sign up for GitHub

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

fix(seeder): 修正菜单表按钮标识 #537

Closed
wants to merge 2 commits into from
Closed

fix(seeder): 修正菜单表按钮标识 #537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
94521c9
fix(seeder): 修正菜单表按钮标识
he426100 Jan 16, 2025
a49a5ee
fix(menu): 同步更新权限标识
he426100 Jan 16, 2025
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
4 changes: 2 additions & 2 deletions app/Http/Admin/Controller/Permission/MenuController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public function pageList(RequestInterface $request): Result
content: new JsonContent(ref: MenuRequest::class, title: '创建菜单')
)]
#[PageResponse(instance: new Result())]
#[Permission(code: 'permission:menu:create')]
#[Permission(code: 'permission:menu:save')]
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

建议重新考虑权限标识的变更策略

控制器中将创建菜单的权限从 permission:menu:create 改为 permission:menu:save 存在以下问题：

  1. 破坏了 RESTful 风格的语义（create 更符合创建操作）
  2. 与现有系统的权限体系不兼容
  3. 需要所有使用方重新配置权限

建议：

  1. 保留原有权限标识
  2. 如确需变更，建议采用别名机制，同时支持新旧两种权限标识

public function create(MenuRequest $request): Result
{
$this->service->create(array_merge($request->validated(), [
Expand All @@ -92,7 +92,7 @@ public function create(MenuRequest $request): Result
content: new JsonContent(ref: MenuRequest::class, title: '编辑菜单')
)]
#[PageResponse(instance: new Result())]
#[Permission(code: 'permission:menu:save')]
#[Permission(code: 'permission:menu:update')]
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

🛠️ Refactor suggestion

权限变更需要提供迁移方案

在将权限标识从 permission:menu:save 更改为 permission:menu:update 时，发现以下问题：

  1. 缺少数据库迁移脚本来更新现有系统中的权限标识
  2. 前端代码中仍然使用旧的权限标识 permission:menu:save

建议：

  • 创建数据库迁移脚本以更新现有权限记录
  • 更新前端代码中的权限标识
  • 在升级文档中说明此变更
🔗 Analysis chain

权限命名需符合语义化原则

将保存方法的权限从 permission:menu:save 改为 permission:menu:update 虽然在语义上更准确，但需要注意：

  1. 确保在数据库迁移脚本中包含权限标识的更新
  2. 在升级指南中提供明确的迁移步骤
  3. 考虑提供脚本自动更新现有系统的权限配置
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 检查是否存在相关的数据库迁移文件
fd -e php -e sql seeder menu

Length of output: 122

Script:

#!/bin/bash
# Search for migration files and seeders
fd -e php migration
fd -e php seeder

# Search for old and new permission codes
rg -l "permission:menu:save"
rg -l "permission:menu:update"

# Search for permission-related files
fd -e php permission

Length of output: 918

Script:

#!/bin/bash
# Check content of the seeder file
cat databases/seeders/menu_seeder_20240926.php

# Check the test file content
cat tests/Feature/Admin/Permission/MenuControllerTest.php

# Check the frontend view file
cat web/src/modules/base/views/permission/menu/menu-tree.vue

Length of output: 28152

public function save(int $id, MenuRequest $request): Result
{
$this->service->updateById($id, array_merge($request->validated(), [
Expand Down
4 changes: 2 additions & 2 deletions databases/seeders/menu_seeder_20240926.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,15 +167,15 @@ public function data(): array
]),
],
[
'name' => 'permission:menu:create',
'name' => 'permission:menu:save',
'meta' => new Meta([
'title' => '菜单保存',
'type' => 'B',
'i18n' => 'baseMenu.permission.menuSave',
]),
],
Comment on lines +170 to 176
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

需要同步更新后端代码中的权限标识

发现前后端代码存在权限标识不一致的情况：

  • 前端代码已使用新的 permission:menu:save 权限标识
  • 后端代码仍在使用旧的 permission:menu:create 权限标识

需要更新以下文件：

  1. app/Http/Admin/Controller/Permission/MenuController.php 中的 Permission 注解
  2. tests/Feature/Admin/Permission/MenuControllerTest.php 中的测试用例
🔗 Analysis chain

菜单标识修改符合系统规范

将菜单管理模块的按钮标识修改为与用户管理、角色管理模块保持一致的命名方式，提高了系统的一致性。

请确认以下相关变更是否已同步更新：

  1. 前端路由配置
  2. 权限检查代码
  3. 相关的API接口路径

运行以下脚本以验证相关文件是否需要更新：

Also applies to: 178-184

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述：检查是否有使用旧的菜单标识的代码

# 搜索可能使用旧标识的文件
echo "检查前端路由配置..."
rg -l "permission:menu:create"

echo "检查权限相关代码..."
rg -l "permission\.menu\.create"

echo "检查API路由..."
rg -l "menu/create"

Length of output: 515

Script:

#!/bin/bash
# 检查文件内容和其他可能的标识符使用

echo "===== MenuController.php 内容 ====="
rg -A 5 -B 5 "permission:menu:create" app/Http/Admin/Controller/Permission/MenuController.php

echo "===== MenuControllerTest.php 内容 ====="
rg -A 5 -B 5 "permission:menu:create" tests/Feature/Admin/Permission/MenuControllerTest.php

echo "===== 检查其他相关权限标识 ====="
rg -l "menu.*?(create|save)"

echo "===== 检查权限检查代码 ====="
ast-grep --pattern 'can($_, "permission:menu:create")'

Length of output: 1644

Script:

#!/bin/bash
# 检查前端代码和数据库迁移文件

echo "===== 检查前端语言文件 ====="
rg -A 2 -B 2 "menu.*?(create|save)" web/src/modules/base/locales/en\[English\].yaml

echo "===== 检查菜单树组件 ====="
rg -A 5 -B 5 "menu.*?(create|save)" web/src/modules/base/views/permission/menu/menu-tree.vue

echo "===== 检查数据库迁移文件 ====="
rg -A 2 -B 2 "permission:menu:(create|save)" databases/seeders/menu_update_20241029.php

Length of output: 984

[
'name' => 'permission:menu:save',
'name' => 'permission:menu:update',
'meta' => new Meta([
'title' => '菜单更新',
'type' => 'B',
Expand Down
4 changes: 2 additions & 2 deletions tests/Feature/Admin/Permission/MenuControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public function testPageList(): void

public function testCreate(): void
{
$this->caseCreate('/admin/menu', 'permission:menu:create', [
$this->caseCreate('/admin/menu', 'permission:menu:save', [
'parent_id' => 0,
'name' => Str::random(10),
'component' => Str::random(10),
Expand Down Expand Up @@ -59,7 +59,7 @@ public function testSave(): void
'meta' => $this->generatorMeta(),
'path' => Str::random(10),
]);
$this->caseSave('/admin/menu/', $entity, 'permission:menu:save', [
$this->caseSave('/admin/menu/', $entity, 'permission:menu:update', [
'name' => Str::random(10),
'component' => Str::random(10),
'redirect' => Str::random(10),
Expand Down
Loading