Skip to content

fix: 修复回收站验收问题#519

Merged
skique merged 1 commit intodidi:feature/peking-2025from
skique:feature/peking-2025
Aug 26, 2025
Merged

fix: 修复回收站验收问题#519
skique merged 1 commit intodidi:feature/peking-2025from
skique:feature/peking-2025

Conversation

@skique
Copy link
Collaborator

@skique skique commented Aug 26, 2025

  1. 字段命名不一致和拼写错误
  2. 菜单项更新逻辑脆弱
  3. 测试数据未模拟依赖
  4. 前端状态管理优化
  5. 接口参数传递问题
  6. 删除操作提示优化

@codecov
Copy link

codecov bot commented Aug 26, 2025

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (feature/peking-2025@9c808b0). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...odules/survey/controllers/recycleBin.controller.ts 0.00% 2 Missing ⚠️
Additional details and impacted files
@@                  Coverage Diff                   @@
##             feature/peking-2025     #519   +/-   ##
======================================================
  Coverage                       ?   79.80%           
======================================================
  Files                          ?      145           
  Lines                          ?     3763           
  Branches                       ?      487           
======================================================
  Hits                           ?     3003           
  Misses                         ?      732           
  Partials                       ?       28           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link

Reasoning Details

我们正在审查一个Pull Request的代码更改。我们将逐个文件分析变更,识别潜在问题,并提供改进建议。

首先,我们分析每个文件的变更:

  1. server/src/enums/exceptionCode.ts

    • 添加了一个新的异常代码 SERVER_ERROR = 5002,用于表示“获取数据失败”。
    • 看起来是合理的扩展,没有问题。
  2. server/src/models/surveyMeta.entity.ts

    • 将字段名从 isCompletedeleted 改为 isCompleteDeleted
    • 修正了拼写错误(将 Completedeleted 改为 CompleteDeleted),这是一个明显的改进。
  3. server/src/modules/survey/__test/collaborator.service.spec.ts

    • 在测试 getManageListByUserId 时,添加了模拟实现和验证调用参数的断言。
    • 之前测试没有实际模拟,现在增加了模拟并验证了查询条件。
    • 改进:测试更完整,验证了方法调用。
  4. server/src/modules/survey/controllers/recycleBin.controller.ts

    • 重构了 getCount 方法,添加了注释,并优化了变量使用。
    • isRecycleBin 硬编码为 true,并删除了多余的变量。
    • 改进了代码可读性,添加了注释说明每一步操作。
  5. server/src/modules/survey/controllers/surveyMeta.controller.ts

    • 在回收站查询时,添加了注释说明。
    • 逻辑没有变化,只是增加了注释,提高了可读性。
  6. server/src/modules/workspace/_test/workspace.service.spec.ts

    • 重写了 getAllSurveyIdListByUserId 的测试用例,增加了多个测试场景:
      • 正常情况:返回两个问卷ID。
      • 用户没有工作空间成员资格:返回空列表。
      • 回收站模式:验证传递的 isDeleted 参数为 true
    • 使用了更具体的模拟数据(ObjectId),并验证了方法调用参数。
    • 改进:测试覆盖更全面。
  7. web/src/management/pages/list/components/RecycleBinList.vue

    • 修改了确认删除的提示信息:将“改问卷”改为“该问卷”,修正了错别字。
  8. web/src/management/pages/list/index.vue

    • isRecycleBin 改为计算属性,基于 menuType 判断。
    • 注释掉了之前直接设置 isRecycleBin.value 的代码,因为现在使用计算属性。
    • 改进:使用计算属性更符合Vue响应式原则,避免手动维护状态。
  9. web/src/management/stores/workSpace.ts

    • getRecycleBinCount 方法中,改为通过查找回收站菜单项来更新计数,而不是硬编码索引。
    • 改进:避免了硬编码索引,更健壮(如果菜单顺序变化也不会出错)。

潜在问题和改进建议:

  1. server/src/modules/survey/controllers/recycleBin.controller.ts 和 surveyMeta.controller.ts

    • 两个控制器中都有类似的逻辑:获取用户管理的协作问卷和用户在工作空间回收站中的问卷,然后合并问卷ID列表。
    • 建议:考虑将这部分逻辑抽取到一个共享函数中,避免重复。但本次变更只是重构和增加注释,没有引入新问题。
  2. server/src/modules/workspace/_test/workspace.service.spec.ts

    • 在测试中,使用了硬编码的ObjectId字符串,但为了更真实,使用了new ObjectId()。不过,在断言中直接使用了字符串形式(例如'507f1f77bcf86cd799439011'),这是正确的,因为ObjectId在转换为字符串后就是这种形式。
    • 注意:在模拟返回的surveyMetaList中,_id字段是ObjectId实例,但在结果中期望的是字符串形式。这是因为在服务方法中,会调用map并转换为字符串。测试中模拟返回的ObjectId实例,在服务方法中会被转换为字符串,所以测试中期望的字符串形式是正确的。
  3. web/src/management/pages/list/index.vue

    • isRecycleBin改为计算属性是一个改进,但注释掉的代码(// isRecycleBin.value = false)应该被删除,以避免混淆。不过,这些注释代码已经被保留,但不再需要。建议在PR中清理掉这些注释行,以保持代码整洁。
  4. web/src/management/stores/workSpace.ts

    • getRecycleBinCount中,现在通过查找菜单项来更新计数,这比硬编码索引更好。但是,如果菜单项中没有回收站菜单(recycleBinMenu为undefined),则不会更新。这可能是预期的,因为如果没有回收站菜单,就不需要更新计数。但为了安全,可以添加一个条件判断,避免在找不到菜单时报错。当前代码已经使用了recycleBinMenu && (recycleBinMenu.count = count),所以是安全的。
  5. 拼写错误修正:在RecycleBinList.vue中修正了错别字(“改问卷”->“该问卷”),这是一个很好的改进。

  6. 测试覆盖:在workspace服务的测试中,增加了多个测试用例,覆盖了正常情况、空列表和回收站模式,这提高了测试质量。

总结:

  • 本次变更主要涉及错误修复、测试增强和代码重构。
  • 没有发现明显的逻辑错误、安全漏洞或代码风格问题。
  • 建议清理掉index.vue中注释掉的代码行,以保持代码整洁。

最终,我们按文件列出问题和建议:

  1. server/src/enums/exceptionCode.ts: 无问题。
  2. server/src/models/surveyMeta.entity.ts: 无问题,修正拼写错误。
  3. server/src/modules/survey/__test/collaborator.service.spec.ts: 无问题,增强了测试。
  4. server/src/modules/survey/controllers/recycleBin.controller.ts: 无问题,代码更清晰。
  5. server/src/modules/survey/controllers/surveyMeta.controller.ts: 无问题,增加了注释。
  6. server/src/modules/workspace/_test/workspace.service.spec.ts: 无问题,测试覆盖更全面。
  7. web/src/management/pages/list/components/RecycleBinList.vue: 修正了错别字,无问题。
  8. web/src/management/pages/list/index.vue:
    • 建议:删除被注释掉的代码(第273、277、281、285行附近的// isRecycleBin.value = false等),以保持代码整洁。
  9. web/src/management/stores/workSpace.ts: 无问题,改进了更新菜单计数的逻辑。

此外,跨控制器的重复逻辑可以考虑抽取,但不在本次PR范围内,可以留待后续优化。

代码审查结果

1. 拼写错误修正

  • 文件: server/src/models/surveyMeta.entity.ts
    • 问题: 字段名 isCompletedeleted 存在拼写错误
    • 改进: 已正确修复为 isCompleteDeleted,符合语义("完全删除"状态)

2. 测试用例增强

  • 文件: server/src/modules/survey/__test/collaborator.service.spec.ts
    • 改进:
      1. 添加了 findSpy 模拟实现和参数验证
      2. 明确测试了 $elemMatch 权限查询逻辑
      3. 确保权限过滤条件 SURVEY_PERMISSION.SURVEY_COOPERATION_MANAGE 被正确使用

3. 回收站逻辑优化

  • 文件: server/src/modules/survey/controllers/recycleBin.controller.ts

    • 改进:
      1. 移除冗余变量 isRecycleBin,直接使用 true 明确回收站语义
      2. 添加注释说明多源数据合并逻辑(协作问卷 + 空间回收站问卷)
      3. 使用 Set 自动处理重复的 surveyId
  • 文件: server/src/modules/survey/controllers/surveyMeta.controller.ts

    • 改进:
      1. 关键步骤添加注释(回收站协作问卷查询)
      2. 明确区分回收站/非回收站的分支逻辑

4. 工作空间服务测试增强

  • 文件: server/src/modules/workspace/_test/workspace.service.spec.ts
    • 改进:
      1. 新增三个测试用例:
        • 正常返回问卷ID列表
        • 用户无工作空间时返回空列表
        • 回收站模式参数验证
      2. 使用真实 ObjectId 模拟数据
      3. 验证方法调用参数(如 isDeleted: true 回收站标志)
      4. 完善空结果处理逻辑

5. 前端用户体验优化

  • 文件: web/src/management/pages/list/components/RecycleBinList.vue
    • 问题: 删除提示存在错别字
    • 改进: "改问卷" → "该问卷"(第315行),提升文案准确性

6. 状态管理优化

  • 文件: web/src/management/pages/list/index.vue

    • 改进:
      1. isRecycleBin 重构为计算属性(computed
      2. 基于 MenuType 自动推导状态,避免手动维护布尔值
      3. 建议: 删除已注释的冗余代码(第273-285行)
  • 文件: web/src/management/stores/workSpace.ts

    • 改进:
      1. 动态查找回收站菜单项(避免硬编码索引)
      2. 安全更新计数:recycleBinMenu && (recycleBinMenu.count = count)
      3. 建议: 添加空值保护 recycleBinMenu?.count = count

7. 异常码扩展

  • 文件: server/src/enums/exceptionCode.ts
    • 改进: 新增 SERVER_ERROR = 5002 错误码,完善错误处理体系

总结建议

  1. 删除冗余代码:清理 index.vue 中已注释的状态设置代码
  2. 增强空值保护:在 workSpace.ts 中使用可选链操作符(?.
  3. 保持测试一致性:所有测试用例应包含 expect 验证和模拟方法调用检查
  4. 语义化命名:类似 isCompleteDeleted 的修正需推广到其他领域

本次变更整体质量较高,主要涉及逻辑优化、测试增强和用户体验改进,未发现安全漏洞或严重逻辑错误。

@skique skique merged commit 336a3ae into didi:feature/peking-2025 Aug 26, 2025
4 of 6 checks 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.

1 participant