biplane / yandex-direct

PHP library for Yandex.Direct API.
MIT License
44 stars 23 forks source link

Removed dot-to-dash substitution in client_login #51

Closed Alexsisukin closed 1 year ago

Alexsisukin commented 1 year ago

В директе доступны логины вида name@domain и при замене точки на тире получаем ошибку вида

Объект не найден: В HTTP-заголовке Client-Login указан несуществующий логин

Предлагаю оставить только strtolower

yethee commented 1 year ago

Если убрать str_replace(), то ошибки нет?

Сейчас нормализация логина выполняется согласно требованиям Директа:

Внимание. Если логин пользователя содержит точки и символы верхнего регистра (заглавные буквы), то для получения нормализованного логина их следует заменить, соответственно, дефисами и символами нижнего регистра.

https://yandex.ru/dev/direct/doc/dg-v4/concepts/finance-token.html

Alexsisukin commented 1 year ago

@yethee Да если убираю str_replace() ошибки нет , проверял на своем акаунте

use Biplane\YandexDirect\Api\V5\Contract\GetClientsRequest;
use Biplane\YandexDirect\ApiServiceFactory;
use Biplane\YandexDirect\ConfigBuilder;

$serviceFactory = new ApiServiceFactory();

$config = ConfigBuilder::create()
    ->setAccessToken('')
    ->setClientLogin('<name>@promopult.ru')
    ->setLocale('ru')
    ->getConfig();

$service = $serviceFactory->createService($config, \Biplane\YandexDirect\Api\V5\Clients::class);
$get_clients_request = GetClientsRequest::create()
    ->setFieldNames(['Type', 'ClientInfo', 'Login', 'ClientId']);

$response = $service->get($get_clients_request);
var_dump($response);

image

Alexsisukin commented 1 year ago

@yethee я не уверен, но мне кажется что документация говорит про нормализацию login без доменной части (если она вообще есть) , оставил str_replace только для строки до знака @

yethee commented 1 year ago

Да, такое решение должно быть безопасным. В смысле, не поломает совместимость с V4. Возможно, можно вообще отказаться от нормализации в случае, когда указан email. Но если нет проблем при нормализации левой части, то можно оставить в таком варианте.

Выполни, пожалуйста, в терминале команду:

$ composer cs-fix

Чтобы все тесты в CI стали зелеными.

Alexsisukin commented 1 year ago

@yethee сейчас тесты ci все зеленые , но локально выполнив composer cs-fix , я получил 587 измененных файлов , должен ли я их запушить?

yethee commented 1 year ago

Не, не надо. Думаю, это связано с тем, что cs-fix запускался на PHP 7.4+

Alexsisukin commented 1 year ago

@yethee спасибо

yethee commented 1 year ago

Добавил новый тег, 5.8.0

Спасибо!