Skip to content

Conversation

@johnnyshut
Copy link
Contributor

@johnnyshut johnnyshut commented Nov 29, 2025

Описание

Реализовано добавление свойств CheckUnique и CodeSeries в класс ChartOfAccounts и ChartOfCharacteristicTypes

Внесенные изменения:

  1. Добавлено свойство checkUnique в класс ChartOfAccounts и ChartOfCharacteristicTypes:

    • Тип: boolean
    • Значение по умолчанию: true
    • Определяет, нужно ли проверять уникальность кода справочника
  2. Обновлены тестовые JSON-файлы:

    • Добавлены поля checkUnique и codeSeries во все тестовые JSON-файлы с корректными значениями

Особенности реализации:

  • Для формата Designer (XML): поля CheckUnique и CodeSeries обязательны и корректно парсятся из XML
  • Для формата EDT (MDO): поле CheckUnique обязательно, поле CodeSeries опционально — если отсутствует, используется значение по умолчанию WHOLE_CATALOG

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

Closes #546

Чеклист

Общие

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

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

Все тесты проходят успешно. Реализация соответствует требованиям issue #546 и обеспечивает корректную работу с обоими форматами (Designer и EDT).

Summary by CodeRabbit

  • New Features

    • Added optional uniqueness validation for chart of accounts and characteristic type plans with configurable validation scope/series.
    • Default scope set to whole-catalog behavior for EDT-format items.
  • Tests

    • Added parameterized tests validating the new uniqueness flag and default scope behavior for both chart types.

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

…counts и ChartOfCharacteristicTypes

Добавлены поля для управления проверкой уникальности кода в классах ChartOfAccounts и ChartOfCharacteristicTypes:
- checkUnique: определяет, нужно ли проверять уникальность кода
- codeSeries: определяет область действия уникальности кода

Оба поля имеют значение по умолчанию: checkUnique = false, codeSeries = WHOLE_CATALOG.
…tion и ChartsOfCharacteristicTypes

Добавлены поля для управления проверкой уникальности кода в классах Configuration и ChartsOfCharacteristicTypes:
- checkUnique: определяет, нужно ли проверять уникальность кода
- codeSeries: определяет область действия уникальности кода

Оба поля имеют значение по умолчанию: checkUnique = false, codeSeries = WHOLE_CATALOG.
…ounts и ChartOfCharacteristicTypes

Добавлены параметризованные тесты testCheckUniqueTrue для проверки, что поле checkUnique установлено в true для планов счетов "ПланСчетов1" и видов характеристик "ПланВидовХарактеристик1". Также проверяется, что поле codeSeries соответствует значению WHOLE_CATALOG.
@johnnyshut
Copy link
Contributor Author

#548
Осенило, что нужны не только справочники, этот pr является расширением функционала который был слит ранее

@johnnyshut
Copy link
Contributor Author

@theshadowco готово

@coderabbitai
Copy link

coderabbitai bot commented Nov 29, 2025

Walkthrough

The pull request adds two new public fields, checkUnique (boolean) and codeSeries (CodeSeries), to ChartOfAccounts and ChartOfCharacteristicTypes, and updates unit tests to assert these fields are populated (defaulting codeSeries to WHOLE_CATALOG when applicable).

Changes

Cohort / File(s) Change Summary
Model Classes: Chart of Accounts & Characteristic Types
src/main/java/com/github/_1c_syntax/bsl/mdo/ChartOfAccounts.java, src/main/java/com/github/_1c_syntax/bsl/mdo/ChartOfCharacteristicTypes.java
Added two public fields with Javadoc: checkUnique (boolean, default false) controlling uniqueness validation, and codeSeries (CodeSeries, default CodeSeries.WHOLE_CATALOG). Imported CodeSeries.
Tests: Chart of Accounts & Characteristic Types
src/test/java/com/github/_1c_syntax/bsl/mdo/ChartOfAccountsTest.java, src/test/java/com/github/_1c_syntax/bsl/mdo/ChartOfCharacteristicTypesTest.java
Added parameterized test testCheckUniqueTrue verifying checkUnique is true and codeSeries equals CodeSeries.WHOLE_CATALOG; added CodeSeries imports and assertions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Mostly symmetric, low-complexity additions across two model classes and tests.
  • Review focus: Javadoc clarity, default values, and test correctness.

Possibly related PRs

Poem

🐰 I nibble code beneath the moonlight,
Two tiny fields hop into sight,
They check each code from leaf to tree,
WholeCatalog scope — safe as can be,
Hooray! 🎉

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: adding support for CodeSeries and CheckUnique properties to ChartOfCharacteristicTypes and ChartOfAccounts classes.
Linked Issues check ✅ Passed The PR adds checkUnique and codeSeries support to ChartOfAccounts and ChartOfCharacteristicTypes, partially addressing issue #546 which requested these features for Catalog class. The core requirements are met with proper parsing and field addition.
Out of Scope Changes check ✅ Passed The PR includes test additions (new test methods in ChartOfAccountsTest and ChartOfCharacteristicTypesTest) that verify the new functionality, which are in-scope for validating the feature implementation.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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: 2

📜 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 cbc5c89 and 6480fd3.

⛔ Files ignored due to path filters (12)
  • src/test/resources/fixtures/mdclasses/ChartsOfAccounts.ПланСчетов1.json is excluded by !src/test/resources/**
  • src/test/resources/fixtures/mdclasses/ChartsOfAccounts.ПланСчетов1_edt.json is excluded by !src/test/resources/**
  • src/test/resources/fixtures/mdclasses/ChartsOfCharacteristicTypes.ПланВидовХарактеристик1.json is excluded by !src/test/resources/**
  • src/test/resources/fixtures/mdclasses/ChartsOfCharacteristicTypes.ПланВидовХарактеристик1_edt.json is excluded by !src/test/resources/**
  • src/test/resources/fixtures/mdclasses/Configuration.json is excluded by !src/test/resources/**
  • src/test/resources/fixtures/mdclasses/Configuration_edt.json is excluded by !src/test/resources/**
  • src/test/resources/fixtures/mdclasses_3_18/Configuration.json is excluded by !src/test/resources/**
  • src/test/resources/fixtures/mdclasses_3_18/Configuration_edt.json is excluded by !src/test/resources/**
  • src/test/resources/fixtures/mdclasses_3_24/Configuration_edt.json is excluded by !src/test/resources/**
  • src/test/resources/fixtures/mdclasses_5_1/Configuration.json is excluded by !src/test/resources/**
  • src/test/resources/fixtures/ssl_3_1/ChartsOfCharacteristicTypes.ДополнительныеРеквизитыИСведения.json is excluded by !src/test/resources/**
  • src/test/resources/fixtures/ssl_3_1/ChartsOfCharacteristicTypes.ДополнительныеРеквизитыИСведения_edt.json is excluded by !src/test/resources/**
📒 Files selected for processing (4)
  • src/main/java/com/github/_1c_syntax/bsl/mdo/ChartOfAccounts.java (2 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/mdo/ChartOfCharacteristicTypes.java (2 hunks)
  • src/test/java/com/github/_1c_syntax/bsl/mdo/ChartOfAccountsTest.java (2 hunks)
  • src/test/java/com/github/_1c_syntax/bsl/mdo/ChartOfCharacteristicTypesTest.java (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-05T10:03:02.229Z
Learnt from: johnnyshut
Repo: 1c-syntax/mdclasses PR: 515
File: src/test/java/com/github/_1c_syntax/bsl/mdo/AccountingRegisterTest.java:35-37
Timestamp: 2025-09-05T10:03:02.229Z
Learning: The MDTestUtils.getMDWithSimpleTest method already correctly handles optional 4th parameter "_edt" through ArgumentsAccessor - it doesn't need modification to process the additional CSV parameter for subdirectory selection.

Applied to files:

  • src/test/java/com/github/_1c_syntax/bsl/mdo/ChartOfCharacteristicTypesTest.java
📚 Learning: 2025-09-05T10:03:02.229Z
Learnt from: johnnyshut
Repo: 1c-syntax/mdclasses PR: 515
File: src/test/java/com/github/_1c_syntax/bsl/mdo/AccountingRegisterTest.java:35-37
Timestamp: 2025-09-05T10:03:02.229Z
Learning: The MDTestUtils.getMDWithSimpleTest method already correctly handles optional 4th parameter through ArgumentsAccessor.size() check and argumentsAccessor.getString(3) call with null handling - no modification needed for CSV parameter processing.

Applied to files:

  • src/test/java/com/github/_1c_syntax/bsl/mdo/ChartOfCharacteristicTypesTest.java
  • src/test/java/com/github/_1c_syntax/bsl/mdo/ChartOfAccountsTest.java
🧬 Code graph analysis (1)
src/test/java/com/github/_1c_syntax/bsl/mdo/ChartOfCharacteristicTypesTest.java (1)
src/test/java/com/github/_1c_syntax/bsl/test_utils/assertions/Assertions.java (1)
  • Assertions (24-29)
🔇 Additional comments (8)
src/test/java/com/github/_1c_syntax/bsl/mdo/ChartOfAccountsTest.java (2)

25-25: LGTM!

The CodeSeries import is properly added to support the new assertions in the test method.


54-79: I'm unable to access the repository directly due to a cloning error, and the public information search didn't provide the specific fixture data needed. Given this limitation, I need to rewrite the review comment to reflect that manual verification of the fixtures is still required.


Fixture verification required before approval.

The test implementation correctly validates both checkUnique and codeSeries properties. However, the request to verify that the XML fixture files contain <checkUnique>true</checkUnique> could not be completed automatically. Since the default value of checkUnique is false in the main class but this test expects true, manually verify that both the Designer and EDT format XML fixture files explicitly set the expected values before merging.

src/test/java/com/github/_1c_syntax/bsl/mdo/ChartOfCharacteristicTypesTest.java (2)

24-30: LGTM!

The necessary imports are properly added to support the new test method assertions.


46-71: I'm unable to access the repository due to infrastructure limitations in the sandbox environment. However, I can provide a rewritten review comment based on the information available from the code snippet provided.

Fixture data verification is necessary but cannot be completed in current environment; test implementation is sound.

The test correctly validates both checkUnique and codeSeries properties for ChartOfCharacteristicTypes using proper assertions with descriptive messages. Since the test expects checkUnique=true but defaults are typically false, the XML fixtures must explicitly set these values. The developer should verify that fixture files for ПланВидовХарактеристик1 (both with _edt suffix and without) contain:

  • <checkUnique>true</checkUnique>
  • Appropriate codeSeries value matching WHOLE_CATALOG

The test follows the established pattern from similar tests in the codebase and uses MDTestUtils.getMDWithSimpleTest() correctly.

src/main/java/com/github/_1c_syntax/bsl/mdo/ChartOfAccounts.java (2)

29-29: LGTM!

The CodeSeries import is properly added to support the new codeSeries field.


135-142: LGTM!

The codeSeries field is properly implemented with clear Javadoc explaining its purpose, default value, and EDT format behavior.

src/main/java/com/github/_1c_syntax/bsl/mdo/ChartOfCharacteristicTypes.java (2)

27-27: LGTM!

The CodeSeries import is properly added to support the new codeSeries field.


129-136: LGTM!

The codeSeries field is properly implemented with clear Javadoc, appropriate default value, and helpful EDT format notes.

…unts и ChartOfCharacteristicTypes

Изменено описание в документации для полей checkUnique в классах ChartOfAccounts и ChartOfCharacteristicTypes, чтобы уточнить, что при значении false проверка уникальности не выполняется.
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: 1

📜 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 6480fd3 and 7943559.

📒 Files selected for processing (2)
  • src/main/java/com/github/_1c_syntax/bsl/mdo/ChartOfAccounts.java (2 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/mdo/ChartOfCharacteristicTypes.java (2 hunks)
⏰ 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). (9)
  • GitHub Check: build (23, windows-latest)
  • GitHub Check: build (23, ubuntu-latest)
  • GitHub Check: build (23, macOS-latest)
  • GitHub Check: build (21, macOS-latest)
  • GitHub Check: build (21, windows-latest)
  • GitHub Check: build (21, ubuntu-latest)
  • GitHub Check: build (17, macOS-latest)
  • GitHub Check: build (17, windows-latest)
  • GitHub Check: build (17, ubuntu-latest)
🔇 Additional comments (3)
src/main/java/com/github/_1c_syntax/bsl/mdo/ChartOfAccounts.java (2)

29-29: CodeSeries import is appropriate and minimal.

The added CodeSeries import cleanly supports the new codeSeries field; no issues here.


126-142: Based on my investigation, I cannot access the repository code or retrieve definitive platform documentation about the ChartOfAccounts checkUnique default value. The web search did not provide clarity on whether the platform defaults to true or false for this property.

The original review comment appropriately identifies this uncertainty and is correctly structured to request verification. It:

  • Acknowledges the current implementation (false)
  • Notes the potential discrepancy with PR description claims
  • Requests developer confirmation of intended behavior
  • Provides clear alternative paths forward

Since I cannot verify the PR description content, platform semantics, or related test expectations without repository access, I will preserve the original review comment's structure and substance, as it appropriately raises this as a verification point that requires developer input.


Confirm intended default for checkUnique in ChartOfAccounts.

The Javadoc now consistently documents that when checkUnique == true, the code must be unique within the scope defined by codeSeries; when false, uniqueness is not checked. The codeSeries default of CodeSeries.WHOLE_CATALOG matches EDT behavior.

However, this implementation sets:

@Default
boolean checkUnique = false;

while the PR description suggests checkUnique should default to true for ChartOfAccounts. Please confirm the intended behavior:

  • If the platform requires uniqueness by default, update the default:
-  @Default
-  boolean checkUnique = false;
+  @Default
+  boolean checkUnique = true;
  • If false is intentional, update the PR description to clarify.
src/main/java/com/github/_1c_syntax/bsl/mdo/ChartOfCharacteristicTypes.java (1)

27-27: CodeSeries import usage is correct.

The CodeSeries import is needed for the new codeSeries field and keeps dependencies aligned with ChartOfAccounts; no further changes needed.

@theshadowco theshadowco merged commit d904dfe into 1c-syntax:develop Dec 1, 2025
12 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.

[NEW] Добавление метода для получения свойства CheckUnique в Catalog

2 participants