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.

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

crm.company.contact.items.get возвращает только один элемент, а нужно несколько #257

Merged
merged 1 commit into from
Feb 10, 2025

Conversation

leshchenko1979
Copy link
Owner

@leshchenko1979 leshchenko1979 commented Feb 10, 2025

Fixes #256

Обзор от Sourcery

Исправления ошибок:

  • Исправлена проблема, когда crm.company.contact.items.get возвращал только один элемент, хотя ожидалось несколько.
Original summary in English

Summary by Sourcery

Bug Fixes:

  • Fix an issue where crm.company.contact.items.get was returning only one item when multiple items were expected.

Copy link

sourcery-ai bot commented Feb 10, 2025

Руководство для проверяющего от Sourcery

Этот PR исправляет проблему, когда эндпоинт crm.company.contact.items.get возвращал только один элемент вместо нескольких. Изменения обновляют логику извлечения ответа, улучшают форматирование тестов и добавляют новый тест с соответствующим файлом фикстуры ответа для проверки возврата нескольких элементов.

Диаграммы не сгенерированы, так как изменения выглядят простыми и не требуют визуального представления.

Изменения на уровне файлов

Изменение Детали Файлы
Рефакторинг форматирования тестов и небольшие корректировки в тестовых функциях
  • Стандартизация форматирования JSON путем замены одинарных кавычек на двойные и очистки от висячих запятых.
  • Скорректированы структурированные параметры в тестовых функциях для улучшения читаемости.
tests/test_server_responses.py
Обновлена логика извлечения ответа в server_response.py
  • Изменен метод extract_results для возврата только первого элемента, если и только если список содержит ровно один элемент; в противном случае возвращается полный извлеченный список.
  • Пересмотрен метод is_nested для проверки длины на основе значений словаря, обеспечивая правильное обнаружение вложенной структуры.
fast_bitrix24/server_response.py
Добавлен новый тест для эндпоинта crm.company.contact.items.get
  • Представлена новая тестовая функция для утверждения, что эндпоинт возвращает несколько элементов.
  • Включен новый файл фикстуры ответа для имитации нескольких элементов контакта.
tests/test_server_responses.py
tests/real_responses/crm_company_contact_items_get.py

Оценка по связанным задачам

Задача Цель Выполнено Объяснение
#256 Убедиться, что crm.company.contact.items.get возвращает несколько элементов контакта, когда присутствует несколько контактов

Возможно, связанные задачи


Советы и команды

Взаимодействие с Sourcery

  • Запустить новую проверку: Оставьте комментарий @sourcery-ai review в pull request.
  • Продолжить обсуждения: Отвечайте непосредственно на комментарии Sourcery.
  • Создать задачу GitHub из комментария проверки: Попросите Sourcery создать
    задачу из комментария проверки, ответив на него. Вы также можете ответить на
    комментарий проверки с помощью @sourcery-ai issue, чтобы создать задачу из него.
  • Сгенерировать заголовок pull request: Напишите @sourcery-ai в любом месте заголовка
    pull request, чтобы сгенерировать заголовок в любое время. Вы также можете оставить комментарий
    @sourcery-ai title в pull request, чтобы (повторно) сгенерировать заголовок в любое время.
  • Сгенерировать краткое описание pull request: Напишите @sourcery-ai summary в любом месте
    тела pull request, чтобы сгенерировать краткое описание PR в любое время именно там, где вы
    хотите. Вы также можете оставить комментарий @sourcery-ai summary в pull request, чтобы
    (повторно) сгенерировать краткое описание в любое время.
  • Сгенерировать руководство для проверяющего: Оставьте комментарий @sourcery-ai guide в pull
    request, чтобы (повторно) сгенерировать руководство для проверяющего в любое время.
  • Разрешить все комментарии Sourcery: Оставьте комментарий @sourcery-ai resolve в pull
    request, чтобы разрешить все комментарии Sourcery. Полезно, если вы уже
    рассмотрели все комментарии и больше не хотите их видеть.
  • Отклонить все проверки Sourcery: Оставьте комментарий @sourcery-ai dismiss в pull
    request, чтобы отклонить все существующие проверки Sourcery. Особенно полезно, если вы
    хотите начать заново с новой проверкой - не забудьте оставить комментарий
    @sourcery-ai review, чтобы запустить новую проверку!
  • Сгенерировать план действий для задачи: Оставьте комментарий @sourcery-ai plan в
    задаче, чтобы сгенерировать план действий для нее.

Настройка вашего опыта

Перейдите на свою панель управления, чтобы:

  • Включить или отключить функции проверки, такие как сгенерированное Sourcery краткое описание pull request,
    руководство для проверяющего и другие.
  • Изменить язык проверки.
  • Добавить, удалить или изменить пользовательские инструкции по проверке.
  • Настроить другие параметры проверки.

Получение помощи

Original review guide in English

Reviewer's Guide by Sourcery

This PR fixes the issue where the crm.company.contact.items.get endpoint was returning only a single element instead of multiple. The changes update the response extraction logic, improve test formatting, and add a new test with a corresponding response fixture to verify multiple items are returned.

No diagrams generated as the changes look simple and do not need a visual representation.

File-Level Changes

Change Details Files
Refactored test formatting and minor adjustments in test functions
  • Standardized JSON formatting by replacing single quotes with double quotes and cleaning up trailing commas.
  • Adjusted structured parameters in test functions to improve readability.
tests/test_server_responses.py
Updated response extraction logic in server_response.py
  • Modified the extract_results method to return only the first element if and only if the list contains exactly one item; otherwise, return the full extracted list.
  • Revised the is_nested method to check the length based on dictionary values ensuring proper nested structure detection.
fast_bitrix24/server_response.py
Added new test for crm.company.contact.items.get endpoint
  • Introduced a new test function to assert the endpoint returns multiple items.
  • Included a new response fixture file to simulate multiple contact items.
tests/test_server_responses.py
tests/real_responses/crm_company_contact_items_get.py

Assessment against linked issues

Issue Objective Addressed Explanation
#256 Ensure that crm.company.contact.items.get returns multiple contact items when multiple contacts are present

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!
  • Generate a plan of action for an issue: Comment @sourcery-ai plan on
    an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@leshchenko1979 leshchenko1979 self-assigned this Feb 10, 2025
Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @leshchenko1979 - I've reviewed your changes - here's some feedback:

Overall Comments:

  • The is_nested function should check if the input is a dictionary with exactly one key, not just one value.
  • Consider adding a more descriptive name to crm_company_contact_items_get.py to clarify its purpose.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟡 Testing: 1 issue found
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

assert results[0]["NAME"] == "Абдуалимова Татьяна Александровна"


def test_crm_company_contact_items_get(bx_dummy):
Copy link

Choose a reason for hiding this comment

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

suggestion (testing): Suggest adding more assertions to cover different aspects of the response.

It would be beneficial to assert the content of the returned list, such as checking the 'CONTACT_ID', 'SORT', 'ROLE_ID', and 'IS_PRIMARY' fields for each item. This ensures that the function not only returns the correct number of elements but also that the elements contain the expected data.

Suggested implementation:

def test_crm_company_contact_items_get(bx_dummy):
    from tests.real_responses.crm_company_contact_items_get import response

    assert isinstance(response, list)
    # Change the expected count if needed
    assert len(response) == 1

    contact_item = response[0]

    # These expected values should be updated to reflect the actual expected data
    assert contact_item["CONTACT_ID"] == "expected_contact_id"
    assert contact_item["SORT"] == "expected_sort"
    assert contact_item["ROLE_ID"] == "expected_role_id"
    assert contact_item["IS_PRIMARY"] == True

Ensure that the expected values (e.g., "expected_contact_id", "expected_sort", "expected_role_id") match the actual response data for the test. If the true expected values are known, replace these placeholder strings accordingly.



def test_crm_company_contact_items_get(bx_dummy):
from tests.real_responses.crm_company_contact_items_get import response
Copy link

Choose a reason for hiding this comment

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

issue (code-quality): Don't import test modules. (dont-import-test-modules)

ExplanationDon't import test modules.

Tests should be self-contained and don't depend on each other.

If a helper function is used by multiple tests,
define it in a helper module,
instead of importing one test from the other.

@leshchenko1979 leshchenko1979 merged commit 9b93011 into master Feb 10, 2025
11 checks passed
@leshchenko1979 leshchenko1979 deleted the leshchenko1979/issue256 branch February 10, 2025 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant