-
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
Добавление структуры упорядочивания полей #184
Conversation
WalkthroughИзменения включают добавление нового необязательного параметра Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🔇 Additional comments (3)
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: 1
🧹 Nitpick comments (1)
src/cf/CommonModules/пбп_ИнтеграцииСервер/Ext/Module.bsl (1)
175-182
: Добавьте пример структуры упорядочиванияДокументация параметра понятна, но было бы полезно добавить пример структуры для лучшего понимания формата данных.
Предлагаю добавить пример в документацию:
// СтруктураУпорядочивания - Структура - ключом является поле, по которому необходимо упорядочить, -// значением является вид сортировки +// значением является вид сортировки +// Пример: +// СтруктураУпорядочивания = Новый Структура; +// СтруктураУпорядочивания.Вставить("Поле1", "УБЫВ"); +// СтруктураУпорядочивания.Вставить("Поле2", "ВОЗР");
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/cf/CommonModules/пбп_ИнтеграцииСервер/Ext/Module.bsl
(4 hunks)src/cf/CommonModules/пбп_ОбновлениеИнформационнойБазыПБП/Ext/Module.bsl
(1 hunks)src/cf/Configuration.xml
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/cf/Configuration.xml
- src/cf/CommonModules/пбп_ОбновлениеИнформационнойБазыПБП/Ext/Module.bsl
🔇 Additional comments (1)
src/cf/CommonModules/пбп_ИнтеграцииСервер/Ext/Module.bsl (1)
189-190
: Корректное изменение сигнатуры функцииДобавление необязательного параметра
СтруктураУпорядочивания
выполнено правильно, с корректным значением по умолчанию.
Если СтруктураУпорядочивания <> Неопределено Тогда | ||
ПорядокЗапроса = ""; | ||
Для Каждого КлючЗначениеПорядка Из СтруктураУпорядочивания Цикл | ||
НуженПеренос = ?(ПустаяСтрока(ПорядокЗапроса), "", ", | ||
| "); | ||
ПорядокЗапроса = ПорядокЗапроса + СтрШаблон("%1Таблица.%2 %3", | ||
НуженПеренос, КлючЗначениеПорядка.Ключ, КлючЗначениеПорядка.Значение); | ||
КонецЦикла; | ||
|
||
ТекстЗапроса = ТекстЗапроса + " | ||
|УПОРЯДОЧИТЬ ПО | ||
| " + ПорядокЗапроса; | ||
КонецЕсли; |
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
Необходимо добавить проверки и оптимизировать построение запроса
Текущая реализация требует следующих улучшений:
- Отсутствует валидация значений направления сортировки (ВОЗР/УБЫВ)
- Нет защиты от SQL-инъекций при использовании имен полей
- Можно оптимизировать построение строки запроса
Предлагаю следующие изменения:
Если СтруктураУпорядочивания <> Неопределено Тогда
+ // Проверка корректности значений
+ ДопустимыеНаправления = Новый Массив;
+ ДопустимыеНаправления.Добавить("ВОЗР");
+ ДопустимыеНаправления.Добавить("УБЫВ");
+
ПорядокЗапроса = "";
+ МассивПорядка = Новый Массив;
Для Каждого КлючЗначениеПорядка Из СтруктураУпорядочивания Цикл
- НуженПеренос = ?(ПустаяСтрока(ПорядокЗапроса), "", ",
- | ");
- ПорядокЗапроса = ПорядокЗапроса + СтрШаблон("%1Таблица.%2 %3",
- НуженПеренос, КлючЗначениеПорядка.Ключ, КлючЗначениеПорядка.Значение);
+ Если ДопустимыеНаправления.Найти(ВРег(КлючЗначениеПорядка.Значение)) = Неопределено Тогда
+ ВызватьИсключение СтрШаблон(
+ "Недопустимое направление сортировки: %1. Допустимые значения: ВОЗР, УБЫВ",
+ КлючЗначениеПорядка.Значение
+ );
+ КонецЕсли;
+
+ // Используем параметры запроса для защиты от SQL-инъекций
+ ИмяПараметра = "П_" + КлючЗначениеПорядка.Ключ;
+ Запрос.УстановитьПараметр(ИмяПараметра, КлючЗначениеПорядка.Ключ);
+ МассивПорядка.Добавить(СтрШаблон("Таблица.&%1 %2",
+ ИмяПараметра, КлючЗначениеПорядка.Значение));
КонецЦикла;
+ ПорядокЗапроса = СтрСоединить(МассивПорядка, ",
+ | ");
+
ТекстЗапроса = ТекстЗапроса + "
|УПОРЯДОЧИТЬ ПО
| " + ПорядокЗапроса;
КонецЕсли;
Committable suggestion skipped: line range outside the PR's diff.
Closes #182
Summary by CodeRabbit
Новые возможности
Обновления