Skip to content

Conversation

@rnik82
Copy link
Collaborator

@rnik82 rnik82 commented Nov 1, 2025

Pull request details

Add admin user management interface. Now admin can update any profile and add new admins or vice versa take over the admin role.
The admin was added via seeds.
Email - [email protected]
Password - password

Issues fixed

issue#1767

Link to demo

https://hexlet-sicp-449a.onrender.com

*/
protected $fillable = [
'name', 'email', 'password',
'name', 'email', 'password', 'github_name', 'is_admin',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://ru.hexlet.io/courses/php-orm-eloquent/lessons/mass-assigment/theory_unit

Но достаточно ли просто не указывать их в форме? Будет ли это безопасно? Не будет. Веб — это HTTP, а значит любой человек, всегда может послать форму в нужный обработчик, добавив туда всё что угодно. Это автоматически означает, что передача данных пачкой в конструктор может привести к очень нежелательным эффектам. Например, к изменению уровня доступа в систему:

Поэтому не стоит админский флаг оставлять в fillable

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Убрал админский флаг fillable

'current_email' => 'Current email',
'go_to_gravatar' => 'Go to Gravatar.com',
'github_name' => 'GitHub username',
'is_admin' => 'Administrator',
Copy link
Collaborator

@fey fey Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

тк это булевый флаг, то стоит его так и называть в тч в интерфейсах

можно просто admin?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Переименовал везде. Вообще is_admin было добавлено еще Дмитрием, когда он админку делал.


@section('content')
<div class="row my-4">
@include('settings._menu')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rnik82 сейчас ты целиком используешь шаблон из профиля, но это некорректно. Форма пользователя и админская форма - они независимы.
тут можно смело это удалять и форму попроще сделать - ведь у нас нет настроек аккаунта пользователя в админке. тут правильней конечно же выводить админское меню
вот для примера как в hexlet-cv сделано - https://github.com/Hexlet/hexlet-cv/blob/main/ruby-version/app/views/web/admin/users/edit.html.slim
https://github.com/Hexlet/hexlet-cv/blob/main/ruby-version/app/views/web/admin/users/_form.html.slim
У каждой сущности своя форма. И за счет шаблонизатора в релсье, там и шаблоны каждый свой подтягивается.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fey тут все сделал как просишь

public function up(): void
{
Schema::table('users', function (Blueprint $table) {
$table->renameColumn('is_admin', 'admin');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rnik82 а для чего переименовывать поле?) Оно ведь булево - верно названо?
Где-то я в комментах писал, относилось к текстам интерфейсным, т.е. в админке норм это назвать admin? или is_admin и так далее. Там нет жесткий требований по переводам.
А вот поле старое лучше вернуть

]);

$user->update($data);
$user->admin = $request->has('admin');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

тк admin в форме - это чекбокс, то стоит его так и назвать.
плюс - валидация. https://laravel.com/docs/12.x/validation#rule-accepted

class="nav-link link-info p-2">{{ __('layout.nav.sicp_book') }}</a></li>
@auth
@if(auth()->user()->is_admin)
@if(auth()->user()->admin)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

давай кстати тут сразу поменяем, не проверку роли, а через проверку политик (@can)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants