Week 079 — Spot the Bug

The partials have their own JavaScript now. And there was a bug this week that looked like a logic problem but was really about what hadn't been saved yet when the check ran.

Week of May 25, 2026

Progress

Monday was getting the item state partials to load at all. Wednesday was migrating the JavaScript out of item.js and into the files it actually belongs to. The item-state-lost partial does double duty — it handles both the safe and lost display states — so a bit of differentiation logic was needed inside the partial to avoid splitting it in two.

By Thursday, state transitions were working end to end. Clicking a segment button swaps in the correct partial, and state changes save cleanly. One small design decision made along the way: on a successful state save, the item page does a full reload. It's a bit heavier than a targeted DOM update, but it guarantees the UI matches exactly what was saved — no stale state, no edge cases hiding behind clever JavaScript.

The "found → lost" path also wired up this week. It's working at the UI level. One thing confirmed in testing: marking a found item as lost again creates a new lost instance rather than reverting the existing one. There's no reversion logic. That's fine for now, but worth watching if real users actually take that path.

The Hard Part

There was a bug where owners were getting notified that their own item had been found — when they were the one who found it. That should never happen. The function isFinderOwner() exists specifically to prevent it.

The function was working correctly. The check wasn't the problem.

Here's the original code — see if you can spot it:

// Notify owner if finder is not the owner.
if (!$this->isFinderOwner($passedItemLostInstanceDTO->getId())) {
    $notificationService->sendMessage(
        $passedUpdateItemCommand->getOwnerId() ?? $savedItemDTO->getOwnerId(),
        $passedUpdateItemCommand->getItemName() ?? $savedItemDTO->getItemName() . ' has been found.',
        NotificationDTO::SOURCE_TYPE_ITEM,
        $passedUpdateItemCommand->getId(),
    );
}

// Update item and lost instance records.
$this->saveItem($passedUpdateItemCommand);
$this->saveItemLostInstance($passedItemLostInstanceDTO->
    with(timestampFound: $dateTime->format('Y-m-d H:i:s')));

isFinderOwner() queries the database to check whether the finder ID on the lost instance belongs to the current user. But the notification check runs before the lost instance record is saved. The finder ID isn't in the database yet. The check finds nothing, assumes it's an outside finder, and fires the notification.

The fix was just swapping the order of the two blocks:

// Update item and lost instance records.
$this->saveItem($passedUpdateItemCommand);
$this->saveItemLostInstance($passedItemLostInstanceDTO->
    with(timestampFound: $dateTime->format('Y-m-d H:i:s')));

// Notify owner if finder is not the owner.
if (!$this->isFinderOwner($passedItemLostInstanceDTO->getId())) {
    $notificationService->sendMessage(
        $passedUpdateItemCommand->getOwnerId() ?? $savedItemDTO->getOwnerId(),
        $passedUpdateItemCommand->getItemName() ?? $savedItemDTO->getItemName() . ' has been found.',
        NotificationDTO::SOURCE_TYPE_ITEM,
        $passedUpdateItemCommand->getId(),
    );
}

Two blocks, swapped. These are the bugs that take time to find not because the logic is wrong, but because it looks right.

The Question That Appeared

This is the second time I've hit a sequencing bug — where a check fails not because of bad logic but because the state it depends on hasn't been written yet. I'm starting to think there's a pattern worth enforcing: any operation that queries state it just wrote should be positioned explicitly after the write. It's the kind of constraint that's hard to communicate in code without a comment, which I generally avoid. Still thinking about this one.

Also still open: should "found → lost" revert the existing lost instance, or is creating a new one fine? Probably needs intentional design before launch.

Next Week

Work through the remaining open state transition cases and take stock of where things stand against the MVP list.