refactor: v1 regression#224
Conversation
if has binding user will prevent remove action
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
wuyiping0628
left a comment
There was a problem hiding this comment.
Review Summary
整体是有价值的 v1 回归修复(角色删除校验、用户已存在、改密确认、API baseURL 统一、items/permission 字段对齐等),但合并前建议先处理下列阻塞项。
Must fix
role-table.vue:onPermissionChange未定义,改权限会运行时报错interceptor.ts:401 刷新仍有竞态 / 未重试原请求 / 无 refresh 时未 reject;刷新成功用router.go(0)偏粗暴.vscode/settings.json:个人 Lua/Redis 配置不应入库zhCN/exception.json:缺少role.NOT_EMPTY/user.PASSWORD_NOT_MATCH等中文文案(仅改了 enUS)
Should fix
update-role.command.ts:先删后写未包事务,失败会丢权限/菜单batch-remove-user.ts:UserRole删除未走事务内emget-role-total.query.ts:过滤条件缓存与 create/remove 的 incr/decr 不一致auth.guard.ts:作者曾标注调试完回滚,请确认当前DomainError透传是否为正式方案
Nice to have
add-role.vue权限展示建议item.desc || item.namepermissionIds.map(item => item)可简化- i18
batchRemove/remove校验变弱
感谢贡献,修完上述几项后再合会更稳。
| @Post('/batch') | ||
| batchRemove(@Body() body: BatchRemoveI18Records) { | ||
| return this.i18Service.batchRemove(body.ids); | ||
| batchRemove(@Body() body: I18nRecordId[]) { |
There was a problem hiding this comment.
去掉 BatchRemoveI18Records DTO 后,body 变为裸数组,Swagger 文档和校验都变弱了。若刻意简化,建议至少保留基础校验;remove 去掉 ParseIntPipe 也有同样问题(见上方 :id)。
3513fd2
into
opentiny:refactor/next-nest-js-backend
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information