review.md

review.md

Review

Общий подход

Некорректно используется сама модель MVC: в частности, в контроллерах *Admin присутствует всего один метод actionIndex(). Так проще обращаться c json-ом, передавая в post-параметрах действие, которое необходимо совершить, но лучше все-таки использовать для каждого действия с моделью свой отдельный путь — легче дебажить, легче ориентироваться в коде, легче переиспользовать код, легко потом прикрутить какой-нибудь backbone с использованием уже почти готового restful api

Не нашли валидации приходящих с клиента данных (возможно, не там искали) — насколько помню, в Yii что-то было. По исходникам Yii я не вижу в QueryBuilder'e какой-либо проверки данных в условии запроса — есть вероятность sql-инъекции.

Частные случаи

if-ы можно разворачивать, чтобы избежать больших уровней вложенности и повышать читабельность кода. Существенно поможет писать код позднее, если ветки условий будут разрастаться.

Лучше еще и в отдельные функции что-то выносить — так будет проще ориентироваться и что-то переиспользовать.

if($model2->load(Yii::$app->request->post())) {
	if(isset($_POST['add_desc'])) {
		$id = $_POST['FashionadminForm']['id'];				//если такого ключа нет, код упадет
		$table3 = Portfolio::getOne($id);					//не очень явный нейминг переменных

		if($table3) {
			$table3->short_desc = $_POST['FashionadminForm']['short_desc'];

			if ($table3->update()) {
				Yii::$app->session->setFlash('msg', 'Запись была успешно обновлена');
				return $this->redirect(['/fashionadmin/?id='.$id]);
			}
			else {
				Yii::$app->session->setFlash('msg', 'Запись не была обновлена');
				return $this->redirect(['/fashionadmin/?id='.$id]);
			}
		}
		else {
			Yii::$app->session->setFlash('msg', 'Записи с выбранным id не было найдено');
			return $this->redirect(['/fashionadmin/?id='.$id]);
		}
	}

	if(isset($_POST['delete_photo'])) {
	....
	}
}

можно преобразовать в что-то вроде

if(!$model2->load(Yii::$app->request->post()))
	return;

if(isset($_POST['add_desc']))
	return $this->addDesc();

if(isset($_POST['delete_photo']))
	return $this->deletePhoto();

private function addDesc(){
	$id = $_POST['FashionadminForm']['id'];
	$portfolio = Portfolio::getOne($id);

	if(!$portfolio){
		Yii::$app->session->setFlash('msg', 'Записи с выбранным id не было найдено');
		return $this->redirect(['/fashionadmin/?id='.$id]);
	}

	//portfolio exist
	$portfolio->short_desc = $_POST['FashionadminForm']['short_desc'];

	if (!$portfolio->update()) {
		Yii::$app->session->setFlash('msg', 'Запись не была обновлена');
		return $this->redirect(['/fashionadmin/?id='.$id]);
	}
     
     //record updated
     Yii::$app->session->setFlash('msg', 'Запись была успешно обновлена');
     return $this->redirect(['/fashionadmin/?id='.$id]);          
}