fixed bugs #136
fixed bugs #136
Conversation
WalkthroughВ данном пулл-реквесте внесены изменения в два файла Changes
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
src/cf/CommonForms/пбп_ФормаРазрешенияКонфликтовПредопределенныхЭлементов/Ext/Form/Module.bsl (1)
206-218: Предлагаю улучшить информативность сообщения об ошибке.Текущая реализация корректна, но можно улучшить:
- Добавить контекст о том, где именно отсутствует элемент (например, в каком справочнике или документе)
- Добавить логирование для системных администраторов
Предлагаемые изменения:
&НаСервере Процедура СообщитьОбОтсутствииЭлемента(Идентификатор) - - ТекстСообщения = НСтр("ru = 'Элемент '%1' отсутствуют в базе!'; - |en = 'The element '%1' is missing from the database!'"); + ИмяОбъектаМетаданных = Метаданные.НайтиПоТипу(ТипЗнч(МенеджерОбъекта)).Синоним; + ТекстСообщения = НСтр("ru = 'Элемент '%1' отсутствует в справочнике ""%2""!'; + |en = 'The element '%1' is missing from the ""%2"" catalog!'"); ТекстСообщения = пбп_СтроковыеФункцииСлужебныйКлиентСервер.ПодставитьПараметрыВСтроку( ТекстСообщения, - Идентификатор); + Идентификатор, + ИмяОбъектаМетаданных); + + ЗаписьЖурналаРегистрации(НСтр("ru='Предопределенные элементы.Отсутствует элемент'"), + УровеньЖурналаРегистрации.Предупреждение,,, + ПодробноеПредставлениеОшибки(ТекстСообщения)); + пбп_ОбщегоНазначенияСлужебный.СообщитьПользователю(ТекстСообщения); - КонецПроцедуры
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
src/cf/CommonForms/пбп_ФормаРазрешенияКонфликтовПредопределенныхЭлементов/Ext/Form/Module.bsl(2 hunks)src/cf/CommonModules/пбп_ПредопределенныеЗначения/Ext/Module.bsl(4 hunks)
🔇 Additional comments (4)
src/cf/CommonForms/пбп_ФормаРазрешенияКонфликтовПредопределенныхЭлементов/Ext/Form/Module.bsl (1)
153-161: Улучшена обработка отсутствующих элементов!
Изменения повышают надежность кода за счет:
- Корректного сброса выборки перед поиском
- Правильной обработки отсутствующих элементов
- Сохранения целостности данных
src/cf/CommonModules/пбп_ПредопределенныеЗначения/Ext/Module.bsl (3)
263-266: Корректная сортировка и обработка дубликатов идентификаторов
Добавление сортировки таблицы и вызов процедуры обработки дубликатов идентификаторов улучшает порядок обработки и предотвращает возможные проблемы, связанные с дубликатами.
456-456: Обновление родителя элемента
Правильное присвоение нового родителя элементу обеспечивает корректное иерархическое расположение элементов.
634-671: Корректная обработка дублирующихся идентификаторов
Новая процедура эффективно обнаруживает и устраняет дубликаты идентификаторов, а также информирует пользователя об ошибках, что повышает надежность системы.
| ТекстСообщения = НСтр("ru = 'Идентификатор ''%1'' не заполнен!'; | ||
| |en = 'Item ''%1'' was successfully updated!'"); | ||
| ТекстСообщения = пбп_СтроковыеФункцииСлужебныйКлиентСервер.ПодставитьПараметрыВСтроку( | ||
| ТекстСообщения, | ||
| Наименование); | ||
| пбп_ОбщегоНазначенияСлужебный.СообщитьПользователю(ТекстСообщения); | ||
|
|
||
| КонецПроцедуры |
There was a problem hiding this comment.
Несоответствие между русским и английским текстами сообщения
Английский текст сообщения не соответствует русскому. Русский текст: 'Идентификатор ''%1'' не заполнен!', а английский: 'Item ''%1'' was successfully updated!'. Нужно исправить английский текст, чтобы он отражал смысл русского сообщения.
Предлагаемое исправление:
- ТекстСообщения = НСтр("ru = 'Идентификатор ''%1'' не заполнен!';
- |en = 'Item ''%1'' was successfully updated!'");
+ ТекстСообщения = НСтр("ru = 'Идентификатор ''%1'' не заполнен!';
+ |en = 'Identifier ''%1'' is not filled!'");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ТекстСообщения = НСтр("ru = 'Идентификатор ''%1'' не заполнен!'; | |
| |en = 'Item ''%1'' was successfully updated!'"); | |
| ТекстСообщения = пбп_СтроковыеФункцииСлужебныйКлиентСервер.ПодставитьПараметрыВСтроку( | |
| ТекстСообщения, | |
| Наименование); | |
| пбп_ОбщегоНазначенияСлужебный.СообщитьПользователю(ТекстСообщения); | |
| КонецПроцедуры | |
| ТекстСообщения = НСтр("ru = 'Идентификатор ''%1'' не заполнен!'; | |
| |en = 'Identifier ''%1'' is not filled!'"); | |
| ТекстСообщения = пбп_СтроковыеФункцииСлужебныйКлиентСервер.ПодставитьПараметрыВСтроку( | |
| ТекстСообщения, | |
| Наименование); | |
| пбп_ОбщегоНазначенияСлужебный.СообщитьПользователю(ТекстСообщения); | |
| КонецПроцедуры |
| ТекстСообщения = НСтр("ru = 'Идентификатор ''%1'' не уникален!'; | ||
| |en = 'Item ''%1'' was successfully updated!'"); | ||
| ТекстСообщения = пбп_СтроковыеФункцииСлужебныйКлиентСервер.ПодставитьПараметрыВСтроку( | ||
| ТекстСообщения, | ||
| Идентификатор); | ||
| пбп_ОбщегоНазначенияСлужебный.СообщитьПользователю(ТекстСообщения); | ||
|
|
||
| КонецПроцедуры |
There was a problem hiding this comment.
Несоответствие между русским и английским текстами сообщения
Английский текст сообщения не соответствует русскому. Русский текст: 'Идентификатор ''%1'' не уникален!', а английский: 'Item ''%1'' was successfully updated!'. Необходимо исправить английский текст, чтобы он соответствовал русскому.
Предлагаемое исправление:
- ТекстСообщения = НСтр("ru = 'Идентификатор ''%1'' не уникален!';
- |en = 'Item ''%1'' was successfully updated!'");
+ ТекстСообщения = НСтр("ru = 'Идентификатор ''%1'' не уникален!';
+ |en = 'Identifier ''%1'' is not unique!'");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ТекстСообщения = НСтр("ru = 'Идентификатор ''%1'' не уникален!'; | |
| |en = 'Item ''%1'' was successfully updated!'"); | |
| ТекстСообщения = пбп_СтроковыеФункцииСлужебныйКлиентСервер.ПодставитьПараметрыВСтроку( | |
| ТекстСообщения, | |
| Идентификатор); | |
| пбп_ОбщегоНазначенияСлужебный.СообщитьПользователю(ТекстСообщения); | |
| КонецПроцедуры | |
| ТекстСообщения = НСтр("ru = 'Идентификатор ''%1'' не уникален!'; | |
| |en = 'Identifier ''%1'' is not unique!'"); | |
| ТекстСообщения = пбп_СтроковыеФункцииСлужебныйКлиентСервер.ПодставитьПараметрыВСтроку( | |
| ТекстСообщения, | |
| Идентификатор); | |
| пбп_ОбщегоНазначенияСлужебный.СообщитьПользователю(ТекстСообщения); | |
| КонецПроцедуры |
Analysis Details0 Issues
Coverage and Duplications
Project ID: PSSL |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/cf/CommonModules/пбп_ПредопределенныеЗначения/Ext/Module.bsl(4 hunks)
🔇 Additional comments (3)
src/cf/CommonModules/пбп_ПредопределенныеЗначения/Ext/Module.bsl (3)
263-266: Корректная сортировка и обработка дублей!
Правильное решение сначала отсортировать по группам и уровням иерархии, а затем обработать дубликаты идентификаторов.
455-456: Корректное обновление родителя!
Обновление ссылки на родителя выполняется после проверки различий, что предотвращает лишние изменения.
561-581: Корректные процедуры для сообщений!
Процедуры СообщитьОДублированииИдентификаторов и СообщитьОНезаполненностиИдентификатора реализованы правильно:
- Корректные переводы для русского и английского языков
- Единый стиль форматирования сообщений
| Процедура ОбработатьДублиИдентификаторов(Таблица) | ||
|
|
||
| Просмотренные = Новый Соответствие; | ||
|
|
||
| СтрокиДляУдаления = Новый Массив; | ||
| Для Каждого Строка Из Таблица Цикл | ||
| Если ПустаяСтрока(Строка.ИдентификаторНастройки) Тогда | ||
| СообщитьОНезаполненностиИдентификатора(Строка.Наименование); | ||
| Продолжить; | ||
| КонецЕсли; | ||
|
|
||
| ИндексСтроки = Таблица.Индекс(Строка); | ||
|
|
||
| Стр = Просмотренные[Строка.ИдентификаторНастройки]; | ||
| Если Стр <> Неопределено Тогда | ||
| Если Стр.Первый Тогда | ||
| СтрокиДляУдаления.Добавить(Таблица[Стр.Индекс]); | ||
| КонецЕсли; | ||
|
|
||
| СтрокиДляУдаления.Добавить(Строка); | ||
| Стр.Индекс = ИндексСтроки; | ||
| Стр.Первый = Ложь; | ||
|
|
||
| СообщитьОДублированииИдентификаторов(Строка.ИдентификаторНастройки); | ||
| Иначе | ||
| Стр = Новый Структура; | ||
| Стр.Вставить("Индекс", ИндексСтроки); | ||
| Стр.Вставить("Первый", Истина); | ||
| Просмотренные.Вставить(Строка.ИдентификаторНастройки, Стр); | ||
| КонецЕсли; | ||
| КонецЦикла; | ||
|
|
||
| Для Каждого Строка Из СтрокиДляУдаления Цикл | ||
| Таблица.Удалить(Строка); | ||
| КонецЦикла; | ||
|
|
||
| КонецПроцедуры |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Оптимизация производительности при удалении дублей
Текущая реализация накапливает массив строк для удаления и затем удаляет их по одной. При большом количестве дублей это может привести к снижению производительности.
Предлагаю оптимизировать удаление строк следующим образом:
- Для Каждого Строка Из СтрокиДляУдаления Цикл
- Таблица.Удалить(Строка);
- КонецЦикла;
+ // Удаляем строки с конца таблицы, чтобы избежать сдвига индексов
+ СтрокиДляУдаления.Сортировать(НаправлениеСортировки.Убыв);
+ Для Каждого Строка Из СтрокиДляУдаления Цикл
+ Таблица.Удалить(Строка);
+ КонецЦикла;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Процедура ОбработатьДублиИдентификаторов(Таблица) | |
| Просмотренные = Новый Соответствие; | |
| СтрокиДляУдаления = Новый Массив; | |
| Для Каждого Строка Из Таблица Цикл | |
| Если ПустаяСтрока(Строка.ИдентификаторНастройки) Тогда | |
| СообщитьОНезаполненностиИдентификатора(Строка.Наименование); | |
| Продолжить; | |
| КонецЕсли; | |
| ИндексСтроки = Таблица.Индекс(Строка); | |
| Стр = Просмотренные[Строка.ИдентификаторНастройки]; | |
| Если Стр <> Неопределено Тогда | |
| Если Стр.Первый Тогда | |
| СтрокиДляУдаления.Добавить(Таблица[Стр.Индекс]); | |
| КонецЕсли; | |
| СтрокиДляУдаления.Добавить(Строка); | |
| Стр.Индекс = ИндексСтроки; | |
| Стр.Первый = Ложь; | |
| СообщитьОДублированииИдентификаторов(Строка.ИдентификаторНастройки); | |
| Иначе | |
| Стр = Новый Структура; | |
| Стр.Вставить("Индекс", ИндексСтроки); | |
| Стр.Вставить("Первый", Истина); | |
| Просмотренные.Вставить(Строка.ИдентификаторНастройки, Стр); | |
| КонецЕсли; | |
| КонецЦикла; | |
| Для Каждого Строка Из СтрокиДляУдаления Цикл | |
| Таблица.Удалить(Строка); | |
| КонецЦикла; | |
| КонецПроцедуры | |
| Процедура ОбработатьДублиИдентификаторов(Таблица) | |
| Просмотренные = Новый Соответствие; | |
| СтрокиДляУдаления = Новый Массив; | |
| Для Каждого Строка Из Таблица Цикл | |
| Если ПустаяСтрока(Строка.ИдентификаторНастройки) Тогда | |
| СообщитьОНезаполненностиИдентификатора(Строка.Наименование); | |
| Продолжить; | |
| КонецЕсли; | |
| ИндексСтроки = Таблица.Индекс(Строка); | |
| Стр = Просмотренные[Строка.ИдентификаторНастройки]; | |
| Если Стр <> Неопределено Тогда | |
| Если Стр.Первый Тогда | |
| СтрокиДляУдаления.Добавить(Таблица[Стр.Индекс]); | |
| КонецЕсли; | |
| СтрокиДляУдаления.Добавить(Строка); | |
| Стр.Индекс = ИндексСтроки; | |
| Стр.Первый = Ложь; | |
| СообщитьОДублированииИдентификаторов(Строка.ИдентификаторНастройки); | |
| Иначе | |
| Стр = Новый Структура; | |
| Стр.Вставить("Индекс", ИндексСтроки); | |
| Стр.Вставить("Первый", Истина); | |
| Просмотренные.Вставить(Строка.ИдентификаторНастройки, Стр); | |
| КонецЕсли; | |
| КонецЦикла; | |
| // Удаляем строки с конца таблицы, чтобы избежать сдвига индексов | |
| СтрокиДляУдаления.Сортировать(НаправлениеСортировки.Убыв); | |
| Для Каждого Строка Из СтрокиДляУдаления Цикл | |
| Таблица.Удалить(Строка); | |
| КонецЦикла; | |
| КонецПроцедуры |
There was a problem hiding this comment.
Ты не прав, СтрокиДляУдаления.Сортировать(НаправлениеСортировки.Убыв); так нельзя сделать
There was a problem hiding this comment.
@denisushakov, спасибо за ваш комментарий. Учту это на будущее.
(_/)
( •_•)
/ ⊃
✏️ Learnings added
Learnt from: denisushakov
PR: firstBitSportivnaya/PSSL#136
File: src/cf/CommonModules/пбп_ПредопределенныеЗначения/Ext/Module.bsl:634-670
Timestamp: 2024-11-01T15:39:13.679Z
Learning: В процедуре `ОбработатьДублиИдентификаторов` нельзя использовать `СтрокиДляУдаления.Сортировать(НаправлениеСортировки.Убыв);` для сортировки массива `СтрокиДляУдаления`.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Analysis Details0 Issues
Coverage and Duplications
Project ID: PSSL |
Summary by CodeRabbit