Русское сообщество разработки на PHP-фреймворке Laravel.
Ты не вошёл. Вход тут.
"То их перемещаешь в модель."
В какую? В модель User или EmailChange ?
Не в сети
А после данной строчки будет работать в этом случае $item->getByIdWithUsers(3); и $item->getTopItems();
Хотелось бы, что бы все таки нет. Понимаешь меня?
Я думаю, что я тебя понимаю, но ты меня не понимаешь. Зачем тебе на коллекции или объекте запускать еще какие-то методы? Ты внедряешь класс модели, его используешь для получения объектов и коллекций сколько угодно раз.
А можешь показать, примерно весь класс модели в таком случае для понимания?
Дай свой емэйл, скину пример.
В какую? В модель User или EmailChange?
В данном случае в модель EmailChange, ты же с данными этой модели работаешь.
Если не уверен в советах, подожди ответов других ребят. Может кто-то опишет тебе другие практики.
Изменено AlexeyMezenin (06.08.2017 21:37:58)
Не в сети
Зачем тебе на коллекции или объекте запускать еще какие-то методы?
Вот именно не зачем. Но если наша модель унаследована от Eloquent то я это могу сделать с экземпляром $item в примере нашем... Вызывать какие угодно Eloqeunt методы из него. А это кажется очень лишнее.
Как от такого избавиться?
Не в сети
Экземпляр объекта $item $user итд, на мой взгляд не должен иметь методов Eloqent через него не должны искаться другие юзеры итд... А это возможно. Или я неверно понял??
Не в сети
Email htchtc052@gmail.com кидай пример
Не в сети
В данном случае в модель EmailChange, ты же с данными этой модели работаешь.
Понимаю, но не считаю это красивым. Считаю что нужен класс (репо) в котором работа с юзеров и с ближайшим к нему связанным функционалом той же активацией и сменой мейла..
Делать все в eloquent моделях кажется неверно. А если у меня 2-3 равнозначные таблицы для работы с каким то функционалом. Тк в функционале как правило join'ы или иные тесные использования этих таблиц, то как то не очень удобно будет выбирать в какой из 2-3 моделек это писать. По мне лучше в таком случае написать один класс, из которого уже вызывать Eloquent методы, для всех трех таблиц. А местами и DB:raw для этих же таблиц...
Ты видимо говоришь, о чем простых и типичных случаях. Я же затачиваюсь и на сложности и не красивости.
Другие ответы подожду.. Но пока кроме тебя все молчат...
Не в сети
join и DB:raw - это не ORM, а Query Bulder. Я без них обходился (был один Raw на 50+ моделей), используя Eloquent. Если ты пишешь с помощью кучи join вместо использования связей, тогда используй репозитории.
Другие ответы подожду.. Но пока кроме тебя все молчат...
Ребята, давайте свои советы, не молчите.
Не в сети
"Если ты пишешь с помощью кучи join вместо использования связей, тогда используй репозитории."
Да я так пишу. Более того реальный проект для которого будем эти тестовые разработки потом использовать уже написан с весьма сложными и многосоставными sql запросами..
Не в сети
"ты пишешь с помощью кучи join вместо использования связей, тогда используй репозитории."
Может и от сервиса не стоит отказываться? Или он скорее не нужен все же когда используется в одной форме.. Но если с помощью него повторный код убивается то видимо и сервис нужен..
Не в сети
htclog81
Используй, пожалуйста, QUOTE для цитирования.
Не пиши 2-3-4 сообщения подряд, а остановись, подумай 5 минут и напиши одно.
Не в сети
Proger_XP Есть ли мнение по теме и по заданным вопросам? Репо и сервисы нужно к месту используются? Как можно упростить? Стоит ли объединить добавление, апдейт юзера, активацию его и смену мейла в UserRepo или лучше разделить на несколько Repo как в исходном посте моем?
Чем например плох пример такой https://github.com/bestmomo/laravel5-ex … sitory.php с репо включающим две модели?
Изменено htclog81 (06.08.2017 22:36:02)
Не в сети
Контроллер
namespace App\Http\Controllers\Auth;
use App\Http\Controllers\Controller;
use \App\Http\Requests\ChangeEmail;
use App\Contracts\Auth\ChangeEmailContract;
use Illuminate\Support\Facades\Request;
use Illuminate\Support\Facades\Auth;
class ChangeEmailController extends Controller
{
public function showForm(Request $request)
{
return view('home.account.email')->with([
'title' => 'Change email',
'pageclass' => 'home_account_email',
]
);
}
public function saveForm(ChangeEmail $request, ChangeEmailContract $changeEmailService)
{
$user = Auth::user();
$changeEmailService->sendChangeEmailMail($user, $request->get('email'));
return redirect()->route('home')->with('status', "Confirmation change E-mail link send to ".$request->get('email'));
}
public function emailSet($token, ChangeEmailContract $changeEmailService)
{
$email = Request::get('email');
try {
$user = $changeEmailService->setEmail($token, $email);
}
catch (\App\Exceptions\ChangeEmailNotFoundException $e) {
return redirect()->route('home')
->with('status', $e->getMessage());
}
Auth::login($user);
return redirect()->route('home')
->with('status', 'You successfully activated your new email!');
}
}
Реквест
namespace App\Http\Requests;
use Illuminate\Foundation\Http\FormRequest;
use Illuminate\Support\Facades\Auth;
class ChangeEmail extends FormRequest
{
/**
* Determine if the user is authorized to make this request.
*
* @return bool
*/
public function authorize()
{
return true;
}
/**
* Get the validation rules that apply to the request.
*
* @return array
*/
public function rules()
{
return [
'email' => 'required|email|unique:users',
'password' => 'required|checkpassword',
];
}
public function messages()
{
return [
'email.required' => 'Please enter an email address',
'email.email' => 'Please enter a valid email address',
'email.unique' => 'This e-mail is already taken. ',
'password.required' => 'Please enter your password',
'password.checkpassword' => 'Your enter wrong password',
];
}
}
Сервис
namespace App\Services\Auth;
use \App\Models\User;
use \App\Contracts\Auth\ChangeEmailContract;
use \App\Exceptions\ChangeEmailNotFoundException;
use Illuminate\Mail\Mailer;
use Illuminate\Mail\Message;
use \App\Models\EmailChange;
use Carbon\Carbon;
class ChangeEmailService implements ChangeEmailContract
{
protected $mailer;
protected $changeEmailRepo;
public function __construct()
{
}
public function sendChangeEmailMail($user, $email)
{
$token = $this->createChangeEmail($user, $email);
\Mail::to($email)->send(
new \App\Mail\ChangeEmail(array(
'email' => $email,
'token' => $token,
))
);
}
public function setEmail($token, $email)
{
$changeEmail = EmailChange::where(array('token' => $token, 'email' => $email))->first();
if ($changeEmail === null) {
throw new ChangeEmailNotFoundException();
}
$user = User::find($changeEmail->user_id);
if (!$user) {
throw new ChangeEmailNotFoundException();
}
$user->email = $email;
$user->save();
EmailChange::where('token', $token)->delete();
return $user;
}
private function createChangeEmail($user, $email)
{
$email_change = EmailChange::where('user_id', $user->id)->first();
$token = $this->getToken();
if (!$email_change) {
return EmailChange::insert([
'user_id' => $user->id,
'token' => $token,
'email' => $email,
'created_at' => new Carbon()
]);
}
return EmailChange::where('user_id', $user->id)->update([
'token' => $token,
'email' => $email,
'created_at' => new Carbon()
]);
}
private function getToken()
{
return hash_hmac('sha256', str_random(40), config('app.key'));
}
}
Провайдер
namespace App\Providers\Auth;
use Illuminate\Support\ServiceProvider;
class ChangeEmailProvider extends ServiceProvider
{
protected $defer = false;
public function register()
{
$this->app->bind('App\Contracts\Auth\ChangeEmailContract', function ($app) {
return new \App\Services\Auth\ChangeEmailService();
});
}
public function provides()
{
return ['\App\Contracts\Auth\ChangeEmailContract'];
}
public function boot()
{
}
}
Алексей! Как видишь я склонился к моделям вместо Repo, ну а сервис и его подключение в провайдер оставил и ради отложенной загрузки и тк это все таки ларавельно. Request тоже заюзал.
Изменено htclog81 (08.08.2017 14:55:45)
Не в сети
Как видишь я склонился к моделям вместо Repo
Намного лучше, но у тебя все еще много логики, связанной с данными, висит в классе ChangeEmailService.
Если захочешь сделать еще чище, то и вместо Mail::to используй notifications или вынеси отправку емэйла в отдельный класс/метод.
Изменено AlexeyMezenin (08.08.2017 17:04:11)
Не в сети
Если захочешь сделать еще чище, то и вместо Mail::to используй notifications или вынеси отправку емэйла в отдельный класс/метод.
Как раз хотелось об этом спросить.. А как вынести в нотификацию для user, в данном случае, учитываю что посылать то надо, не на $user->email, а не $request->email, то есть на то, что юзер ввел в форму новый мейл...
Не в сети
у тебя все еще много логики, связанной с данными
Что бы ты вынес в модель?
Не в сети
$user->verified = true;
$user->save();
Activation::where('token', $token)->delete();
Кстати не замечал в примерах, что бы такие вещи оборачивались в транзакцию. Ну то есть когда мы сохранили статус юзера то должны удалять запись с токеном. По отдельности нет смысла. Нужно ли указывать явно транзакцию. Хотя случай конечно не критичный...
Изменено htclog81 (08.08.2017 18:24:30)
Не в сети
Еще вопрос о валидаторе, да той же самой смены мейла.
Все ли норм? И не стоит ли все возможные кастомные валидаторы где то в одном месте накапливать.
Смысл валидатора в том, что бы проверять что юзер ввел свой пароль при попытке менять мейл
namespace App\Providers;
use Illuminate\Support\ServiceProvider;
use Illuminate\Support\Facades\Validator;
use Illuminate\Support\Facades\Auth;
class CheckPasswordValidator extends ServiceProvider
{
/**
* Bootstrap the application services.
*
* @return void
*/
public function boot()
{
Validator::extend('checkpassword', function ($attribute, $value, $parameters, $validator) {
$email = Auth::user()->email;
if (Auth::attempt(array('email' => $email, 'password' => $value))){
return true;
}else{
return false;
}
});
}
/**
* Register the application services.
*
* @return void
*/
public function register()
{
//
}
}
Изменено htclog81 (08.08.2017 18:57:27)
Не в сети