Skip to content
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.

Sign up for GitHub

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

Jump to bottom

855 Проверка form-items-single-event-handler #998

Merged
merged 7 commits into from Jul 3, 2022
Merged

855 Проверка form-items-single-event-handler #998

merged 7 commits into from Jul 3, 2022

Conversation

RedMammoth
Copy link
Contributor

@RedMammoth RedMammoth commented Feb 21, 2022 •
edited

Что сделано

  • Реализована проверка единственного назначения обработчиков событий элементов формы

Чек-лист

Общее:

  • ветка PR обновлена из master и нет конфликтов
  • Тесты-кейсы, JUnit тесты правильного и неправильного состояния
  • Измененные Вами исходники отформатированы в соответствии с конвенцией
  • Авто-аудит (SonarQube и CheckStyle) пройден, покрытие кода хорошее, ошибок нет, плохой код устранен
  • Добавлена запись в ИСТОРИЮ ИЗМЕНЕНИЯ, включаемая в пользовательскую документацию плагина

Если применимо:

  • Пользовательская документация на доп.инструменты написана (на русском)
  • Описание проверок - на двух языках

Закрываемые задачи

Closes #855

@1C-Company @marmyshev прошу сделать аудит

@vadimeg vadimeg requested a review from marmyshev June 6, 2022 07:22
@marmyshev
Copy link
Collaborator

@RedMammoth ping! тут что-то далается? как-то завис реквест... :) может даже из-за меня?

@RedMammoth
Copy link
Contributor Author

@RedMammoth ping! тут что-то далается? как-то завис реквест... :) может даже из-за меня?

У меня было пару проблем, я наверное в телеграме их спрашивал. Сейчас обновлю ветку из мастера и освежу в памяти, то с чем не получилось разобраться. Напишу вопросы в ПР, чтобы не потерялись

@RedMammoth RedMammoth force-pushed the feature/855-form-items-one-event-handler branch from f8667c9 to 24000c6 Compare June 24, 2022 20:17
@RedMammoth
Copy link
Contributor Author

@marmyshev Первая проблема, если я вызываю очистку проекта, то в процедуру check ни разу не попадает топовый объект Form, поэтому ошибки не находятся. Ошибки определяются только если изменить какой-нибудь обработчик события в форме/элементах

@RedMammoth
Copy link
Contributor Author

@marmyshev Второе что мне не нравится: если я при добавлении Issue указываю event и фичу FormPackage.Literals.EVENT_HANDLER__NAME. То при двойном клике на ошибке в панели ошибок открывается форма, но на обработчике не позиционируются, также надо обработчиками не висит маркер, ну и Location пустой. Поэтому если таких ошибок несколько на одной форме, пользователям будет сложно понять, к чему относится каждая:
image

@marmyshev
Copy link
Collaborator

Второе что мне не нравится: если я при добавлении Issue указываю event и фичу FormPackage.Literals.EVENT_HANDLER__NAME. То при двойном клике на ошибке в панели ошибок открывается форма, но на обработчике не позиционируются, также надо обработчиками не висит маркер, ну и Location пустой. Поэтому если таких ошибок несколько на одной форме, пользователям будет сложно понять, к чему относится каждая:

Это в общем проблема в ЕДТ - нужно регать ошибку на отображение контекста, на редактор форм если не показывают.
ИМХО, правильно вешать ошибку ровно туда где она есть по сути - это на имя обработчика у элемента.

@marmyshev
Copy link
Collaborator

Первая проблема, если я вызываю очистку проекта, то в процедуру check ни разу не попадает топовый объект Form, поэтому ошибки не находятся. Ошибки определяются только если изменить какой-нибудь обработчик события в форме/элементах

Предложил как поправить подписку на объект - мне кажется должно получиться, с учетом того что реакция на изменения подчиненных объектов потом уже делается "расширением"

@RedMammoth RedMammoth mentioned this pull request Jun 27, 2022
Open
@vadimeg vadimeg self-requested a review June 29, 2022 11:53
@vadimeg
Copy link
Collaborator

vadimeg commented Jun 29, 2022

@RedMammoth You have 2 Checkstyle violations

@RedMammoth RedMammoth force-pushed the feature/855-form-items-one-event-handler branch from 3689645 to 10978d4 Compare June 30, 2022 00:34
@RedMammoth RedMammoth marked this pull request as ready for review June 30, 2022 00:35
@RedMammoth RedMammoth changed the title 855 Проверка form-items-one-event-handler 855 Проверка form-items-single-event-handler Jun 30, 2022
vadimeg
vadimeg requested changes Jun 30, 2022
View reviewed changes
Copy link
Collaborator

@vadimeg vadimeg left a comment

Choose a reason for hiding this comment

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

Требуются изменения

@vadimeg vadimeg self-requested a review June 30, 2022 09:58
@marmyshev marmyshev added the Analyze Включение анализа SonarCloud для PR из форков label Jul 1, 2022
@marmyshev marmyshev added this to the 0.3 для EDT 2022.1 milestone Jul 1, 2022
@RedMammoth
рефакторинг
11365c4 
переделал инъекцию сервиса через RSP
добавил тест, проверяющий очистки маркера после удаления элемента формы
исправил замечания сонара
@RedMammoth
Переделал зависимости на интервалы
40948dc
@sonarcloud
Copy link

sonarcloud bot commented Jul 2, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

94.2% 94.2% Coverage
0.0% 0.0% Duplication

@marmyshev marmyshev merged commit b9d7209 into 1C-Company:master Jul 3, 2022
@RedMammoth RedMammoth deleted the feature/855-form-items-one-event-handler branch July 3, 2022 11:36
DmitryBelov-e1c pushed a commit that referenced this pull request Jul 12, 2022
@RedMammoth
#855 Проверка form-items-single-event-handler (#998)
1615c88 
* Добавил проверку единичного использования обработчика событий
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marmyshev marmyshev marmyshev requested changes

@vadimeg vadimeg vadimeg approved these changes

Assignees

@RedMammoth RedMammoth

Labels
Analyze Включение анализа SonarCloud для PR из форков
Projects
None yet
Milestone
0.3 для EDT 2022.1
3 participants
@RedMammoth @marmyshev @vadimeg