Skip to content

Conversation

@theshadowco
Copy link
Member

@theshadowco theshadowco commented Jan 13, 2026

Описание

Связанные задачи

Closes

Чеклист

Общие

  • Ветка PR обновлена из develop
  • Отладочные, закомментированные и прочие, не имеющие смысла участки кода удалены
  • Изменения покрыты тестами
  • Обязательные действия перед коммитом выполнены (запускал команду gradlew precommit)

Дополнительно

Summary by CodeRabbit

  • Bug Fixes
    • Fixed data conversion process to properly handle restriction template elements, improving system stability when processing role configurations.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 13, 2026

📝 Walkthrough

Walkthrough

The change adds handling in RoleDataConverter to skip "restrictionTemplate" nodes during deserialization. A new constant is introduced to identify these nodes, and the unmarshal method branches to skip them while continuing with existing behavior for other node types.

Changes

Cohort / File(s) Summary
RoleDataConverter unmarshal enhancement
src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/RoleDataConverter.java
Added constant RESTRICTION_TEMPLATE_NODE_NAME to identify restrictionTemplate nodes. Implemented node-skipping logic in unmarshal to bypass restrictionTemplate elements by continuing to parent level. Minor formatting adjustment to null-check conditional.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Poem

🐰 A template once haunted the node-reading way,
Till we taught the converter to skip and play,
With a constant so clever, a branch clean and clear,
The restrictionTemplate now vanishes—hurrah, my dear! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Фикс ошибки чтения роли' (Fix role reading error) is directly related to the main change: excluding restrictionTemplate node handling to fix role data conversion issues.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/RoleDataConverter.java (1)

84-86: Pre-existing issue: continue skips moveUp(), potentially corrupting reader state.

When fieldClass == null, the continue statement bypasses the reader.moveUp() call at line 91, leaving the reader positioned inside the child node. The same pattern issue exists in readRight (lines 103-105).

Consider aligning with the new restrictionTemplate handling pattern:

♻️ Suggested fix
        var fieldClass = (Class<?>) TransformationUtils.fieldType(builder, name);
        if (fieldClass == null) {
+          reader.moveUp();
          continue;
        }

And similarly in readRight:

      if (fieldClass == null) {
+        reader.moveUp();
        continue;
      }
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 60f1bea and 0a57a1f.

⛔ Files ignored due to path filters (2)
  • src/test/resources/ext/designer/mdclasses/src/cf/Roles/Роль1/Ext/Rights.xml is excluded by !src/test/resources/**
  • src/test/resources/ext/edt/mdclasses/configuration/src/Roles/Роль1/Rights.rights is excluded by !src/test/resources/**
📒 Files selected for processing (1)
  • src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/RoleDataConverter.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
  • GitHub Check: build (21, windows-latest)
  • GitHub Check: build (25, windows-latest)
  • GitHub Check: build (17, ubuntu-latest)
  • GitHub Check: build (25, ubuntu-latest)
  • GitHub Check: build (17, windows-latest)
  • GitHub Check: build (21, ubuntu-latest)
  • GitHub Check: build (17, macOS-latest)
  • GitHub Check: build (21, macOS-latest)
  • GitHub Check: QA
  • GitHub Check: build (17, macOS-latest)
  • GitHub Check: build (25, ubuntu-latest)
  • GitHub Check: build (21, macOS-latest)
  • GitHub Check: build (17, ubuntu-latest)
  • GitHub Check: build (17, windows-latest)
  • GitHub Check: build (25, macOS-latest)
  • GitHub Check: build (25, windows-latest)
  • GitHub Check: build (21, windows-latest)
  • GitHub Check: build (21, ubuntu-latest)
🔇 Additional comments (1)
src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/RoleDataConverter.java (1)

41-41: LGTM! The restrictionTemplate handling is correctly implemented.

The pattern of calling moveUp() before continue is correct — it properly exits the child node entered by moveDown() at line 52, then skips the duplicate moveUp() at line 68.

Also applies to: 57-60

@sonarqubecloud
Copy link

@github-actions
Copy link
Contributor

github-actions bot commented Jan 13, 2026

Test Results

  603 files  ±0    603 suites  ±0   9m 16s ⏱️ +7s
  284 tests ±0    284 ✅ ±0  0 💤 ±0  0 ❌ ±0 
2 646 runs  ±0  2 646 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 0a57a1f. ± Comparison against base commit 60f1bea.

♻️ This comment has been updated with latest results.

@theshadowco theshadowco merged commit ecb937d into develop Jan 13, 2026
25 checks passed
@theshadowco theshadowco deleted the feature/rightsBug branch January 13, 2026 12:14
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.

2 participants