Add expert application and dashboard management features#13
Merged
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a complete expert application workflow system. It adds new controllers for expert applications and dashboards, implements role-based route protection with a new middleware, removes the appearance/theme customization system, creates database migrations for expert applications with i18n fields, updates authentication flows with role-based redirects, adds frontend pages for expert application submission and dashboard, and modifies navigation and layouts to support distinct admin and expert user roles. Changes
Sequence Diagram(s)sequenceDiagram
actor Applicant
participant Form as Become Expert Form
participant Controller as ExpertApplicationController
participant DB as Database
participant AdminUI as Admin Review Page
participant AdminController as Admin Controller
participant MailService as Mail Service
participant ExpertDash as Expert Dashboard
Applicant->>Form: Fill application form
Form->>Controller: POST /experts/become (validated data + CV)
Controller->>DB: Create ExpertApplication (status=pending)
DB-->>Controller: Application created
Controller-->>Form: Redirect with success
Note over AdminUI: Admin reviews applications
AdminUI->>AdminController: PATCH /admin/expert-applications/{id}/review
AdminController->>DB: Begin transaction
DB->>DB: Resolve/create User
DB->>DB: Create/update Expert (link to User)
DB->>DB: Update ExpertApplication (reviewed_by, status=accepted)
DB-->>AdminController: Transaction committed
AdminController->>MailService: Send ExpertAccountCreated email
MailService->>Applicant: Email with dashboard access
Applicant-->>ExpertDash: Login and access expert dashboard
ExpertDash->>DB: Load expert profile
DB-->>ExpertDash: Display expert metadata
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (9)
.env.example (1)
50-53: Consider aligningconfig/mail.phpdefault port with.env.example(1025).If a user runs without copying/applying
.env.example(or doesn’t setMAIL_PORT), Laravel’s smtp config fallback currently usesenv('MAIL_PORT', 2525)(per the provided context). That means behavior differs depending on whetherMAIL_PORTis set. If your intent is “defaults should use 1025”, consider updating the config fallback to 1025 as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.example around lines 50 - 53, The .env.example sets MAIL_PORT=1025 but config/mail.php currently falls back to env('MAIL_PORT', 2525), causing inconsistent behavior if MAIL_PORT is unset; update the fallback in config/mail.php to use env('MAIL_PORT', 1025) (or otherwise change the default value used by the SMTP port lookup) so the config default matches .env.example's 1025 value and users get consistent behavior when MAIL_PORT is not provided.resources/js/pages/auth/register.tsx (1)
10-12: Reorder type import before regular imports.The ESLint
import/orderrule flags that the type import from@/types/translationshould occur before imports from@/routes. Type imports should typically be grouped at the beginning or in a specific order based on project configuration.♻️ Suggested import reordering
import { Spinner } from '@/components/ui/spinner'; import { useTranslation } from '@/contexts/TranslationContext'; -import { login } from '@/routes'; -import { store } from '@/routes/register'; -import type { TranslationContextValue } from '@/types/translation'; +import type { TranslationContextValue } from '@/types/translation'; +import { login } from '@/routes'; +import { store } from '@/routes/register';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/pages/auth/register.tsx` around lines 10 - 12, The import order violates the import/order rule: move the type-only import TranslationContextValue so it appears before the regular imports (login and store). Update the top of the file so the "import type { TranslationContextValue } from '@/types/translation';" statement is placed above the other import lines (login and store) to satisfy ESLint and keep type imports grouped first.resources/js/layouts/auth-layout.tsx (1)
12-16: Minor nit: consider propagatingundefinedinstead of forcing empty strings.Defaulting
title/descriptionto''can lead to empty headings/paragraphs in the template. If the template can handleundefinedcleanly, consider removing the defaults (or only defaulting when you truly want empty text rendered).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/layouts/auth-layout.tsx` around lines 12 - 16, The AuthLayout currently passes empty strings for title/description which renders empty elements; instead propagate undefined when those props are not provided so the AuthLayoutTemplate can decide to omit rendering. Locate the return that uses AuthLayoutTemplate (component: AuthLayoutTemplate, props: title and description) and stop defaulting title/description to '' — pass the incoming title and description as-is (or explicitly pass undefined) so empty-string defaults are removed and the template can handle absence of values.app/Http/Middleware/EnsureUserHasRole.php (1)
18-20: Consider redirecting unauthenticated users instead of aborting.When
$useris null (unauthenticated), the middleware aborts with HTTP 403 (Forbidden). Standard Laravel authentication middleware typically redirects unauthenticated users to the login page (HTTP 302) rather than returning a 403 error. This is a better user experience and aligns with Laravel conventions.🔄 Proposed refactor for unauthenticated user handling
use Closure; use Illuminate\Http\Request; use Symfony\Component\HttpFoundation\Response; +use Illuminate\Support\Facades\Auth; class EnsureUserHasRole { public function handle(Request $request, Closure $next, string ...$roles): Response { $user = $request->user(); if (! $user) { - abort(403); + return redirect()->route('login'); } if ($roles === []) { return $next($request); } if (! in_array((string) $user->role, $roles, true)) { abort(403); } return $next($request); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Http/Middleware/EnsureUserHasRole.php` around lines 18 - 20, The middleware EnsureUserHasRole currently aborts(403) when $user is null; change that to redirect unauthenticated users to the login route instead of returning 403. In the handle method, detect when $user is falsy and return a redirect-to-login (preserving intended URL) rather than aborting; keep the abort(403) behavior only for authenticated users who lack the required role (i.e., when $user exists but fails the role check). Update the logic around the $user check to first handle unauthenticated redirects and then enforce role-based aborts.app/Models/User.php (1)
15-16: Consider removingemail_verified_atfrom mass-assignable attributes.Including
email_verified_atin the#[Fillable]attribute allows it to be set directly via mass assignment (e.g.,User::create(['email_verified_at' => now()])). Typically, email verification should be handled through Laravel's built-in verification flow to ensure proper security guarantees. This could allow creating verified users without actual email verification.🛡️ Proposed fix for email verification security
-#[Fillable(['name', 'email', 'password', 'role', 'email_verified_at'])] +#[Fillable(['name', 'email', 'password', 'role'])]If you need this for seeding/testing, consider using a factory state instead:
// In UserFactory public function verified(): static { return $this->state(['email_verified_at' => now()]); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Models/User.php` around lines 15 - 16, The User model currently exposes email_verified_at in the #[Fillable] list which allows mass-assignment of verification timestamps; remove email_verified_at from the #[Fillable([...])] attribute in the User class so verification can only be set via the verification flow or explicit code, and instead add a factory state (e.g., UserFactory::verified) that sets email_verified_at for tests/seeding when needed.resources/js/pages/expert/profile-edit.jsx (1)
6-12: Consider extracting FieldError to a shared component.The
FieldErrorcomponent is a reusable UI element used to display validation errors. Consider moving it to a shared components directory if it will be used across other forms in the application.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/pages/expert/profile-edit.jsx` around lines 6 - 12, Extract the FieldError component into a shared UI components module so it can be reused across forms: create a new shared component (e.g., FieldError) that exports the function/component, move the JSX and className logic there, update the current file's import to import FieldError from the new shared component, and ensure any other form files import the shared FieldError instead of duplicating the component; keep the prop signature ({ error }) and return behavior identical to preserve existing usage.resources/views/emails/expert-account-created.blade.php (2)
18-23: Consider security implications of password delivery via email.The temporary password is sent in plain text. While acceptable for initial account setup, consider:
- Using a password reset link with limited-time validity instead
- Forcing password change on first login (verify this is implemented in Security settings)
- Logging the action for audit purposes
The current implementation does include instructions to change the password (Line 20), which is good practice.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/views/emails/expert-account-created.blade.php` around lines 18 - 23, The template expert-account-created.blade.php currently injects a plain-text $temporaryPassword into the email; replace this flow by sending a time-limited password reset token/link instead (generate token in the account creation controller and pass e.g. $passwordResetUrl to the view), ensure backend logic forces a password change on first login (verify/enable the "force password reset" flag in the user model or auth middleware), and add an audit log entry when the initial account email is sent (log in the same controller method that creates the user and mails the view).
1-33: Add support for i18n/localization if the application supports multiple languages.The email template uses hardcoded English strings. If this application serves users in multiple locales, consider using Laravel's translation system with
__('key')for translatable strings like the greeting, body text, and sign-off.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/views/emails/expert-account-created.blade.php` around lines 1 - 33, Replace hardcoded English strings in the expert-account-created Blade view by using Laravel's translation helpers (e.g., __('...') or `@lang`) and translation keys (create keys such as emails.expert.subject, emails.expert.greeting, emails.expert.body, emails.expert.login_label, emails.expert.temporary_password, emails.expert.footer) so dynamic content like $user->name, $user->email and $temporaryPassword are passed as parameters (e.g., __('emails.expert.greeting', ['name' => $user->name])) and URLs remain built with url('/login') and url('/expert/dashboard'); update the view to call those translation keys and add corresponding entries to your language files so the template supports multiple locales.app/Http/Responses/LoginResponse.php (1)
10-25: Consider using role constants or an enum instead of magic strings.The role values
'expert'and'admin'are hardcoded strings. For maintainability and type safety, consider defining role constants in the User model or using a backed enum:// In User model const ROLE_ADMIN = 'admin'; const ROLE_EXPERT = 'expert'; // Or: enum Role: string { case ADMIN = 'admin'; case EXPERT = 'expert'; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Http/Responses/LoginResponse.php` around lines 10 - 25, Replace the magic role strings in LoginResponse::toResponse with centralized role identifiers: add ROLE_ADMIN and ROLE_EXPERT constants (or a backed enum Role) to the User model and update the checks in toResponse to compare $user->role against User::ROLE_EXPERT / User::ROLE_ADMIN (or Role::EXPERT/Role::ADMIN) instead of the literal 'expert' and 'admin', ensuring imports/uses are added where needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Around line 52-53: Dotenv key ordering in .env.example triggers linter
warnings: move MAIL_HOST to appear before MAIL_MAILER and move MAIL_PORT to
appear before MAIL_SCHEME so keys are alphabetically/specified-order correct;
update the entries for MAIL_HOST and MAIL_PORT in .env.example accordingly
(ensuring their values remain unchanged) so the dotenv-linter ordering warnings
are resolved.
In `@app/Http/Controllers/Admin/ExpertApplicationController.php`:
- Around line 71-100: Move the pending-status check and row locking into the DB
transaction: inside the transaction callback, re-query the application with a
SELECT ... FOR UPDATE (e.g., $application =
\App\Models\ExpertApplication::where('id',
$application->id)->lockForUpdate()->first();) then verify $application->status
=== 'pending' before calling createOrResolveExpertUser or
createExpertFromApplication and updating the row; if not pending, throw/return
to abort the transaction. This ensures a pessimistic lock on the application row
and prevents two admins from concurrently accepting the same application.
In `@app/Http/Controllers/Expert/ProfileController.php`:
- Around line 21-34: The profile rendering and update validation in
ProfileController omit twitter_url and instagram_url so experts cannot edit
them; add 'twitter_url' and 'instagram_url' to the returned 'expert' array in
the Inertia::render payload (sourcing from
$socials['twitter']/$socials['instagram'] or $details as appropriate) and update
the controller's validation/processing (e.g., in the update method referenced
around lines 42-63) to accept, validate (URL/nullable), sanitize, and persist
these two fields when saving the expert's socials. Ensure the same keys
('twitter_url', 'instagram_url') are consistently used in the render payload,
validation rules, and persistence logic.
- Around line 64-77: The controller currently overwrites all locale variants
with the English values; fetch the existing translations from the $expert model
for name, title and details (e.g., $expert->name, $expert->title,
$expert->details['bio']), then update only the 'en' key while preserving 'fr'
and 'ar' (merge existing arrays and set 'en' => $name/$title), and for
details.bio preserve any existing localized bio values instead of replacing the
entire array; use these merged arrays in $expert->update(...) so only the
English entry is replaced and other locales remain intact.
In `@app/Http/Controllers/ExpertApplicationController.php`:
- Around line 67-70: Store CVs on a non-public disk instead of the 'public'
disk: replace the store call in ExpertApplicationController (the code that sets
$cvPath from $request->file('cv')->store(...)) to use a private disk (e.g.,
Storage::disk('private')->putFile('expert-applications/cv',
$request->file('cv')) or $request->file('cv')->store('expert-applications/cv',
'private')), ensure the 'private' disk is configured in filesystems.php, and
stop exposing direct public URLs—serve downloads via an authenticated controller
(using Storage::disk('private')->download(...) or generate temporary signed
URLs) so access is authorization-based rather than path-based.
In `@app/Mail/ExpertAccountCreated.php`:
- Around line 12-20: The ExpertAccountCreated mailable is sent synchronously;
make it async by having the class implement
Illuminate\Contracts\Queue\ShouldQueue (add the interface to the class
declaration for ExpertAccountCreated) — it already uses the Queueable trait and
SerializesModels so no further changes to the mailable body are needed — then
update the send site to use Mail::to(...)->queue(new ExpertAccountCreated(...))
instead of Mail::to(...)->send(...) (replace the synchronous call where
ExpertAccountCreated is instantiated). Ensure the ShouldQueue import is added
and tests/queues are configured where required.
In `@resources/js/layouts/app-layout.tsx`:
- Around line 20-24: The settings routes are being treated as back-office only
for admin/expert, which excludes regular authenticated users; update the logic
so settings are recognized independently: keep const isSettingsPage =
currentPath.startsWith('/settings/'); change isBackOfficePage to only detect
admin/expert areas (e.g. currentPath.startsWith('/admin/') ||
currentPath.startsWith('/expert/')) and ensure downstream layout decisions use
isSettingsPage || isBackOfficePage (or similar) so /settings/* uses the settings
shell for all authenticated users regardless of role.
In `@resources/js/pages/experts/become.jsx`:
- Around line 116-133: The form `locale` field is hardcoded to 'en' in the
`useForm` call so submissions always store English; change initialization to use
the current translation locale from `useTranslation()` (e.g. `i18n.language` or
equivalent) when building the initial state for `useForm` so `locale` reflects
the active language; update the `useForm` call in this file (where `locale` is
set) to use the value returned by `useTranslation()` (fall back to 'en' only if
that value is missing).
In `@resources/js/pages/experts/index.jsx`:
- Around line 239-250: The three dashboard labels currently hardcoded
("Experts", "Countries", "Active filters") and the empty-state text further down
should be replaced with the localization layer used elsewhere (e.g., the
TransText component or t function) so the page isn't mixed-language for FR/AR
users; locate the JSX where expertsProp.length, countriesCount, and
activeFilters.length are rendered and replace their adjacent literal label
strings with <TransText> keys or t('...') calls consistent with the rest of the
file, and do the same for the empty-state text referenced around the empty-state
block (lines noted in the review) so all UI strings use the same translation
keys/components.
In `@routes/web.php`:
- Around line 117-120: The current shared Route::get('dashboard', fn () =>
redirect()->route('admin.dashboard')) is protected by ->middleware('role:admin')
so experts 403; remove that role middleware and replace the closure with a
role-aware dispatch that checks the authenticated user's role (e.g.,
auth()->user()->hasRole(...) or isAdmin()/isExpert()) and redirects to the
appropriate route (admin.dashboard, expert.dashboard, etc.), keeping the route
name('dashboard') and the auth/verified middleware intact so stale /dashboard
links route users by role instead of forbidding them.
---
Nitpick comments:
In @.env.example:
- Around line 50-53: The .env.example sets MAIL_PORT=1025 but config/mail.php
currently falls back to env('MAIL_PORT', 2525), causing inconsistent behavior if
MAIL_PORT is unset; update the fallback in config/mail.php to use
env('MAIL_PORT', 1025) (or otherwise change the default value used by the SMTP
port lookup) so the config default matches .env.example's 1025 value and users
get consistent behavior when MAIL_PORT is not provided.
In `@app/Http/Middleware/EnsureUserHasRole.php`:
- Around line 18-20: The middleware EnsureUserHasRole currently aborts(403) when
$user is null; change that to redirect unauthenticated users to the login route
instead of returning 403. In the handle method, detect when $user is falsy and
return a redirect-to-login (preserving intended URL) rather than aborting; keep
the abort(403) behavior only for authenticated users who lack the required role
(i.e., when $user exists but fails the role check). Update the logic around the
$user check to first handle unauthenticated redirects and then enforce
role-based aborts.
In `@app/Http/Responses/LoginResponse.php`:
- Around line 10-25: Replace the magic role strings in LoginResponse::toResponse
with centralized role identifiers: add ROLE_ADMIN and ROLE_EXPERT constants (or
a backed enum Role) to the User model and update the checks in toResponse to
compare $user->role against User::ROLE_EXPERT / User::ROLE_ADMIN (or
Role::EXPERT/Role::ADMIN) instead of the literal 'expert' and 'admin', ensuring
imports/uses are added where needed.
In `@app/Models/User.php`:
- Around line 15-16: The User model currently exposes email_verified_at in the
#[Fillable] list which allows mass-assignment of verification timestamps; remove
email_verified_at from the #[Fillable([...])] attribute in the User class so
verification can only be set via the verification flow or explicit code, and
instead add a factory state (e.g., UserFactory::verified) that sets
email_verified_at for tests/seeding when needed.
In `@resources/js/layouts/auth-layout.tsx`:
- Around line 12-16: The AuthLayout currently passes empty strings for
title/description which renders empty elements; instead propagate undefined when
those props are not provided so the AuthLayoutTemplate can decide to omit
rendering. Locate the return that uses AuthLayoutTemplate (component:
AuthLayoutTemplate, props: title and description) and stop defaulting
title/description to '' — pass the incoming title and description as-is (or
explicitly pass undefined) so empty-string defaults are removed and the template
can handle absence of values.
In `@resources/js/pages/auth/register.tsx`:
- Around line 10-12: The import order violates the import/order rule: move the
type-only import TranslationContextValue so it appears before the regular
imports (login and store). Update the top of the file so the "import type {
TranslationContextValue } from '@/types/translation';" statement is placed above
the other import lines (login and store) to satisfy ESLint and keep type imports
grouped first.
In `@resources/js/pages/expert/profile-edit.jsx`:
- Around line 6-12: Extract the FieldError component into a shared UI components
module so it can be reused across forms: create a new shared component (e.g.,
FieldError) that exports the function/component, move the JSX and className
logic there, update the current file's import to import FieldError from the new
shared component, and ensure any other form files import the shared FieldError
instead of duplicating the component; keep the prop signature ({ error }) and
return behavior identical to preserve existing usage.
In `@resources/views/emails/expert-account-created.blade.php`:
- Around line 18-23: The template expert-account-created.blade.php currently
injects a plain-text $temporaryPassword into the email; replace this flow by
sending a time-limited password reset token/link instead (generate token in the
account creation controller and pass e.g. $passwordResetUrl to the view), ensure
backend logic forces a password change on first login (verify/enable the "force
password reset" flag in the user model or auth middleware), and add an audit log
entry when the initial account email is sent (log in the same controller method
that creates the user and mails the view).
- Around line 1-33: Replace hardcoded English strings in the
expert-account-created Blade view by using Laravel's translation helpers (e.g.,
__('...') or `@lang`) and translation keys (create keys such as
emails.expert.subject, emails.expert.greeting, emails.expert.body,
emails.expert.login_label, emails.expert.temporary_password,
emails.expert.footer) so dynamic content like $user->name, $user->email and
$temporaryPassword are passed as parameters (e.g., __('emails.expert.greeting',
['name' => $user->name])) and URLs remain built with url('/login') and
url('/expert/dashboard'); update the view to call those translation keys and add
corresponding entries to your language files so the template supports multiple
locales.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eda6240f-7577-42cc-915f-71799150379e
📒 Files selected for processing (49)
.env.exampleapp/Http/Controllers/Admin/ExpertApplicationController.phpapp/Http/Controllers/Admin/ExpertController.phpapp/Http/Controllers/Expert/DashboardController.phpapp/Http/Controllers/Expert/ProfileController.phpapp/Http/Controllers/ExpertApplicationController.phpapp/Http/Middleware/EnsureUserHasRole.phpapp/Http/Middleware/HandleAppearance.phpapp/Http/Responses/LoginResponse.phpapp/Mail/ExpertAccountCreated.phpapp/Models/Expert.phpapp/Models/ExpertApplication.phpapp/Models/User.phpapp/Providers/FortifyServiceProvider.phpbootstrap/app.phpconfig/fortify.phpdatabase/migrations/2026_04_27_000001_create_expert_applications_table.phpdatabase/migrations/2026_04_27_120000_add_user_id_to_experts_table.phpdatabase/migrations/2026_04_27_130000_add_i18n_fields_to_expert_applications_table.phpdatabase/migrations/2026_04_27_131000_add_profile_fields_to_expert_applications_table.phpresources/js/app.tsxresources/js/components/app-sidebar.tsxresources/js/components/appearance-tabs.tsxresources/js/components/two-factor-setup-modal.tsxresources/js/hooks/use-appearance.tsxresources/js/layouts/app-layout.tsxresources/js/layouts/auth-layout.tsxresources/js/layouts/auth/auth-simple-layout.tsxresources/js/layouts/settings/layout.tsxresources/js/pages/admin/experts/application-show.jsxresources/js/pages/admin/experts/applications.jsxresources/js/pages/admin/experts/create.jsxresources/js/pages/admin/experts/index.jsxresources/js/pages/admin/experts/partials/ExpertPublicProfileDetails.jsxresources/js/pages/auth/login.tsxresources/js/pages/auth/register.tsxresources/js/pages/expert/dashboard.jsxresources/js/pages/expert/profile-edit.jsxresources/js/pages/experts/Partials/Details/ProfileSidebar.jsxresources/js/pages/experts/[id].jsxresources/js/pages/experts/become.jsxresources/js/pages/experts/index.jsxresources/js/pages/settings/appearance.tsxresources/views/app.blade.phpresources/views/emails/expert-account-created.blade.phproutes/admin.phproutes/expert.phproutes/settings.phproutes/web.php
💤 Files with no reviewable changes (12)
- resources/js/layouts/settings/layout.tsx
- resources/js/app.tsx
- resources/js/pages/settings/appearance.tsx
- resources/js/pages/experts/Partials/Details/ProfileSidebar.jsx
- routes/settings.php
- resources/js/components/appearance-tabs.tsx
- resources/js/components/two-factor-setup-modal.tsx
- app/Http/Middleware/HandleAppearance.php
- app/Http/Controllers/Admin/ExpertController.php
- resources/js/pages/admin/experts/create.jsx
- resources/js/hooks/use-appearance.tsx
- resources/js/pages/admin/experts/partials/ExpertPublicProfileDetails.jsx
Comment on lines
52
to
+53
| MAIL_HOST=127.0.0.1 | ||
| MAIL_PORT=2525 | ||
| MAIL_PORT=1025 |
There was a problem hiding this comment.
Fix dotenv-linter key ordering warnings (if lint is enforced in CI).
The static analysis hints report:
MAIL_HOSTshould be ordered beforeMAIL_MAILERMAIL_PORTshould be ordered beforeMAIL_SCHEME
This is unlikely to affect runtime behavior, but it will fail linting if you treat dotenv ordering warnings as errors. Consider reordering these keys in .env.example to keep CI clean.
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 52-52: [UnorderedKey] The MAIL_HOST key should go before the MAIL_MAILER key
(UnorderedKey)
[warning] 53-53: [UnorderedKey] The MAIL_PORT key should go before the MAIL_SCHEME key
(UnorderedKey)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.env.example around lines 52 - 53, Dotenv key ordering in .env.example
triggers linter warnings: move MAIL_HOST to appear before MAIL_MAILER and move
MAIL_PORT to appear before MAIL_SCHEME so keys are
alphabetically/specified-order correct; update the entries for MAIL_HOST and
MAIL_PORT in .env.example accordingly (ensuring their values remain unchanged)
so the dotenv-linter ordering warnings are resolved.
Comment on lines
+71
to
+100
| if ($application->status !== 'pending') { | ||
| return back()->with('error', 'This application has already been reviewed.'); | ||
| } | ||
|
|
||
| $credentialPayload = null; | ||
| $mailFailed = false; | ||
|
|
||
| DB::transaction(function () use ($application, $data, $request, &$credentialPayload): void { | ||
| $expertId = $application->expert_id; | ||
|
|
||
| if ($data['decision'] === 'accepted') { | ||
| $account = $this->createOrResolveExpertUser($application); | ||
| $expert = $this->createExpertFromApplication($application, $account['user']); | ||
| $expertId = $expert->id; | ||
|
|
||
| $credentialPayload = [ | ||
| 'user' => $account['user'], | ||
| 'temporary_password' => $account['temporary_password'], | ||
| 'new_account' => $account['new_account'], | ||
| ]; | ||
| } | ||
|
|
||
| $application->update([ | ||
| 'status' => $data['decision'], | ||
| 'admin_notes' => $data['admin_notes'] ?? null, | ||
| 'reviewed_at' => now(), | ||
| 'reviewed_by_id' => $request->user()?->id, | ||
| 'expert_id' => $expertId, | ||
| ]); | ||
| }); |
There was a problem hiding this comment.
Lock the application row before processing a review.
The pending check happens before DB::transaction(), and the transaction never lockForUpdate()s the application row. Two admins can accept the same application concurrently, both pass the stale status check, and both create/link expert records before one final status update wins.
Suggested direction
- if ($application->status !== 'pending') {
- return back()->with('error', 'This application has already been reviewed.');
- }
-
$credentialPayload = null;
$mailFailed = false;
DB::transaction(function () use ($application, $data, $request, &$credentialPayload): void {
+ $application = ExpertApplication::query()
+ ->whereKey($application->getKey())
+ ->lockForUpdate()
+ ->firstOrFail();
+
+ if ($application->status !== 'pending') {
+ throw new \RuntimeException('This application has already been reviewed.');
+ }
+
$expertId = $application->expert_id;
...
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Http/Controllers/Admin/ExpertApplicationController.php` around lines 71 -
100, Move the pending-status check and row locking into the DB transaction:
inside the transaction callback, re-query the application with a SELECT ... FOR
UPDATE (e.g., $application = \App\Models\ExpertApplication::where('id',
$application->id)->lockForUpdate()->first();) then verify $application->status
=== 'pending' before calling createOrResolveExpertUser or
createExpertFromApplication and updating the row; if not pending, throw/return
to abort the transaction. This ensures a pessimistic lock on the application row
and prevents two admins from concurrently accepting the same application.
Comment on lines
+21
to
+34
| return Inertia::render('expert/profile-edit', [ | ||
| 'expert' => [ | ||
| 'id' => $expert->id, | ||
| 'name' => (string) ($expert->name['en'] ?? ''), | ||
| 'title' => (string) ($expert->title['en'] ?? ''), | ||
| 'email' => (string) ($expert->email ?? ''), | ||
| 'phone' => (string) ($details['phone'] ?? ''), | ||
| 'country' => (string) ($expert->country ?? ''), | ||
| 'city' => is_string($expert->location) ? $expert->location : '', | ||
| 'expertise' => (string) ($details['expertise_text'] ?? ''), | ||
| 'bio' => (string) (($details['bio'][0]['en'] ?? '') ?: ''), | ||
| 'linkedin_url' => (string) ($socials['linkedin'] ?? ''), | ||
| 'portfolio_url' => (string) ($details['portfolio_url'] ?? ''), | ||
| ], |
There was a problem hiding this comment.
Twitter and Instagram can't be managed from the profile flow.
The application form captures twitter_url and instagram_url, but this controller never returns or validates them. After onboarding, experts can no longer review or update those social links from their profile.
Also applies to: 42-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Http/Controllers/Expert/ProfileController.php` around lines 21 - 34, The
profile rendering and update validation in ProfileController omit twitter_url
and instagram_url so experts cannot edit them; add 'twitter_url' and
'instagram_url' to the returned 'expert' array in the Inertia::render payload
(sourcing from $socials['twitter']/$socials['instagram'] or $details as
appropriate) and update the controller's validation/processing (e.g., in the
update method referenced around lines 42-63) to accept, validate (URL/nullable),
sanitize, and persist these two fields when saving the expert's socials. Ensure
the same keys ('twitter_url', 'instagram_url') are consistently used in the
render payload, validation rules, and persistence logic.
Comment on lines
+64
to
+77
| $details['bio'] = [ | ||
| [ | ||
| 'en' => trim((string) ($data['bio'] ?? '')), | ||
| 'fr' => trim((string) ($data['bio'] ?? '')), | ||
| 'ar' => trim((string) ($data['bio'] ?? '')), | ||
| ], | ||
| ]; | ||
|
|
||
| $title = trim((string) ($data['title'] ?? '')); | ||
| $name = trim((string) $data['name']); | ||
|
|
||
| $expert->update([ | ||
| 'name' => ['en' => $name, 'fr' => $name, 'ar' => $name], | ||
| 'title' => ['en' => $title, 'fr' => $title, 'ar' => $title], |
There was a problem hiding this comment.
Don't overwrite translated fields with the English edit.
This update path rewrites name, title, and details.bio so that fr and ar are replaced with the English value on every save. Any translations collected during application review are lost as soon as the expert edits their profile.
Suggested direction
- $details['bio'] = [
- [
- 'en' => trim((string) ($data['bio'] ?? '')),
- 'fr' => trim((string) ($data['bio'] ?? '')),
- 'ar' => trim((string) ($data['bio'] ?? '')),
- ],
- ];
+ $existingBio = is_array($details['bio'][0] ?? null) ? $details['bio'][0] : [];
+ $bioEn = trim((string) ($data['bio'] ?? ''));
+ $details['bio'] = [[
+ 'en' => $bioEn,
+ 'fr' => (string) ($existingBio['fr'] ?? ''),
+ 'ar' => (string) ($existingBio['ar'] ?? ''),
+ ]];
...
- 'name' => ['en' => $name, 'fr' => $name, 'ar' => $name],
- 'title' => ['en' => $title, 'fr' => $title, 'ar' => $title],
+ 'name' => array_replace(is_array($expert->name) ? $expert->name : [], ['en' => $name]),
+ 'title' => array_replace(is_array($expert->title) ? $expert->title : [], ['en' => $title]),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Http/Controllers/Expert/ProfileController.php` around lines 64 - 77, The
controller currently overwrites all locale variants with the English values;
fetch the existing translations from the $expert model for name, title and
details (e.g., $expert->name, $expert->title, $expert->details['bio']), then
update only the 'en' key while preserving 'fr' and 'ar' (merge existing arrays
and set 'en' => $name/$title), and for details.bio preserve any existing
localized bio values instead of replacing the entire array; use these merged
arrays in $expert->update(...) so only the English entry is replaced and other
locales remain intact.
Comment on lines
+67
to
+70
| $cvPath = null; | ||
| if ($request->hasFile('cv')) { | ||
| $cvPath = $request->file('cv')->store('expert-applications/cv', 'public'); | ||
| } |
There was a problem hiding this comment.
Store CV uploads on a private disk.
These files are personal documents, but they are being written to the publicly served public disk and later exposed via a direct URL. That makes CV access path-based instead of authorization-based. Use a private disk and serve downloads through an authenticated controller or temporary signed URL.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Http/Controllers/ExpertApplicationController.php` around lines 67 - 70,
Store CVs on a non-public disk instead of the 'public' disk: replace the store
call in ExpertApplicationController (the code that sets $cvPath from
$request->file('cv')->store(...)) to use a private disk (e.g.,
Storage::disk('private')->putFile('expert-applications/cv',
$request->file('cv')) or $request->file('cv')->store('expert-applications/cv',
'private')), ensure the 'private' disk is configured in filesystems.php, and
stop exposing direct public URLs—serve downloads via an authenticated controller
(using Storage::disk('private')->download(...) or generate temporary signed
URLs) so access is authorization-based rather than path-based.
Comment on lines
+12
to
+20
| class ExpertAccountCreated extends Mailable | ||
| { | ||
| use Queueable, SerializesModels; | ||
|
|
||
| public function __construct( | ||
| public User $user, | ||
| public ?string $temporaryPassword, | ||
| public bool $isNewAccount, | ||
| ) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
echo "Inspect ExpertAccountCreated usage:"
rg -n -C3 '\bExpertAccountCreated\b|->send\(|->queue\(|Mail::to\(' app
echo
echo "Inspect the expert application review controller:"
fd 'ExpertApplicationController.php' app --exec sed -n '1,260p' {}Repository: lionsgeeks/tilila
Length of output: 18738
Implement ShouldQueue and dispatch this mailable asynchronously.
The mail is currently sent synchronously with ->send() at line 107 of ExpertApplicationController.php, blocking the request while SMTP communication completes. Although the database transaction commits before the mail send attempt, synchronous delivery still creates a reliability and performance hazard: slow mail services will hang the admin response, and failures—though handled—interrupt the workflow.
Add the ShouldQueue interface to ExpertAccountCreated and switch to Mail::to(...)->queue(new ExpertAccountCreated(...)) to dispatch delivery asynchronously.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Mail/ExpertAccountCreated.php` around lines 12 - 20, The
ExpertAccountCreated mailable is sent synchronously; make it async by having the
class implement Illuminate\Contracts\Queue\ShouldQueue (add the interface to the
class declaration for ExpertAccountCreated) — it already uses the Queueable
trait and SerializesModels so no further changes to the mailable body are needed
— then update the send site to use Mail::to(...)->queue(new
ExpertAccountCreated(...)) instead of Mail::to(...)->send(...) (replace the
synchronous call where ExpertAccountCreated is instantiated). Ensure the
ShouldQueue import is added and tests/queues are configured where required.
Comment on lines
+20
to
+24
| const isSettingsPage = currentPath.startsWith('/settings/'); | ||
| const isBackOfficePage = | ||
| currentPath.startsWith('/admin/') || | ||
| currentPath.startsWith('/expert/') || | ||
| (isSettingsPage && (role === 'admin' || role === 'expert')); |
There was a problem hiding this comment.
Don’t exclude regular users from the settings shell.
/settings/* is now considered a back-office page only for admin/expert, so normal authenticated users fall back to the public navbar/footer layout on their account settings pages. That’s a regression for the default profile/security flow.
Suggested fix
const isSettingsPage = currentPath.startsWith('/settings/');
const isBackOfficePage =
currentPath.startsWith('/admin/') ||
currentPath.startsWith('/expert/') ||
- (isSettingsPage && (role === 'admin' || role === 'expert'));
+ (isSettingsPage && Boolean(page.props.auth?.user));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const isSettingsPage = currentPath.startsWith('/settings/'); | |
| const isBackOfficePage = | |
| currentPath.startsWith('/admin/') || | |
| currentPath.startsWith('/expert/') || | |
| (isSettingsPage && (role === 'admin' || role === 'expert')); | |
| const isSettingsPage = currentPath.startsWith('/settings/'); | |
| const isBackOfficePage = | |
| currentPath.startsWith('/admin/') || | |
| currentPath.startsWith('/expert/') || | |
| (isSettingsPage && Boolean(page.props.auth?.user)); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@resources/js/layouts/app-layout.tsx` around lines 20 - 24, The settings
routes are being treated as back-office only for admin/expert, which excludes
regular authenticated users; update the logic so settings are recognized
independently: keep const isSettingsPage = currentPath.startsWith('/settings/');
change isBackOfficePage to only detect admin/expert areas (e.g.
currentPath.startsWith('/admin/') || currentPath.startsWith('/expert/')) and
ensure downstream layout decisions use isSettingsPage || isBackOfficePage (or
similar) so /settings/* uses the settings shell for all authenticated users
regardless of role.
Comment on lines
+116
to
+133
| } = useForm({ | ||
| name_i18n: { en: '', fr: '', ar: '' }, | ||
| email: '', | ||
| phone: '', | ||
| country: '', | ||
| city: '', | ||
| industries_text: '', | ||
| languages_text: '', | ||
| title_i18n: { en: '', fr: '', ar: '' }, | ||
| expertise_i18n: { en: '', fr: '', ar: '' }, | ||
| bio_i18n: { en: '', fr: '', ar: '' }, | ||
| linkedin_url: '', | ||
| twitter_url: '', | ||
| instagram_url: '', | ||
| portfolio_url: '', | ||
| cv: null, | ||
| locale: 'en', | ||
| }); |
There was a problem hiding this comment.
Submit the active locale instead of hardcoding English.
locale is always initialized to 'en', so every application is stored as English even when the form is completed in French or Arabic. Use the value from useTranslation() when building the form payload.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@resources/js/pages/experts/become.jsx` around lines 116 - 133, The form
`locale` field is hardcoded to 'en' in the `useForm` call so submissions always
store English; change initialization to use the current translation locale from
`useTranslation()` (e.g. `i18n.language` or equivalent) when building the
initial state for `useForm` so `locale` reflects the active language; update the
`useForm` call in this file (where `locale` is set) to use the value returned by
`useTranslation()` (fall back to 'en' only if that value is missing).
Comment on lines
+239
to
+250
| <div className="mx-auto mt-8 grid max-w-3xl grid-cols-3 gap-3"> | ||
| <div className="rounded-xl border border-border/70 bg-card/70 p-4 text-center shadow-sm"> | ||
| <p className="text-2xl font-bold text-tblack">{expertsProp.length}</p> | ||
| <p className="text-xs text-tgray">Experts</p> | ||
| </div> | ||
| <div className="rounded-xl border border-border/70 bg-card/70 p-4 text-center shadow-sm"> | ||
| <p className="text-2xl font-bold text-tblack">{countriesCount}</p> | ||
| <p className="text-xs text-tgray">Countries</p> | ||
| </div> | ||
| <div className="rounded-xl border border-border/70 bg-card/70 p-4 text-center shadow-sm"> | ||
| <p className="text-2xl font-bold text-tblack">{activeFilters.length}</p> | ||
| <p className="text-xs text-tgray">Active filters</p> |
There was a problem hiding this comment.
New copy here bypasses the translation layer.
These labels and the empty-state text are hardcoded in English, while the rest of the page is localized via TransText/t. FR/AR users will get a mixed-language page.
Also applies to: 314-317
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@resources/js/pages/experts/index.jsx` around lines 239 - 250, The three
dashboard labels currently hardcoded ("Experts", "Countries", "Active filters")
and the empty-state text further down should be replaced with the localization
layer used elsewhere (e.g., the TransText component or t function) so the page
isn't mixed-language for FR/AR users; locate the JSX where expertsProp.length,
countriesCount, and activeFilters.length are rendered and replace their adjacent
literal label strings with <TransText> keys or t('...') calls consistent with
the rest of the file, and do the same for the empty-state text referenced around
the empty-state block (lines noted in the review) so all UI strings use the same
translation keys/components.
Comment on lines
117
to
+120
| Route::middleware(['auth', 'verified'])->group(function () { | ||
| Route::inertia('dashboard', 'dashboard')->name('dashboard'); | ||
| Route::get('dashboard', fn () => redirect()->route('admin.dashboard')) | ||
| ->middleware('role:admin') | ||
| ->name('dashboard'); |
There was a problem hiding this comment.
Keep the shared dashboard route role-aware.
This route now 403s for experts, so any generic redirect or stale /dashboard link breaks as soon as an expert hits it. The shared entry point should dispatch by role instead of being admin-only.
Suggested fix
Route::middleware(['auth', 'verified'])->group(function () {
- Route::get('dashboard', fn () => redirect()->route('admin.dashboard'))
- ->middleware('role:admin')
- ->name('dashboard');
+ Route::get('dashboard', function () {
+ $user = request()->user();
+
+ return match ($user?->role) {
+ 'admin' => redirect()->route('admin.dashboard'),
+ 'expert' => redirect()->route('expert.dashboard'),
+ default => abort(403),
+ };
+ })->name('dashboard');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@routes/web.php` around lines 117 - 120, The current shared
Route::get('dashboard', fn () => redirect()->route('admin.dashboard')) is
protected by ->middleware('role:admin') so experts 403; remove that role
middleware and replace the closure with a role-aware dispatch that checks the
authenticated user's role (e.g., auth()->user()->hasRole(...) or
isAdmin()/isExpert()) and redirects to the appropriate route (admin.dashboard,
expert.dashboard, etc.), keeping the route name('dashboard') and the
auth/verified middleware intact so stale /dashboard links route users by role
instead of forbidding them.
This was referenced May 6, 2026
This was referenced May 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a new workflow for managing expert applications, adds new controllers for expert dashboards and profile management, and updates mail configuration defaults. It also removes the ability for admins to manually create experts from the admin panel, ensuring that expert accounts are only created via the application review process.
Expert application management and onboarding:
ExpertApplicationControllerto allow admins to review, accept, or deny expert applications. Accepting an application creates or updates user and expert records, and sends an onboarding email.createandstoremethods fromExpertController.Expert user experience:
DashboardControllerfor experts, providing a dashboard view with basic expert information.ProfileControllerfor experts, allowing them to view and update their profile information, including contact details, expertise, and social links.Configuration updates:
.env.examplefromlogtosmtpand updated the default mail port to1025for better alignment with local SMTP testing tools.Summary by CodeRabbit
Release Notes
New Features
Changes