Skip to content

feat(admin): add in-app notifications#44

Merged
ibourgeois merged 5 commits into
mainfrom
codex/20-admin-notifications
Apr 10, 2026
Merged

feat(admin): add in-app notifications#44
ibourgeois merged 5 commits into
mainfrom
codex/20-admin-notifications

Conversation

@ibourgeois
Copy link
Copy Markdown
Contributor

Closes #20\n\nAdds in-app notifications with database delivery:\n- admin compose screen for all users, role targets, or selected users\n- user inbox with read/unread state\n- database notification storage and tests for delivery + inbox behavior

Copilot AI review requested due to automatic review settings April 10, 2026 11:21
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds database-backed in-app notifications (admin compose + user inbox with read/unread), but also introduces substantial additional functionality around idea drafts, AI-powered idea refinement, and promoting ideas into projects.

Changes:

  • Add admin UI + controller actions to send in-app notifications to all users / roles / selected users, and add inbox UI with read/unread toggles.
  • Introduce idea draft CRUD (controller, policy, views) plus “propose project” flow and new projects table/model/factory.
  • Add Laravel AI integration and an IdeaRefinementAgent + service, with feature tests around refinement.

Reviewed changes

Copilot reviewed 29 out of 30 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
tests/Feature/Notifications/InboxTest.php Covers inbox rendering + mark-read behavior and access control.
tests/Feature/Admin/NotificationsTest.php Covers admin notification delivery targeting modes + authorization.
routes/web.php Wires idea routes and notification routes (user + admin).
app/Http/Controllers/Admin/NotificationController.php Implements inbox rendering, admin compose + delivery, read/unread endpoints.
app/Notifications/InAppNotification.php Defines database notification payload (title/body/url).
database/migrations/2026_04_10_111853_create_notifications_table.php Adds database storage for notifications.
resources/views/notifications/index.blade.php User inbox UI with read/unread controls.
resources/views/admin/notifications/index.blade.php Admin compose UI + recent admin inbox list.
resources/views/admin/dashboard.blade.php Adds admin nav entry to notifications screen.
resources/views/components/ui/button.blade.php Adds support for rendering buttons as anchors (as="a").
resources/views/components/ui/textarea.blade.php Adds value support for textarea component (old input/edit forms).
app/Http/Controllers/IdeaController.php Adds idea draft CRUD + refine/propose actions.
app/Policies/IdeaPolicy.php Defines authorization rules for idea drafts.
app/Providers/AppServiceProvider.php Registers the IdeaPolicy with the Gate.
resources/views/ideas/index.blade.php Adds “New idea” CTA and edit link in list.
resources/views/ideas/create.blade.php New draft creation screen using shared form partial.
resources/views/ideas/edit.blade.php Draft edit screen + AI refine + propose project + delete actions.
resources/views/ideas/_form.blade.php Shared form partial for create/edit draft.
tests/Feature/Ideas/IdeasDraftCrudTest.php Exercises create/edit/update/delete and ownership enforcement.
tests/Feature/Ideas/IdeaProposalTest.php Exercises promoting a draft to a proposed project + authorization.
database/migrations/2026_04_10_110550_create_projects_table.php Adds projects table for proposed projects.
app/Models/Project.php New Project model + relationships + casts.
database/factories/ProjectFactory.php Factory for Project model.
app/Models/Idea.php Adds project() relationship.
config/ai.php Adds AI provider configuration (Ollama defaults).
composer.json Adds laravel/ai dependency.
composer.lock Locks new dependencies (laravel/ai, prism-php/prism, etc.).
app/Ai/Agents/Ideas/IdeaRefinementAgent.php Defines structured-output AI agent for idea refinement.
app/Services/Ideas/IdeaRefinementService.php Service to prompt the agent and return refinement fields.
tests/Feature/Ideas/IdeaRefinementTest.php Exercises refinement via faked agent and session display.
Comments suppressed due to low confidence (1)

composer.json:19

  • This PR is described as adding in-app notifications, but it also introduces Laravel AI (laravel/ai) and related idea-refinement features. If that’s intentional, the PR title/description (and linked issue) should be updated; otherwise, consider splitting AI/refinement into a separate PR to keep scope focused and reviewable.
    "require": {
        "php": "^8.3",
        "blade-ui-kit/blade-heroicons": "^2.7",
        "laravel/ai": "^0.5.0",
        "laravel/fortify": "^1.36",
        "laravel/framework": "^13.0",
        "laravel/tinker": "^3.0",
        "livewire/livewire": "^4.2",
        "silber/bouncer": "^1.0"

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +16 to +28
public function index(Request $request): View
{
if ($request->user()->isAn('admin')) {
return view('admin.notifications.index', [
'users' => User::query()->orderBy('first_name')->get(),
'notifications' => $request->user()->notifications()->latest()->limit(10)->get(),
]);
}

return view('notifications.index', [
'notifications' => $request->user()->notifications()->latest()->get(),
]);
}
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

index() returns the admin compose view whenever the current user is an admin, even on the non-admin /notifications route. That means in-app notifications sent with url: route('notifications.index') will take admin recipients to the admin compose screen (and not the inbox UI with read/unread actions), which is likely unintended. Consider splitting user inbox vs admin compose into separate controllers/routes (e.g., always render the inbox for /notifications, and only render the compose screen on /admin/notifications).

Copilot uses AI. Check for mistakes.
Comment on lines +39 to +40
'user_ids' => ['array'],
'user_ids.*' => ['integer', 'exists:users,id'],
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

For target = users, user_ids is not required, so submitting the form with no selected users silently sends to an empty recipient list. Add validation to require a non-empty user_ids array when targeting specific users (and consider rejecting/ignoring user_ids when target is not users).

Suggested change
'user_ids' => ['array'],
'user_ids.*' => ['integer', 'exists:users,id'],
'user_ids' => ['exclude_unless:target,users', 'required_if:target,users', 'array', 'min:1'],
'user_ids.*' => ['exclude_unless:target,users', 'integer', 'exists:users,id'],

Copilot uses AI. Check for mistakes.

$recipients = match ($validated['target']) {
'all' => User::query()->get(),
'role' => User::query()->whereIs($validated['role'])->get(),
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

The role targeting options/validation include user, but the codebase only appears to create/assign the admin role (no user role is assigned on registration). As written, choosing role user will likely send to zero recipients. Either create/assign a user role consistently, or change the targeting logic/UI to represent “non-admin users” explicitly.

Suggested change
'role' => User::query()->whereIs($validated['role'])->get(),
'role' => $validated['role'] === 'admin'
? User::query()->whereIs('admin')->get()
: User::query()->get()->reject(fn (User $user) => $user->isAn('admin')),

Copilot uses AI. Check for mistakes.
Comment on lines +50 to +59
'all' => User::query()->get(),
'role' => User::query()->whereIs($validated['role'])->get(),
'users' => User::query()->whereIn('id', $validated['user_ids'] ?? [])->get(),
};

Notification::send($recipients, $notification);

return back()->with('status', 'Notification sent.');
}

Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

Recipient selection uses User::query()->get() (and similar) which loads all matching users into memory before dispatching notifications. For large user counts this can become slow and memory-heavy; consider chunking the query and sending in batches and/or queueing the notification delivery.

Suggested change
'all' => User::query()->get(),
'role' => User::query()->whereIs($validated['role'])->get(),
'users' => User::query()->whereIn('id', $validated['user_ids'] ?? [])->get(),
};
Notification::send($recipients, $notification);
return back()->with('status', 'Notification sent.');
}
'all' => User::query(),
'role' => User::query()->whereIs($validated['role']),
'users' => User::query()->whereIn('id', $validated['user_ids'] ?? []),
};
$this->sendNotificationInChunks($recipients, $notification);
return back()->with('status', 'Notification sent.');
}
private function sendNotificationInChunks(\Illuminate\Database\Eloquent\Builder $recipients, InAppNotification $notification): void
{
$recipients->chunkById(500, function ($users) use ($notification): void {
Notification::send($users, $notification);
});
}

Copilot uses AI. Check for mistakes.
Comment on lines +71 to +79
public function markUnread(Request $request, DatabaseNotification $notification): RedirectResponse
{
if ($notification->notifiable_id !== $request->user()?->id) {
abort(403);
}

$notification->markAsUnread();

return back();
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

markUnread() is a new behavior but there is no test that actually hits the notifications.unread route and asserts read_at becomes null again. Adding a feature test for this route would help ensure unread toggling works end-to-end.

Copilot uses AI. Check for mistakes.
Comment thread routes/web.php
Comment on lines 3 to +19
use App\Http\Controllers\Admin\ImpersonationController;
use App\Http\Controllers\Admin\NotificationController;
use App\Http\Controllers\Admin\UserController;
use App\Models\Idea;
use App\Http\Controllers\IdeaController;
use Illuminate\Support\Facades\Route;

Route::get('/', function () {
return view('home');
})->name('home');

Route::middleware(['auth'])->group(function (): void {
Route::get('/ideas', function () {
return view('ideas.index', [
'ideas' => Idea::query()
->whereBelongsTo(auth()->user())
->orderByDesc('updated_at')
->get(),
]);
})->name('ideas.index');
Route::resource('ideas', IdeaController::class)->except(['show']);
Route::post('/ideas/{idea}/refine', [IdeaController::class, 'refine'])->name('ideas.refine');
Route::post('/ideas/{idea}/propose', [IdeaController::class, 'propose'])->name('ideas.propose');
Route::get('/notifications', [NotificationController::class, 'index'])->name('notifications.index');
Route::post('/notifications/{notification}/read', [NotificationController::class, 'markRead'])->name('notifications.read');
Route::post('/notifications/{notification}/unread', [NotificationController::class, 'markUnread'])->name('notifications.unread');
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

The non-admin notification inbox routes are wired to App\Http\Controllers\Admin\NotificationController, which is also responsible for the admin compose screen. This couples two separate concerns and forces role-based branching inside the controller. Consider using a dedicated (non-admin) controller for /notifications and reserving the Admin controller for /admin/notifications.

Copilot uses AI. Check for mistakes.
Comment on lines +36 to +46
$contextItems = [
[
'label' => 'Inbox',
'href' => route('ideas.index'),
'icon' => 'heroicon-o-inbox-stack',
],
[
'label' => 'Drafts',
'href' => route('ideas.index'),
'current' => true,
'icon' => 'heroicon-o-pencil-square',
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

The context navigation defines an “Inbox” item but links it to route('ideas.index'), which is the same target as “Drafts/Shared/Proposals” here. This looks like a copy/paste mistake and makes the context nav misleading; point “Inbox” at the intended inbox route or remove it until a distinct destination exists.

Copilot uses AI. Check for mistakes.
Comment on lines +36 to +46
$contextItems = [
[
'label' => 'Inbox',
'href' => route('ideas.index'),
'icon' => 'heroicon-o-inbox-stack',
],
[
'label' => 'Drafts',
'href' => route('ideas.index'),
'current' => true,
'icon' => 'heroicon-o-pencil-square',
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

The context navigation defines an “Inbox” item but links it to route('ideas.index'), which is the same target as “Drafts/Shared/Proposals” here. This looks like a copy/paste mistake and makes the context nav misleading; point “Inbox” at the intended inbox route or remove it until a distinct destination exists.

Copilot uses AI. Check for mistakes.
Comment on lines +100 to +112
public function propose(Idea $idea): RedirectResponse
{
Gate::authorize('update', $idea);

$project = Project::query()->create([
'idea_id' => $idea->id,
'user_id' => $idea->user_id,
'title' => $idea->title,
'summary' => $idea->summary,
'details' => $idea->details,
'status' => 'proposed',
'proposed_at' => now(),
]);
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

Idea is modeled with a hasOne(Project::class) relationship, but propose() always creates a new Project record without checking for an existing one. This can create multiple projects for a single idea while the model/API implies one; consider preventing duplicates (e.g., check for an existing project and reuse/redirect) or change the relationship to hasMany if multiples are valid.

Copilot uses AI. Check for mistakes.
Comment on lines +15 to +26
$table->id();
$table->foreignId('idea_id')->constrained()->cascadeOnDelete();
$table->foreignId('user_id')->constrained()->cascadeOnDelete();
$table->string('title');
$table->string('summary');
$table->text('details');
$table->string('status')->default('proposed');
$table->timestamp('proposed_at')->nullable();
$table->timestamps();

$table->index(['idea_id', 'status']);
});
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

The schema doesn’t enforce the implied one-to-one between ideas and projects (Idea has hasOne(Project)), so multiple projects can be created for the same idea. If the intent is one project per idea, add a unique constraint on idea_id (or otherwise enforce it) to keep the database consistent with the model.

Copilot uses AI. Check for mistakes.
@ibourgeois ibourgeois merged commit ffc62e3 into main Apr 10, 2026
1 check passed
@ibourgeois ibourgeois deleted the codex/20-admin-notifications branch April 10, 2026 11:30
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.

feat(admin-notifications): add in-app notification management

2 participants