App\Integrations\Traits\SynchronizerPicklist::import()

Complexity

Conditions	9
Paths	72

Size

Total Lines	36
Code Lines	27

Duplication

Lines	0
Ratio	0 %

Importance

Changes	1
Bugs	0	Features	0

<?php
	public function import(): void
	{
		if ($this->config->get('log_all')) {
			$this->controller->log('Start import ' . $this->name, []);
		}
		$isRoleBased = $this->fieldModel->isRoleBased();
		$values = $this->getPicklistValues();

It seems like getPicklistValues() must be provided by classes using this trait. How about adding it as abstract method to this trait?

		$i = 0;
		foreach ($this->cache as $key => $value) {

The property cache does not exist on App\Integrations\Traits\SynchronizerPicklist. Did you mean cacheList?

			if (empty($value)) {
				continue;
			}
			$name = mb_strtolower($value);
			if (empty($values[$name])) {
				try {
					$itemModel = $this->fieldModel->getItemModel();
					$itemModel->validateValue('name', $value);
					$itemModel->set('name', $value);
					if ($isRoleBased) {
						if (empty($this->roleIdList)) {
							$this->roleIdList = array_keys(\Settings_Roles_Record_Model::getAll());
						}
						$itemModel->set('roles', $this->roleIdList);
					}
					$itemModel->save();
					$this->cacheList[$value] = $key;

The property cacheList does not exist. Although not strictly required by PHP, it is generally a best practice to declare properties explicitly.

					++$i;
				} catch (\Throwable $ex) {
					$this->logError('import ' . $this->name, ['API' => $value], $ex);

It seems like logError() must be provided by classes using this trait. How about adding it as abstract method to this trait?

				}
			} else {
				$this->cacheList[$values[$name]] = $key;
			}
		}
		if ($this->config->get('log_all')) {
			$this->controller->log('End import ' . $this->name, ['imported' => $i]);
		}