Skip to content

Fix/market invite#799

Open
fangdudu wants to merge 2 commits into
iflytek:mainfrom
fangdudu:fix/market-invite
Open

Fix/market invite#799
fangdudu wants to merge 2 commits into
iflytek:mainfrom
fangdudu:fix/market-invite

Conversation

@fangdudu
Copy link
Copy Markdown
Contributor

📝 Pull Request 描述 | Description

🎯 变更类型 | Change Type

  • ✨ 新功能 | New Feature
  • 🐛 Bug 修复 | Bug Fix
  • 📚 文档更新 | Documentation
  • 🎨 代码格式/样式 | Code Style
  • ♻️ 重构 | Refactoring
  • ⚡ 性能优化 | Performance
  • ✅ 测试相关 | Tests
  • 🔧 配置变更 | Configuration
  • 🔨 构建/CI | Build/CI
  • 🌐 国际化 | Internationalization
  • ⬆️ 依赖升级 | Dependencies Update

🔗 相关 Issue | Related Issues

  • Closes #
  • Related to #

📋 变更内容 | Changes Made

主要变更 | Main Changes

技术细节 | Technical Details


🧪 测试 | Testing

测试环境 | Test Environment

  • Windows 10/11
  • Linux
  • macOS
  • Docker

测试步骤 | Test Steps

测试结果 | Test Results


📸 截图/录屏 | Screenshots/Recordings

变更前 | Before

变更后 | After


⚠️ 破坏性变更 | Breaking Changes

  • 此 PR 包含破坏性变更 | This PR contains breaking changes
破坏性变更详情 | Breaking Changes Details

✅ 检查清单 | Checklist

代码质量 | Code Quality

  • 代码遵循项目的编码规范 | Code follows project coding standards
  • 已进行自我代码审查 | Self-reviewed the code
  • 代码有适当的注释(特别是复杂逻辑)| Code has appropriate comments (especially for complex logic)
  • 更新了相关文档 | Updated relevant documentation
  • 没有产生新的警告 | No new warnings generated

测试 | Testing

  • 添加了相应的测试用例 | Added corresponding test cases
  • 所有测试通过 | All tests pass
  • 手动测试验证通过 | Manual testing verification passed

文档 | Documentation

  • 更新了 README(如需要)| Updated README (if needed)
  • 更新了 API 文档(如需要)| Updated API documentation (if needed)
  • 更新了用户指南(如需要)| Updated user guide (if needed)
  • 更新了 CHANGELOG(如需要)| Updated CHANGELOG (if needed)

其他 | Others

  • 已与相关利益方沟通 | Communicated with relevant stakeholders
  • 不影响现有功能 | Does not affect existing functionality
  • 考虑了向后兼容性 | Considered backward compatibility
  • 考虑了性能影响 | Considered performance impact
  • 考虑了安全性 | Considered security

📌 额外说明 | Additional Notes


🙏 致谢 | Acknowledgements


📖 提示 | Tips:

  • 确保 PR 标题简洁明了,使用动词开头(例如:Add, Fix, Update, Remove)
  • Ensure PR title is concise and clear, starting with a verb (e.g., Add, Fix, Update, Remove)
  • 尽量保持 PR 的改动范围小而集中,便于审查
  • Try to keep PR changes small and focused for easier review
  • 遵循项目的分支管理策略
  • Follow the project's branch management strategy

/cc @maintainers

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the NotifySendServiceImpl to handle tenant ID assignments during notification creation and team joining. Specifically, it introduces conditional logic to set the tenant ID for 'teamMarketInvite' messages and ensures the tenant ID is populated when a user joins a team. Review feedback suggests simplifying the conditional assignment logic in createNotify to improve readability and removing a redundant duplicate call to setTenantId in the acceptJoinTeam method.

Comment on lines 100 to +105
notifySend.setTenantId(createNotifyDto.getTenantId());
if (StringUtils.equals(createNotifyDto.getMessageType(), "teamMarketInvite")) {
if (StringUtils.isNotEmpty(marketUser.getTenantId())) {
notifySend.setTenantId(marketUser.getTenantId());
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The logic for setting the tenant ID can be simplified to improve readability and avoid redundant assignments to the notifySend object. This approach clearly determines the correct tenant ID before applying it.

            String tenantId = createNotifyDto.getTenantId();
            if (StringUtils.equals(createNotifyDto.getMessageType(), "teamMarketInvite") && StringUtils.isNotEmpty(marketUser.getTenantId())) {
                tenantId = marketUser.getTenantId();
            }
            notifySend.setTenantId(tenantId);

Comment on lines 332 to +333
appMarketUser.setUpdaterId(userId);

appMarketUser.setTenantId(notifySend.getTenantId());
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The assignment appMarketUser.setTenantId(notifySend.getTenantId()); is redundant here as it was already performed on line 329. Removing the duplicate call improves code clarity.

Suggested change
appMarketUser.setUpdaterId(userId);
appMarketUser.setTenantId(notifySend.getTenantId());
appMarketUser.setUpdaterId(userId);

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