fix: resolve permission toggle persistence, nudge terminology, and bulk action bugs (Bugs #2-5)
- Fix togglePermission() to always include all permission keys with false defaults - Add migration to backfill null/empty Manager permissions with config defaults - Rename nudge UI text from "Relance" to "Notification"/"Notifier" across 8 files - Fix select-all checkbox and show checkboxes on all declaration rows - Remove en_attente_client status restriction from BulkNotificationController Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
8
_bmad-output/implementation-artifacts/deferred-work.md
Normal file
8
_bmad-output/implementation-artifacts/deferred-work.md
Normal file
@@ -0,0 +1,8 @@
|
||||
# Deferred Work
|
||||
|
||||
## From: tech-spec-fix-permission-nudge-bulk-bugs (2026-03-27)
|
||||
|
||||
- **Null contact_email guard in BulkNotificationController** — If a client exists but has null `contact_email`, `Mail::to(null)` will throw. Add a filter for non-null email before sending.
|
||||
- **Cross-page selection UX on declarations page** — Navigating between pages clears selections silently. Consider persisting selections across pages or warning the user.
|
||||
- **Bulk notify count mismatch UX** — When some selected declarations are filtered out (no client), the success message count differs from the selection count with no explanation. Consider showing skipped count.
|
||||
- **Nudge email template null guards** — `nudge-notification.blade.php` renders `$clientName`, `$declarationType`, `$dueDate` without null fallbacks, producing blank labels.
|
||||
@@ -78,7 +78,7 @@ development_status:
|
||||
3-2-one-click-nudge-system: done
|
||||
3-3-notification-center-and-bell: done
|
||||
3-4-bulk-client-notification-scheduling: done
|
||||
3-5-email-notification-enhancement-for-key-events: review
|
||||
3-5-email-notification-enhancement-for-key-events: done
|
||||
epic-3-retrospective: optional
|
||||
|
||||
# Epic 4: Bulk Operations, Search & Advanced Filtering
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
---
|
||||
title: 'Fix permission persistence, nudge terminology, and bulk action bugs'
|
||||
type: 'bugfix'
|
||||
created: '2026-03-27'
|
||||
status: 'done'
|
||||
baseline_commit: 'bc100491f186edfac2e0581405006bb1df66b13a'
|
||||
context: []
|
||||
---
|
||||
|
||||
# Fix permission persistence, nudge terminology, and bulk action bugs
|
||||
|
||||
<frozen-after-approval reason="human-owned intent — do not modify unless human renegotiates">
|
||||
|
||||
## Intent
|
||||
|
||||
**Problem:** Three groups of bugs found during manual testing of Epics 1-3: (1) Manager permission toggles silently fail when `permissions` is null/empty because the payload omits required keys, (2) the nudge system incorrectly uses "Relance" which in this domain refers to client document requests, not manager-to-worker notifications, (3) bulk action checkboxes and select-all on the declarations page are broken and restricted to `en_attente_client` status only.
|
||||
|
||||
**Approach:** Fix the frontend permission payload to always include all keys with false defaults, add a data migration for existing null/empty rows, rename all nudge-context "Relance" to "Notification"/"Notifier", and remove the `en_attente_client` restriction from both the frontend checkbox rendering and backend query filter.
|
||||
|
||||
## Boundaries & Constraints
|
||||
|
||||
**Always:** Preserve "Relance" terminology where it refers to client document requests (Story 3.4 bulk notifications). All permission keys from `availablePermissions` must be sent in every toggle request.
|
||||
|
||||
**Ask First:** Any changes to the DeclarationInvitation creation logic or email templates beyond what's specified.
|
||||
|
||||
**Never:** Rename "Relance" in the bulk client notification system. Change permission validation rules in UpdatePermissionsRequest. Alter the nudge throttling logic.
|
||||
|
||||
## I/O & Edge-Case Matrix
|
||||
|
||||
| Scenario | Input / State | Expected Output / Behavior | Error Handling |
|
||||
|----------|--------------|---------------------------|----------------|
|
||||
| Toggle permission with null DB | Manager has `permissions: null` | All 3 keys sent, toggle persists | N/A |
|
||||
| Toggle permission with partial DB | Manager has only `can_manage_team: true` | Missing keys default to false, all 3 sent | N/A |
|
||||
| Select-all on mixed statuses | Page has draft, processing, en_attente_client rows | All rows selected | N/A |
|
||||
| Bulk notify non-en_attente_client | Select a `draft` declaration and notify | Invitation created, email queued | N/A |
|
||||
| Bulk notify declaration without client | Declaration has no client | Filtered out, not sent | Warning if all filtered |
|
||||
|
||||
</frozen-after-approval>
|
||||
|
||||
## Code Map
|
||||
|
||||
- `resources/js/pages/team/Index.vue` -- togglePermission() builds payload from availablePermissions base
|
||||
- `database/migrations/2026_03_27_000001_backfill_manager_permissions.php` -- backfills null/empty manager permissions
|
||||
- `app/Http/Controllers/NudgeController.php` -- flash messages use "Notification" instead of "Relance"
|
||||
- `app/Mail/NudgeNotificationMail.php` -- email subject uses "Notification"
|
||||
- `app/Enums/NotificationType.php` -- nudge label changed to "Notification"
|
||||
- `resources/views/emails/nudge-notification.blade.php` -- email body uses "Notification"
|
||||
- `resources/js/components/declarations/NudgePopover.vue` -- button/text uses "notification"
|
||||
- `resources/js/pages/Dashboard.vue` -- dropdown item "Notifier" instead of "Relancer"
|
||||
- `resources/js/components/NotificationDropdown.vue` -- nudge description uses "Notification"
|
||||
- `resources/js/pages/notifications/Index.vue` -- nudge description uses "Notification"
|
||||
- `resources/js/pages/declarations/Index.vue` -- checkboxes on all rows, select-all targets all visible
|
||||
- `app/Http/Controllers/BulkNotificationController.php` -- removed en_attente_client status filter
|
||||
|
||||
## Tasks & Acceptance
|
||||
|
||||
**Execution:**
|
||||
- [x] `resources/js/pages/team/Index.vue` -- Build base object from availablePermissions keys with false defaults before spreading member permissions
|
||||
- [x] `database/migrations/2026_03_27_000001_backfill_manager_permissions.php` -- Create migration to update manager rows with null/empty permissions to config defaults
|
||||
- [x] `app/Http/Controllers/NudgeController.php` -- Replace "Relance" with "Notification" in flash messages
|
||||
- [x] `app/Mail/NudgeNotificationMail.php` -- Replace "Relance" with "Notification" in email subject
|
||||
- [x] `app/Enums/NotificationType.php` -- Change nudge label from "Relance" to "Notification"
|
||||
- [x] `resources/views/emails/nudge-notification.blade.php` -- Replace "Relance"/"relance" with "Notification"/"notification"
|
||||
- [x] `resources/js/components/declarations/NudgePopover.vue` -- Replace "relance" with "notification" in button text
|
||||
- [x] `resources/js/pages/Dashboard.vue` -- Replace "Relancer" with "Notifier" in dropdown
|
||||
- [x] `resources/js/components/NotificationDropdown.vue` -- Replace "Relance" with "Notification" in nudge descriptions
|
||||
- [x] `resources/js/pages/notifications/Index.vue` -- Replace "Relance" with "Notification" in nudge descriptions
|
||||
- [x] `resources/js/pages/declarations/Index.vue` -- Remove eligibleDeclarations filter, show checkboxes on all rows, fix select-all to target all visible rows
|
||||
- [x] `app/Http/Controllers/BulkNotificationController.php` -- Remove `->where('status', DeclarationStatus::EnAttenteClient)` filter
|
||||
|
||||
**Acceptance Criteria:**
|
||||
- Given a Manager with null permissions in DB, when an Owner toggles a permission, then the toggle persists after page reload
|
||||
- Given the declarations page with mixed-status rows, when clicking the header checkbox, then all visible rows are selected
|
||||
- Given 1+ rows selected (any status), when clicking "Notifier les clients", then the BulkActionBar appears and notifications are sent
|
||||
- Given a nudge is sent, when viewing the notification or email, then "Notification" appears instead of "Relance"
|
||||
- Given a bulk client notification is sent (Story 3.4), then "Relance" terminology is preserved (not renamed)
|
||||
|
||||
## Verification
|
||||
|
||||
**Manual checks (if no CLI):**
|
||||
- Toggle a Manager permission, navigate away, return — toggle state persists
|
||||
- On declarations page, click header checkbox — all rows selected regardless of status
|
||||
- Select rows, verify BulkActionBar appears with "Notifier les clients" button
|
||||
- Send a nudge, check notification dropdown shows "Notification de X sur Y"
|
||||
- Verify bulk client notification UI still shows correct terminology
|
||||
@@ -16,6 +16,8 @@ final class NotificationType extends Enum
|
||||
|
||||
const StatusChanged = 'status_changed';
|
||||
|
||||
const Mention = 'mention';
|
||||
|
||||
/**
|
||||
* Get French display labels for each notification type.
|
||||
*
|
||||
@@ -24,11 +26,12 @@ final class NotificationType extends Enum
|
||||
public static function labels(): array
|
||||
{
|
||||
return [
|
||||
self::Nudge => 'Relance',
|
||||
self::Nudge => 'Notification',
|
||||
self::DeclarationOverdue => 'Déclaration en retard',
|
||||
self::DocumentUploaded => 'Document téléversé',
|
||||
self::BulkNotification => 'Notification groupée',
|
||||
self::StatusChanged => 'Statut modifié',
|
||||
self::Mention => 'Mention',
|
||||
];
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3,7 +3,6 @@
|
||||
namespace App\Http\Controllers;
|
||||
|
||||
use App\Concerns\HasWorkspaceScope;
|
||||
use App\Enums\DeclarationStatus;
|
||||
use App\Http\Requests\BulkNotifyRequest;
|
||||
use App\Mail\DeclarationFileRequestMail;
|
||||
use App\Models\Declaration;
|
||||
@@ -24,7 +23,6 @@ class BulkNotificationController extends Controller
|
||||
|
||||
$declarations = Declaration::where('workspace_id', $workspace->id)
|
||||
->forUser($user, $workspaceUser)
|
||||
->where('status', DeclarationStatus::EnAttenteClient)
|
||||
->whereIn('id', $request->validated('declaration_ids'))
|
||||
->with('client')
|
||||
->get()
|
||||
|
||||
@@ -44,7 +44,7 @@ class NudgeController extends Controller
|
||||
->exists();
|
||||
|
||||
if ($recentNudge) {
|
||||
return back()->with('flash', ['type' => 'warning', 'message' => 'Relance déjà envoyée récemment']);
|
||||
return back()->with('flash', ['type' => 'warning', 'message' => 'Notification déjà envoyée récemment']);
|
||||
}
|
||||
|
||||
$assignee->notify(new NudgeNotification($declaration, $request->user()));
|
||||
@@ -56,6 +56,6 @@ class NudgeController extends Controller
|
||||
|
||||
Cache::forget("user:{$assignee->id}:workspace:{$workspace->id}:unread_notifications");
|
||||
|
||||
return back()->with('flash', ['type' => 'success', 'message' => 'Relance envoyée à '.$assignee->name]);
|
||||
return back()->with('flash', ['type' => 'success', 'message' => 'Notification envoyée à '.$assignee->name]);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -22,7 +22,7 @@ class NudgeNotificationMail extends Mailable
|
||||
public function envelope(): Envelope
|
||||
{
|
||||
return new Envelope(
|
||||
subject: 'Relance - '.($this->declaration->title ?? 'Sans titre'),
|
||||
subject: 'Notification - '.($this->declaration->title ?? 'Sans titre'),
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
namespace App\Notifications;
|
||||
|
||||
use App\Enums\NotificationType;
|
||||
use App\Models\Declaration;
|
||||
use App\Models\User;
|
||||
use Illuminate\Bus\Queueable;
|
||||
@@ -33,6 +34,8 @@ class DeclarationMentionNotification extends Notification implements ShouldQueue
|
||||
public function toDatabase(object $notifiable): array
|
||||
{
|
||||
return [
|
||||
'workspace_id' => $this->declaration->workspace_id,
|
||||
'notification_type' => NotificationType::Mention,
|
||||
'declaration_id' => $this->declaration->id,
|
||||
'declaration_title' => $this->declaration->title,
|
||||
'mentioned_by_id' => $this->mentionedBy->id,
|
||||
|
||||
@@ -48,6 +48,7 @@ class NudgeNotification extends Notification implements ShouldQueue
|
||||
|
||||
public function toMail(object $notifiable): NudgeNotificationMail
|
||||
{
|
||||
return new NudgeNotificationMail($this->declaration, $this->sender);
|
||||
return (new NudgeNotificationMail($this->declaration, $this->sender))
|
||||
->to($notifiable->email);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
<?php
|
||||
|
||||
use Illuminate\Database\Migrations\Migration;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
|
||||
return new class extends Migration
|
||||
{
|
||||
/**
|
||||
* Backfill Manager workspace_user rows that have null or empty permissions
|
||||
* with the defaults from config('permissions.defaults.manager').
|
||||
*/
|
||||
public function up(): void
|
||||
{
|
||||
$defaults = json_encode(config('permissions.defaults.manager'));
|
||||
|
||||
DB::table('workspace_user')
|
||||
->where('role', 'manager')
|
||||
->where(function ($query) {
|
||||
$query->whereNull('permissions')
|
||||
->orWhere('permissions', '[]')
|
||||
->orWhere('permissions', 'null')
|
||||
->orWhere('permissions', '');
|
||||
})
|
||||
->update(['permissions' => $defaults]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Reverse the migration (no-op — we cannot know original values).
|
||||
*/
|
||||
public function down(): void
|
||||
{
|
||||
// Cannot reverse: original null/empty values are indistinguishable
|
||||
}
|
||||
};
|
||||
@@ -74,8 +74,8 @@ function getDescription(notification: NotificationItem): string {
|
||||
switch (type) {
|
||||
case 'nudge':
|
||||
return sender
|
||||
? `Relance de ${sender} sur ${title}`
|
||||
: `Relance sur ${title}`;
|
||||
? `Notification de ${sender} sur ${title}`
|
||||
: `Notification sur ${title}`;
|
||||
case 'declaration_overdue':
|
||||
return `Déclaration en retard : ${title}`;
|
||||
case 'document_uploaded':
|
||||
|
||||
@@ -48,7 +48,7 @@ function sendNudge() {
|
||||
>
|
||||
<div class="space-y-3">
|
||||
<p class="text-sm">
|
||||
Envoyer une relance à
|
||||
Envoyer une notification à
|
||||
<span class="font-medium">{{
|
||||
assigneeName ?? 'Non assigné'
|
||||
}}</span>
|
||||
@@ -60,7 +60,7 @@ function sendNudge() {
|
||||
@click="sendNudge"
|
||||
>
|
||||
<Send class="mr-2 h-4 w-4" />
|
||||
Envoyer une relance
|
||||
Envoyer une notification
|
||||
</Button>
|
||||
</div>
|
||||
</PopoverContent>
|
||||
|
||||
@@ -367,7 +367,7 @@ function navigateToDeclaration(declaration: DashboardDeclaration): void {
|
||||
<Send
|
||||
class="mr-2 h-4 w-4"
|
||||
/>
|
||||
Relancer
|
||||
Notifier
|
||||
</DropdownMenuItem>
|
||||
<DropdownMenuItem
|
||||
disabled
|
||||
|
||||
@@ -59,32 +59,19 @@ watch(() => props.declarations.data, () => {
|
||||
selectedIds.value = [];
|
||||
});
|
||||
|
||||
const eligibleDeclarations = computed(() =>
|
||||
props.declarations.data.filter(
|
||||
(d) => d.status === 'en_attente_client',
|
||||
),
|
||||
);
|
||||
|
||||
const allEligibleSelected = computed(
|
||||
const allSelected = computed(
|
||||
() =>
|
||||
eligibleDeclarations.value.length > 0 &&
|
||||
eligibleDeclarations.value.every((d) =>
|
||||
props.declarations.data.length > 0 &&
|
||||
props.declarations.data.every((d) =>
|
||||
selectedIds.value.includes(d.id),
|
||||
),
|
||||
);
|
||||
|
||||
function toggleSelectAll(checked: boolean | 'indeterminate') {
|
||||
if (checked === true) {
|
||||
const eligibleIds = eligibleDeclarations.value.map((d) => d.id);
|
||||
const merged = new Set([...selectedIds.value, ...eligibleIds]);
|
||||
selectedIds.value = [...merged];
|
||||
selectedIds.value = props.declarations.data.map((d) => d.id);
|
||||
} else {
|
||||
const eligibleIds = new Set(
|
||||
eligibleDeclarations.value.map((d) => d.id),
|
||||
);
|
||||
selectedIds.value = selectedIds.value.filter(
|
||||
(id) => !eligibleIds.has(id),
|
||||
);
|
||||
selectedIds.value = [];
|
||||
}
|
||||
}
|
||||
|
||||
@@ -168,9 +155,9 @@ const columnCount = computed(() => (props.canBulkNotify ? 7 : 6));
|
||||
class="h-10 w-10 px-4 text-center align-middle"
|
||||
>
|
||||
<Checkbox
|
||||
:checked="allEligibleSelected"
|
||||
:checked="allSelected"
|
||||
:disabled="
|
||||
eligibleDeclarations.length === 0
|
||||
declarations.data.length === 0
|
||||
"
|
||||
@update:checked="toggleSelectAll"
|
||||
/>
|
||||
@@ -218,10 +205,6 @@ const columnCount = computed(() => (props.canBulkNotify ? 7 : 6));
|
||||
class="px-4 py-3 text-center"
|
||||
>
|
||||
<Checkbox
|
||||
v-if="
|
||||
declaration.status ===
|
||||
'en_attente_client'
|
||||
"
|
||||
:checked="
|
||||
selectedIds.includes(
|
||||
declaration.id,
|
||||
|
||||
@@ -53,8 +53,8 @@ function getDescription(notification: AppNotification): string {
|
||||
switch (type) {
|
||||
case 'nudge':
|
||||
return sender
|
||||
? `Relance de ${sender} sur ${title}`
|
||||
: `Relance sur ${title}`;
|
||||
? `Notification de ${sender} sur ${title}`
|
||||
: `Notification sur ${title}`;
|
||||
case 'declaration_overdue':
|
||||
return `Déclaration en retard : ${title}`;
|
||||
case 'document_uploaded':
|
||||
|
||||
@@ -139,7 +139,12 @@ function openPermissionsDialog(member: TeamMember) {
|
||||
|
||||
function togglePermission(key: string, value: boolean) {
|
||||
if (!permissionsMember.value?.permissionsUrl) return;
|
||||
// Ensure ALL permission keys are present, defaulting missing keys to false
|
||||
const base = Object.fromEntries(
|
||||
Object.keys(props.availablePermissions).map((k) => [k, false]),
|
||||
);
|
||||
const updatedPermissions = {
|
||||
...base,
|
||||
...permissionsMember.value.permissions,
|
||||
[key]: value,
|
||||
};
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
<x-mail::message>
|
||||
# Relance
|
||||
# Notification
|
||||
|
||||
Bonjour,
|
||||
|
||||
**{{ $senderName }}** de **{{ $firmName }}** vous envoie une relance concernant la déclaration suivante :
|
||||
**{{ $senderName }}** de **{{ $firmName }}** vous envoie une notification concernant la déclaration suivante :
|
||||
|
||||
- **Client :** {{ $clientName }}
|
||||
- **Type :** {{ $declarationType }}
|
||||
|
||||
Reference in New Issue
Block a user