feat(transactions): make new-transaction marker cross-device (#611)
## What The "new since last visit" highlight (#609) stored its marker in `localStorage` (`transactions-last-visit`), so it was **per-device**: each device kept its own last-visit timestamp and re-flagged the same rows. This moves the marker to a per-user server column so a single "last visit" is shared across all your devices. ## How - New nullable `transactions_last_visited_at` column on `users` (same pattern as `last_active_at` / `paywall_seen_at`). - `TransactionController@index` reads the stored marker, renders the list with the **old** value (`lastVisitAt` prop), then advances it forward to the newest `created_at` it served. - Frontend drops the `localStorage` read/write and just freezes the `lastVisitAt` prop at mount; `isNewSince` per-row logic is unchanged. ## Why newest-served `created_at` and not `now()` Advancing to `now()` would mark a back-dated synced row (old `transaction_date`, lands on a later page that wasn't served) as seen, so it would never be highlighted — the exact "hiding" failure the per-row design avoids. Advancing only to what was actually served keeps the feature's "err on showing, never hiding" stance. The marker only ever moves forward. ## Notes - Same filter caveat as before: opening the list with a filter applied advances the marker using the filtered payload. Carried over from the original `localStorage` behavior; not a regression. - Removed the now-dead `loadLastVisit` / `saveLastVisit` / `newestCreatedAt` helpers and their tests. ## Tests - `NewTransactionsMarkerTest` (feature): null marker on first visit + stores newest served; later visit sees previous marker + advances forward; marker never moves backward. - `new-transactions.test.ts` (`isNewSince`), Pint, ESLint, Prettier all pass.
This commit is contained in:
parent
884038c13b
commit
ee69c51a84
|
|
@ -30,6 +30,8 @@ class TransactionController extends Controller
|
|||
$user = $request->user();
|
||||
$validated = $request->validated();
|
||||
|
||||
$lastVisitAt = $user->transactions_last_visited_at;
|
||||
|
||||
$perPage = (int) ($validated['per_page'] ?? 50);
|
||||
$sortParam = $validated['sort'] ?? '-transaction_date';
|
||||
|
||||
|
|
@ -77,6 +79,11 @@ class TransactionController extends Controller
|
|||
->append('ai_categorized');
|
||||
});
|
||||
|
||||
$newestServed = $transactions->getCollection()->max('created_at');
|
||||
if ($newestServed && (! $lastVisitAt || $newestServed->gt($lastVisitAt))) {
|
||||
$user->forceFill(['transactions_last_visited_at' => $newestServed])->save();
|
||||
}
|
||||
|
||||
$appliedFilters = [
|
||||
'date_from' => $validated['date_from'] ?? null,
|
||||
'date_to' => $validated['date_to'] ?? null,
|
||||
|
|
@ -128,6 +135,7 @@ class TransactionController extends Controller
|
|||
'labels' => $labels,
|
||||
'automationRules' => $automationRules,
|
||||
'hasAiConsent' => $user->hasActiveAiConsent(),
|
||||
'lastVisitAt' => $lastVisitAt?->toISOString(),
|
||||
]);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -27,6 +27,7 @@ use Laravel\Pennant\Concerns\HasFeatures;
|
|||
/**
|
||||
* @property ?Carbon $last_logged_in_at
|
||||
* @property ?Carbon $last_active_at
|
||||
* @property ?Carbon $transactions_last_visited_at
|
||||
*/
|
||||
class User extends Authenticatable implements HasLocalePreference, MustVerifyEmail
|
||||
{
|
||||
|
|
@ -77,6 +78,7 @@ class User extends Authenticatable implements HasLocalePreference, MustVerifyEma
|
|||
'paywall_seen_at' => 'datetime',
|
||||
'last_logged_in_at' => 'datetime',
|
||||
'last_active_at' => 'datetime',
|
||||
'transactions_last_visited_at' => 'datetime',
|
||||
];
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,28 @@
|
|||
<?php
|
||||
|
||||
use Illuminate\Database\Migrations\Migration;
|
||||
use Illuminate\Database\Schema\Blueprint;
|
||||
use Illuminate\Support\Facades\Schema;
|
||||
|
||||
return new class extends Migration
|
||||
{
|
||||
/**
|
||||
* Run the migrations.
|
||||
*/
|
||||
public function up(): void
|
||||
{
|
||||
Schema::table('users', function (Blueprint $table) {
|
||||
$table->timestamp('transactions_last_visited_at')->nullable()->after('last_active_at');
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Reverse the migrations.
|
||||
*/
|
||||
public function down(): void
|
||||
{
|
||||
Schema::table('users', function (Blueprint $table) {
|
||||
$table->dropColumn('transactions_last_visited_at');
|
||||
});
|
||||
}
|
||||
};
|
||||
|
|
@ -1,6 +1,6 @@
|
|||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import { isNewSince, newestCreatedAt } from './new-transactions';
|
||||
import { isNewSince } from './new-transactions';
|
||||
|
||||
const tx = (created_at: string) => ({ created_at });
|
||||
|
||||
|
|
@ -34,31 +34,3 @@ describe('isNewSince', () => {
|
|||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('newestCreatedAt', () => {
|
||||
it('returns null for an empty list', () => {
|
||||
expect(newestCreatedAt([])).toBeNull();
|
||||
});
|
||||
|
||||
it('returns the only created_at for a single item', () => {
|
||||
expect(newestCreatedAt([tx('2026-06-29T10:00:00Z')])).toBe(
|
||||
'2026-06-29T10:00:00Z',
|
||||
);
|
||||
});
|
||||
|
||||
it('finds the latest regardless of input order', () => {
|
||||
expect(
|
||||
newestCreatedAt([
|
||||
tx('2026-06-20T08:00:00Z'),
|
||||
tx('2026-06-29T10:00:00Z'),
|
||||
tx('2026-06-25T09:00:00Z'),
|
||||
]),
|
||||
).toBe('2026-06-29T10:00:00Z');
|
||||
});
|
||||
|
||||
it('ignores unparseable timestamps', () => {
|
||||
expect(
|
||||
newestCreatedAt([tx('not-a-date'), tx('2026-06-20T08:00:00Z')]),
|
||||
).toBe('2026-06-20T08:00:00Z');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,50 +1,5 @@
|
|||
import { type Transaction } from '@/types/transaction';
|
||||
|
||||
const LAST_VISIT_KEY = 'transactions-last-visit';
|
||||
|
||||
export function loadLastVisit(): string | null {
|
||||
if (typeof window === 'undefined') return null;
|
||||
|
||||
try {
|
||||
return localStorage.getItem(LAST_VISIT_KEY);
|
||||
} catch (error) {
|
||||
console.error('Failed to load last visit:', error);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
export function saveLastVisit(value: string): void {
|
||||
if (typeof window === 'undefined') return;
|
||||
|
||||
try {
|
||||
localStorage.setItem(LAST_VISIT_KEY, value);
|
||||
} catch (error) {
|
||||
console.error('Failed to save last visit:', error);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The most recent `created_at` (insertion time) across the given transactions,
|
||||
* or null when there are none. Compared as timestamps so it does not rely on a
|
||||
* particular string format.
|
||||
*/
|
||||
export function newestCreatedAt(
|
||||
transactions: Pick<Transaction, 'created_at'>[],
|
||||
): string | null {
|
||||
let newest: string | null = null;
|
||||
let newestMs = -Infinity;
|
||||
|
||||
for (const transaction of transactions) {
|
||||
const ms = Date.parse(transaction.created_at);
|
||||
if (!Number.isNaN(ms) && ms > newestMs) {
|
||||
newest = transaction.created_at;
|
||||
newestMs = ms;
|
||||
}
|
||||
}
|
||||
|
||||
return newest;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a transaction was inserted after the given visit timestamp, i.e. it
|
||||
* arrived since the user last opened the list. Compared per-row (not as a
|
||||
|
|
|
|||
|
|
@ -83,12 +83,7 @@ import {
|
|||
type CursorPaginatedResponse,
|
||||
} from '@/lib/cursor-pagination';
|
||||
import { consoleDebug } from '@/lib/debug';
|
||||
import {
|
||||
isNewSince,
|
||||
loadLastVisit,
|
||||
newestCreatedAt,
|
||||
saveLastVisit,
|
||||
} from '@/lib/new-transactions';
|
||||
import { isNewSince } from '@/lib/new-transactions';
|
||||
import { captureEvent } from '@/lib/posthog';
|
||||
import { getBulkDeleteConfirmationText } from '@/lib/transaction-delete-confirmation';
|
||||
import { mergeReEvaluatedTransaction } from '@/lib/transaction-re-evaluation';
|
||||
|
|
@ -142,6 +137,7 @@ interface Props {
|
|||
labels: Label[];
|
||||
automationRules: AutomationRule[];
|
||||
hasAiConsent: boolean;
|
||||
lastVisitAt: string | null;
|
||||
}
|
||||
|
||||
const COLUMN_VISIBILITY_KEY = 'transactions-column-visibility';
|
||||
|
|
@ -433,6 +429,7 @@ export default function Transactions({
|
|||
labels: initialLabels,
|
||||
automationRules,
|
||||
hasAiConsent,
|
||||
lastVisitAt,
|
||||
}: Props) {
|
||||
const locale = useLocale();
|
||||
const { auth, features } = usePage<SharedData>().props;
|
||||
|
|
@ -490,19 +487,9 @@ export default function Transactions({
|
|||
);
|
||||
|
||||
// Frozen at mount so per-row "new" marks stay stable for the whole visit.
|
||||
const [lastVisitAtMount] = useState<string | null>(loadLastVisit);
|
||||
|
||||
// Mark this visit as seen, once, using the newest created_at from the
|
||||
// initial payload. Rows that arrive later in the same visit (load-more,
|
||||
// refresh after a sync, categorization) must NOT advance the marker, or
|
||||
// they'd be recorded as already-seen and never flagged on the next visit.
|
||||
useEffect(() => {
|
||||
const latest = newestCreatedAt(serverTransactions.data);
|
||||
if (latest) {
|
||||
saveLastVisit(latest);
|
||||
}
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, []);
|
||||
// The marker is server-side (per user), so it's shared across devices; the
|
||||
// controller advances it on load using the newest created_at it served.
|
||||
const [lastVisitAtMount] = useState<string | null>(() => lastVisitAt);
|
||||
|
||||
// Sync filter state when appliedFilters prop changes
|
||||
useEffect(() => {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,65 @@
|
|||
<?php
|
||||
|
||||
use App\Models\Account;
|
||||
use App\Models\Transaction;
|
||||
use App\Models\User;
|
||||
use Illuminate\Support\Carbon;
|
||||
|
||||
use function Pest\Laravel\actingAs;
|
||||
|
||||
beforeEach(function () {
|
||||
$this->user = User::factory()->onboarded()->create();
|
||||
$this->account = Account::factory()->create(['user_id' => $this->user->id]);
|
||||
});
|
||||
|
||||
test('first visit exposes a null marker and stores the newest served created_at', function () {
|
||||
$newest = Carbon::parse('2026-06-29T10:00:00Z');
|
||||
|
||||
Transaction::factory()->plaintext()->create([
|
||||
'user_id' => $this->user->id,
|
||||
'account_id' => $this->account->id,
|
||||
'created_at' => Carbon::parse('2026-06-20T08:00:00Z'),
|
||||
]);
|
||||
Transaction::factory()->plaintext()->create([
|
||||
'user_id' => $this->user->id,
|
||||
'account_id' => $this->account->id,
|
||||
'created_at' => $newest,
|
||||
]);
|
||||
|
||||
$response = actingAs($this->user)->get(route('transactions.index'));
|
||||
|
||||
$response->assertInertia(fn ($page) => $page->where('lastVisitAt', null));
|
||||
expect($this->user->fresh()->transactions_last_visited_at->equalTo($newest))->toBeTrue();
|
||||
});
|
||||
|
||||
test('a later visit sees the previous marker and advances it forward', function () {
|
||||
$previousVisit = Carbon::parse('2026-06-25T00:00:00Z');
|
||||
$this->user->forceFill(['transactions_last_visited_at' => $previousVisit])->save();
|
||||
|
||||
$newest = Carbon::parse('2026-06-29T10:00:00Z');
|
||||
Transaction::factory()->plaintext()->create([
|
||||
'user_id' => $this->user->id,
|
||||
'account_id' => $this->account->id,
|
||||
'created_at' => $newest,
|
||||
]);
|
||||
|
||||
$response = actingAs($this->user)->get(route('transactions.index'));
|
||||
|
||||
$response->assertInertia(fn ($page) => $page->where('lastVisitAt', $previousVisit->toISOString()));
|
||||
expect($this->user->fresh()->transactions_last_visited_at->equalTo($newest))->toBeTrue();
|
||||
});
|
||||
|
||||
test('the marker never moves backward when nothing newer was served', function () {
|
||||
$marker = Carbon::parse('2026-06-29T23:00:00Z');
|
||||
$this->user->forceFill(['transactions_last_visited_at' => $marker])->save();
|
||||
|
||||
Transaction::factory()->plaintext()->create([
|
||||
'user_id' => $this->user->id,
|
||||
'account_id' => $this->account->id,
|
||||
'created_at' => Carbon::parse('2026-06-20T08:00:00Z'),
|
||||
]);
|
||||
|
||||
actingAs($this->user)->get(route('transactions.index'))->assertSuccessful();
|
||||
|
||||
expect($this->user->fresh()->transactions_last_visited_at->equalTo($marker))->toBeTrue();
|
||||
});
|
||||
|
|
@ -57,7 +57,7 @@ test('account show page does not exceed query threshold', function () {
|
|||
});
|
||||
|
||||
test('transactions index page does not exceed query threshold', function () {
|
||||
assertMaxQueries(20, function () {
|
||||
assertMaxQueries(21, function () {
|
||||
$this->get(route('transactions.index'))->assertOk();
|
||||
}, 'Transactions Index');
|
||||
});
|
||||
|
|
@ -164,7 +164,7 @@ test('transactions page query count does not scale with number of transactions',
|
|||
]);
|
||||
|
||||
// Same threshold — paginated queries should not scale
|
||||
assertMaxQueries(20, function () {
|
||||
assertMaxQueries(21, function () {
|
||||
$this->get(route('transactions.index'))->assertOk();
|
||||
}, 'Transactions with 120 records');
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue