-
Notifications
You must be signed in to change notification settings - Fork 120
fix service Widgets #619
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
base: master
Are you sure you want to change the base?
fix service Widgets #619
Conversation
|
|
||
| foreach ($response as $key => $value) { | ||
| $widgetModel->$key = $value; | ||
| $widgetModel->$key = $widget->$key; |
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.
а что именно за ошибка была?
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.
в других методах нет подобной ошибки?
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.
а что именно за ошибка была?
Ошибка типизации, когда Widgets::install пытался пропихнуть массив в эту функцию
public function setSettingsTemplate(?SettingsTemplatesCollection $settingsTemplate)
Когда эта функция ожидает объект
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.
в других методах нет подобной ошибки?
Похожий функционал нашел в Widgets::uninstall и Webhooks::subscribe
Метод Widgets::uninstall переделал и теперь он возвращает bool
Метод Webhooks::subscribe реализовал также как и Widgets::install
В других сервисах похожего функционала не нашел что могло бы в будущем вызвать такую ошибку
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.
Или как вариант, можно изменить метод __set в классе BaseApiModel на
public function __set($name, $value)
{
$methodName = 'set' . Str::camel(Str::ucfirst($name));
if (method_exists($this, $methodName) && is_callable([$this, $methodName])) {
$method = new \ReflectionMethod($this, $methodName);
$param = $method->getParameters()[0] ?? null;
$type = $param?->getType();
if ($type instanceof \ReflectionNamedType && !$type->isBuiltin()) {
$class = $type->getName();
if (is_array($value) && method_exists($class, 'fromArray')) {
$value = $class::fromArray($value);
}
}
$this->$methodName($value);
}
}
Тут была добавлена рефлексия, которая позволяет получать тип аргумента функции и если передается массив, когда функция ожидает объект, то будет вызываться функция ::fromArray этого объекта в которой будет указан передаваемый массив
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.
скорее так исправить
это более системное решение
4b6c350 to
ade476e
Compare
| { | ||
| $method = new \ReflectionMethod($this, $methodName); | ||
| $param = $method->getParameters()[0] ?? null; | ||
| $type = $param?->getType(); |
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.
либа пхп71 поддерживает
| $response = $this->request->delete($this->getMethod() . '/' . $widgetModel->getCode()); | ||
|
|
||
| return $widgetModel; | ||
| return $response['result']; |
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.
уверен, что это нужно?
нарушение обратной совместимости
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.
Не думаю, что это нужно т.к жалоб не было)
Изменил, эту функцию, чтобы она возвращала именно bool, а не пыталась перезаписывать передаваемый объект пустым телом ответа
Сейчас откатил это изменение
No description provided.