-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Добавлена возможность выбора типа СУБД вручную на форме настройки #177
Conversation
WalkthroughИзменения в pull request направлены на расширение функциональности настроек интеграции с внешними источниками данных. Добавлен новый атрибут "ТипСУБД" в справочник настроек интеграции, который позволяет указывать тип базы данных при подключении. Реализованы новые методы проверки и обработки параметров подключения, включая автоматическое определение типа базы данных по строке подключения и расширенную валидацию настроек интеграции. Changes
Sequence DiagramsequenceDiagram
participant User
participant Form
participant Integration
participant Server
User->>Form: Выбирает тип СУБД
Form->>Integration: Устанавливает тип СУБД
Integration->>Server: Проверяет параметры подключения
Server-->>Integration: Возвращает результат проверки
Integration->>User: Отображает статус валидации
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/cf/Catalogs/пбп_НастройкиИнтеграции/Forms/ФормаЭлемента/Ext/Form/Module.bsl (2)
127-135
: Учтите пограничные случаи при определении типа СУБД
В процедуреСтрокаПодключенияМногострочнаяПриИзменении
мы заполняемТипСУБД
на основе частичного поиска названия в строке подключения. Такая проверка может не охватить все варианты наименований СУБД или их версий. Рекомендуется предусмотреть обработку возможных отклонений (например, наличие пробелов, регистров, префиксов).
616-635
: Дополните определение типа БД
ФункцияПолучитьИмяСУБДВнешнегоИсточникаДанныхПоСтрокеСоединения
ориентируется на конкретные фрагменты текста. Если строка подключения содержит иные варианты написания (например, пробелы, нестандартные обозначения), тип БД не будет определён. При необходимости обработайте больше вариантов или используйте более надёжное распознавание.src/cf/Catalogs/пбп_НастройкиИнтеграции/Forms/ФормаЭлемента/Ext/Form.xml (1)
143-215
: Обратите внимание на возможную опечатку в названии «IDM Db2»
В коде встречаетсяIBM DB2
, а здесь указаноIDM Db2
. Если это не намеренное различие, целесообразно исправить, чтобы быть последовательным в названии СУБД.- <v8:content>IDM Db2</v8:content> + <v8:content>IBM Db2</v8:content>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/cf/Catalogs/пбп_НастройкиИнтеграции.xml
(1 hunks)src/cf/Catalogs/пбп_НастройкиИнтеграции/Ext/ObjectModule.bsl
(2 hunks)src/cf/Catalogs/пбп_НастройкиИнтеграции/Forms/ФормаЭлемента/Ext/Form.xml
(1 hunks)src/cf/Catalogs/пбп_НастройкиИнтеграции/Forms/ФормаЭлемента/Ext/Form/Module.bsl
(6 hunks)src/cf/CommonModules/пбп_ИнтеграцииСервер/Ext/Module.bsl
(2 hunks)src/cf/CommonModules/пбп_ИнтеграцииСлужебный/Ext/Module.bsl
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/cf/CommonModules/пбп_ИнтеграцииСервер/Ext/Module.bsl
🔇 Additional comments (5)
src/cf/Catalogs/пбп_НастройкиИнтеграции/Ext/ObjectModule.bsl (1)
169-177
: Логика проверки типа интеграции выглядит корректно
Функция ясно указывает, что объект относится к прямому подключению к БД. В целом, этого достаточно для принятия решений в коде.
src/cf/CommonModules/пбп_ИнтеграцииСлужебный/Ext/Module.bsl (1)
172-172
: Убедитесь в необходимости добавления «ТипСУБД»
Добавление в список реквизитов выглядит оправданно и дополняет логику по хранению информации о типе БД. Проверьте, что все модули, использующие возвращаемую структуру, должным образом обрабатывают новое поле.
src/cf/Catalogs/пбп_НастройкиИнтеграции/Forms/ФормаЭлемента/Ext/Form/Module.bsl (1)
214-215
: Корректно настроена видимость поля «ТипСУБД»
Видимость контролов задаётся в зависимости от типа интеграции. Это облегчает использование формы, скрывая нерелевантные поля. В целом, логика выглядит последовательной.
Also applies to: 224-225, 267-329
src/cf/Catalogs/пбп_НастройкиИнтеграции/Forms/ФормаЭлемента/Ext/Form.xml (1)
139-142
: Обработчик изменения многострочной строки
Добавлен обработчик СтрокаПодключенияМногострочнаяПриИзменении
. Это хорошая практика, позволяющая своевременно реагировать на корректировки строки подключения. Всё выглядит корректно.
src/cf/Catalogs/пбп_НастройкиИнтеграции.xml (1)
856-899
: Необходимо уточнить взаимодействие с существующими атрибутами
Рекомендуется рассмотреть следующие аспекты:
- Автоматическое определение типа СУБД из строки подключения
- Валидация соответствия выбранного типа СУБД формату строки подключения
- Добавление значения по умолчанию для совместимости с существующими данными
Для проверки использования атрибута в существующем коде:
✅ Verification successful
Атрибут ТипСУБД корректно интегрирован в существующую функциональность
Анализ кода показывает, что:
- В форме элемента реализовано автоматическое определение типа СУБД из строки подключения через функцию
ПолучитьИмяСУБДВнешнегоИсточникаДанныхПоСтрокеСоединения
- В модуле
пбп_ИнтеграцииСервер
значение атрибута корректно используется при формировании параметров соединения - В объектном модуле каталога реализована проверка изменений значения ТипСУБД
- Атрибут интегрирован в пользовательский интерфейс формы элемента с соответствующими элементами управления
Дополнительные действия по интеграции не требуются.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Поиск использования атрибута ТипСУБД в модулях форм и объектов
rg -t bsl "ТипСУБД"
# Поиск связанных изменений в формах
fd --extension xml "ФормаЭлемента.xml" --exec grep -l "ТипСУБД" {} \;
Length of output: 316
Script:
#!/bin/bash
# Поиск использования ТипСУБД во всех файлах
rg "ТипСУБД"
# Поиск связанных атрибутов в файле настроек интеграции
rg "СтрокаПодключения|ТипИнтеграции" src/cf/Catalogs/пбп_НастройкиИнтеграции.xml
# Поиск форм элемента каталога
fd "ФормаЭлемента" --extension xml
Length of output: 2714
Если Не ЭтоНовый() И ПроверитьЭтоИнтеграцияСВнешнимИсточникомДанных() Тогда | ||
Запрос = Новый Запрос; | ||
Запрос.УстановитьПараметр("Ссылка", Ссылка); | ||
Запрос.Текст = "ВЫБРАТЬ | ||
| пбп_НастройкиИнтеграции.СтрокаПодключения КАК СтрокаПодключения, | ||
| пбп_НастройкиИнтеграции.Порт КАК Порт, | ||
| пбп_НастройкиИнтеграции.ИмяОбъекта КАК ИмяОбъекта, | ||
| пбп_НастройкиИнтеграции.ТипСУБД КАК ТипСУБД | ||
|ИЗ | ||
| Справочник.пбп_НастройкиИнтеграции КАК пбп_НастройкиИнтеграции | ||
|ГДЕ | ||
| пбп_НастройкиИнтеграции.Ссылка = &Ссылка"; | ||
|
||
ВыборкаРезультатаЗапроса = Запрос.Выполнить().Выбрать(); | ||
ВыборкаРезультатаЗапроса.Следующий(); | ||
|
||
Если ИмяОбъекта <> ВыборкаРезультатаЗапроса.ИмяОбъекта | ||
Или СтрокаПодключения <> ВыборкаРезультатаЗапроса.СтрокаПодключения | ||
Или Порт <> ВыборкаРезультатаЗапроса.Порт | ||
Или ТипСУБД <> ВыборкаРезультатаЗапроса.ТипСУБД Тогда | ||
ТекстСообщения = НСтр("ru = 'После записи настроек интеграции необходимо нажать на ""Установить соединение с базой данных для обновления параметров соединения с внешним источником данных""'"); | ||
пбп_Переадресация.СообщитьПользователю(ТекстСообщения); | ||
КонецЕсли; | ||
КонецЕсли; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Уточните обработку отсутствия записей в выборке
Если запись в справочнике не найдена или не прочиталась, выполнение ВыборкаРезультатаЗапроса.Следующий()
вернёт Ложь, и дальнейшая обработка полей вызовет ошибку. Рекомендуется дополнить проверку, чтобы избежать потенциальных исключений и корректно уведомлять пользователя, если исходные данные отсутствуют.
Если ВыборкаРезультатаЗапроса.Количество() = 0 Тогда
// Предупредить или обработать отсутствие записи
Возврат;
КонецЕсли;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Это не требуется, так как запрос выполняется с указанием ссылки на текущий элемент справочника. В условии, где выполняется запрос, есть проверка Не ЭтоНовый(), следовательно ссылка должна быть и запись всегда будет в выборке
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thvvmas, принято. Если уверены, что в вашем сценарии выборка всегда будет возвращать запись из справочника (так как объект не новый и ссылка гарантированно присутствует), то дополнительная проверка действительно не требуется. Если впоследствии возникнет необходимость игнорировать подобные замечания от статического анализатора, можно либо отключить соответствующее правило в конфигурации, либо продолжать оставлять как есть, учитывая логику обработки в вашем коде.
&НаСервереБезКонтекста | ||
Функция ПроверитьЭтоИнтеграцияСВнешнимИсточникомДанных(ТипИнтеграции) | ||
|
||
ЭлементНаследования = Справочники.пбп_ТипыИнтеграций | ||
.ПолучитьПредопределенныйЭлементНаследованияНастроекТипаИнтеграции(ТипИнтеграции); | ||
|
||
Возврат ЭлементНаследования = Справочники.пбп_ТипыИнтеграций.ПрямоеПодключениеКБД; | ||
|
||
КонецФункции | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Избегайте дублирования кода
Функция ПроверитьЭтоИнтеграцияСВнешнимИсточникомДанных
повторяется в модуле объекта и здесь. Если возможно, вынесите общую логику в единый метод или модуль, чтобы упростить сопровождение и не нарушать принцип DRY.
// Пример вынесения в единый модуль (упростив код):
// В одном общем модуле:
Функция ПроверитьЭтоИнтеграцияСВнешнимИсточникомДанных(ТипИнтеграции) Экспорт
Возврат Справочники.пбп_ТипыИнтеграций
.ПолучитьПредопределенныйЭлементНаследованияНастроекТипаИнтеграции(ТипИнтеграции)
= Справочники.пбп_ТипыИнтеграций.ПрямоеПодключениеКБД;
КонецФункции
// Затем вызывать везде, где нужно.
<Attribute uuid="6c9902f2-7e3f-4d2e-b51e-efd40a6134c0"> | ||
<Properties> | ||
<Name>ТипСУБД</Name> | ||
<Synonym> | ||
<v8:item> | ||
<v8:lang>ru</v8:lang> | ||
<v8:content>Тип СУБД</v8:content> | ||
</v8:item> | ||
</Synonym> | ||
<Comment/> | ||
<Type> | ||
<v8:Type>xs:string</v8:Type> | ||
<v8:StringQualifiers> | ||
<v8:Length>30</v8:Length> | ||
<v8:AllowedLength>Variable</v8:AllowedLength> | ||
</v8:StringQualifiers> | ||
</Type> | ||
<PasswordMode>false</PasswordMode> | ||
<Format/> | ||
<EditFormat/> | ||
<ToolTip/> | ||
<MarkNegatives>false</MarkNegatives> | ||
<Mask/> | ||
<MultiLine>false</MultiLine> | ||
<ExtendedEdit>false</ExtendedEdit> | ||
<MinValue xsi:nil="true"/> | ||
<MaxValue xsi:nil="true"/> | ||
<FillFromFillingValue>false</FillFromFillingValue> | ||
<FillValue xsi:type="xs:string"/> | ||
<FillChecking>DontCheck</FillChecking> | ||
<ChoiceFoldersAndItems>Items</ChoiceFoldersAndItems> | ||
<ChoiceParameterLinks/> | ||
<ChoiceParameters/> | ||
<QuickChoice>Auto</QuickChoice> | ||
<CreateOnInput>Auto</CreateOnInput> | ||
<ChoiceForm/> | ||
<LinkByType/> | ||
<ChoiceHistoryOnInput>Auto</ChoiceHistoryOnInput> | ||
<Use>ForItem</Use> | ||
<Indexing>DontIndex</Indexing> | ||
<FullTextSearch>Use</FullTextSearch> | ||
<DataHistory>Use</DataHistory> | ||
</Properties> | ||
</Attribute> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Рекомендуется добавить валидацию значений атрибута ТипСУБД
Для обеспечения целостности данных и предотвращения ошибок ввода предлагается:
- Заменить тип данных
xs:string
на перечисление с фиксированным списком поддерживаемых СУБД - Добавить проверку заполнения (
FillChecking>ShowError</FillChecking
)
Пример реализации:
- <Type>
- <v8:Type>xs:string</v8:Type>
- <v8:StringQualifiers>
- <v8:Length>30</v8:Length>
- <v8:AllowedLength>Variable</v8:AllowedLength>
- </v8:StringQualifiers>
- </Type>
+ <Type>
+ <v8:Type>cfg:EnumRef.пбп_ТипыСУБД</v8:Type>
+ </Type>
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this 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/cf/Catalogs/пбп_НастройкиИнтеграции/Forms/ФормаЭлемента/Ext/Form/Module.bsl (1)
612-631
: Улучшить определение типа СУБДТекущая реализация функции
ПолучитьИмяСУБДВнешнегоИсточникаДанныхПоСтрокеСоединения
может быть улучшена:
- Добавить поддержку разных форматов строк подключения
- Учесть регистр при поиске
- Добавить документацию по поддерживаемым форматам
Предлагаемая реализация:
&НаКлиенте Функция ПолучитьИмяСУБДВнешнегоИсточникаДанныхПоСтрокеСоединения(СтрокаСоединения) СтрокаСоединенияВРег = ВРег(СтрокаСоединения); ПоддерживаемыеСУБД = Новый Соответствие; ПоддерживаемыеСУБД.Вставить("SQL SERVER", "MSSQLServer"); ПоддерживаемыеСУБД.Вставить("POSTGRESQL", "PostgreSQL"); ПоддерживаемыеСУБД.Вставить("IBM DB2", "IBMDB2"); ПоддерживаемыеСУБД.Вставить("ORACLE", "OracleDatabase"); ПоддерживаемыеСУБД.Вставить("MYSQL", "MySQL"); Для Каждого КлючИЗначение Из ПоддерживаемыеСУБД Цикл Если СтрНайти(СтрокаСоединенияВРег, КлючИЗначение.Ключ) > 0 Тогда Возврат КлючИЗначение.Значение; КонецЕсли; КонецЦикла; Возврат ""; КонецФункции
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/cf/Catalogs/пбп_НастройкиИнтеграции/Ext/ObjectModule.bsl
(1 hunks)src/cf/Catalogs/пбп_НастройкиИнтеграции/Forms/ФормаЭлемента/Ext/Form/Module.bsl
(6 hunks)src/cf/CommonModules/пбп_ИнтеграцииСлужебный/Ext/Module.bsl
(2 hunks)
🔇 Additional comments (6)
src/cf/Catalogs/пбп_НастройкиИнтеграции/Ext/ObjectModule.bsl (1)
67-90
: 🛠️ Refactor suggestion
Проверьте обработку результатов запроса
В процедуре ПередЗаписью
добавлена проверка параметров подключения к внешнему источнику данных, включая новый параметр ТипСУБД
. Однако есть несколько моментов, требующих внимания:
- Отсутствует проверка успешности выполнения запроса
- Нет обработки случая, когда запрос не вернул записей
Рекомендуется добавить проверки:
ВыборкаРезультатаЗапроса = Запрос.Выполнить().Выбрать();
Если ВыборкаРезультатаЗапроса.Количество() = 0 Тогда
Возврат;
КонецЕсли;
ВыборкаРезультатаЗапроса.Следующий();
src/cf/CommonModules/пбп_ИнтеграцииСлужебный/Ext/Module.bsl (2)
172-172
: LGTM: Добавлен атрибут ТипСУБД
Корректное добавление атрибута ТипСУБД
в список получаемых реквизитов объекта.
263-278
: Функция соответствует принципу единой ответственности
Функция ПроверитьЭтоИнтеграцияСВнешнимИсточникомДанных
реализована корректно и используется в других модулях системы. Хорошо, что функция вынесена в общий модуль для переиспользования.
src/cf/Catalogs/пбп_НастройкиИнтеграции/Forms/ФормаЭлемента/Ext/Form/Module.bsl (3)
127-134
: Добавлен обработчик изменения строки подключения
Реализована автоматическая установка типа СУБД при изменении строки подключения. Логика корректна и соответствует требованиям.
214-214
: Добавлено управление видимостью поля ТипСУБД
Корректная интеграция нового поля в существующую логику управления видимостью элементов формы.
Also applies to: 224-225
605-610
: Переиспользование общей функциональности
Правильное решение использовать функцию из общего модуля пбп_ИнтеграцииСлужебный
.
#159
Summary by CodeRabbit
Новые возможности
Улучшения
Исправления