base/src/main/java/com/tinyengine/it/service/material/impl/BlockGroupServiceImpl.java (2)

169-171: Security concern: Remove hardcoded user IDs.

Using hardcoded user IDs ("1") is a security risk. Consider injecting the actual user context.

-        String groupCreatedBy = "1"; // 获取登录用户id
-        String blockCreatedBy = "1";
+        String groupCreatedBy = userContextService.getCurrentUserId();
+        String blockCreatedBy = userContextService.getCurrentUserId();

---

190-203: Fix incorrect block version lookup.

The version lookup uses the wrong block group ID. It's using the local blockGroup variable instead of blockGroupTemp from the iteration.

         for (BlockGroup blockGroupTemp : blockGroupsListResult) {
             for (Block block : blockGroupTemp.getBlocks()) {
                 BlockCarriersRelation queryParam = new BlockCarriersRelation();
                 queryParam.setBlockId(block.getId());
-                queryParam.setHostId(blockGroup.getId());
+                queryParam.setHostId(blockGroupTemp.getId());
                 queryParam.setHostType(Enums.BlockGroup.BLOCK_GROUP.getValue());

base/src/main/resources/mappers/BlockGroupMapper.xml (1)

332-340: New queryBlockGroupByCondition Select Statement Addition
The new select statement effectively reuses the <include> fragments for the base column list and conditional filters (BlockGroupByCondition), ensuring that SQL logic remains consistent and centralized. The WHERE 1=1 clause is a common idiom to facilitate the dynamic appending of conditions; however, if it’s not necessary in your context, consider removing it or documenting its purpose for future maintainers.

base/src/main/java/com/tinyengine/it/service/material/impl/BlockGroupServiceImpl.java (2)

186-186: Improve null check implementation.

While checking for null IDs is good, consider a more defensive implementation that avoids potential edge cases.

-        if (blockGroupsListResult.isEmpty() || blockGroupsListResult.get(0).getId() == null) {
+        if (blockGroupsListResult.isEmpty() || blockGroupsListResult.stream().anyMatch(bg -> bg.getId() == null)) {

---

166-206: Consider refactoring to separate concerns.

The method handles multiple responsibilities: querying block groups, filtering by user, and updating version information. Consider splitting these into separate methods for better maintainability.

Example refactor:

@Override
public List<BlockGroup> getBlockGroupByIdsOrAppId(List<Integer> ids, Integer appId, String from) {
    String blockCreatedBy = getBlockCreatedBy(from);
    String groupCreatedBy = userContextService.getCurrentUserId();
    
    List<BlockGroup> blockGroups = queryBlockGroups(ids, appId, blockCreatedBy, groupCreatedBy);
    
    if (blockGroups.isEmpty() || blockGroups.stream().anyMatch(bg -> bg.getId() == null)) {
        return blockGroups;
    }
    
    return enrichBlockVersions(blockGroups);
}

private List<BlockGroup> queryBlockGroups(List<Integer> ids, Integer appId, String blockCreatedBy, String groupCreatedBy) {
    if (ids != null) {
        return queryByIds(ids, blockCreatedBy, groupCreatedBy);
    }
    if (appId != null) {
        return blockGroupMapper.queryBlockGroupByAppId(appId, blockCreatedBy, groupCreatedBy);
    }
    return blockGroupMapper.queryAllBlockGroupAndBlock(blockCreatedBy, groupCreatedBy);
}

private List<BlockGroup> enrichBlockVersions(List<BlockGroup> blockGroups) {
    for (BlockGroup blockGroup : blockGroups) {
        enrichBlockVersionsForGroup(blockGroup);
    }
    return blockGroups;
}

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro