Skip to content

Commit 3d209f8

Browse files
committed
refactor: drop free-text note from ping (violated no-free-text convention)
Every message type in this app forbids free text (MoodType "No free text", StatusType "not a notes field", etc.), so PingType's optional prose `note` field was inconsistent. Removed the field, the --note option, and the note-in-payload — the ping payload is now just {user_ids}. That eliminated the only user-controlled value that could make the envelope invalid, so the now-unreachable InvalidEnvelopeException try/catch and renderEnvelopeError helper were removed too (the envelope is valid by construction). Dropped the two note-max-length tests accordingly.
1 parent a2abed5 commit 3d209f8

5 files changed

Lines changed: 10 additions & 75 deletions

File tree

app/Console/Commands/PingThreadUsers.php

Lines changed: 4 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,11 @@
22

33
namespace App\Console\Commands;
44

5-
use App\Exceptions\InvalidEnvelopeException;
65
use App\Interfaces\MessageServiceInterface;
76
use App\Models\Thread;
87
use App\Models\User;
98
use Illuminate\Console\Command;
109
use Illuminate\Database\Eloquent\Collection;
11-
use Illuminate\Http\JsonResponse;
1210

1311
class PingThreadUsers extends Command
1412
{
@@ -19,8 +17,7 @@ class PingThreadUsers extends Command
1917
{--thread= : Existing thread UUID to ping into (omit to create a new thread)}
2018
{--from= : Sender / thread-creator user UUID}
2119
{--user=* : User UUID(s) to ping}
22-
{--subject= : Subject for a NEW thread (required when --thread is omitted)}
23-
{--note= : Optional short note included in the ping payload}';
20+
{--subject= : Subject for a NEW thread (required when --thread is omitted)}';
2421

2522
/**
2623
* @var string
@@ -34,7 +31,6 @@ public function handle(MessageServiceInterface $service): int
3431
$fromId = $this->option('from');
3532
/** @var array<int, string> $userIds */
3633
$userIds = array_values(array_unique($this->option('user')));
37-
$note = $this->option('note');
3834

3935
$threadId = is_string($threadId) && $threadId !== '' ? $threadId : null;
4036
$subject = is_string($subject) && $subject !== '' ? $subject : null;
@@ -83,21 +79,11 @@ public function handle(MessageServiceInterface $service): int
8379
$envelope = [
8480
'type' => 'ping',
8581
'version' => '1.0',
86-
'payload' => array_filter(
87-
['user_ids' => $userIds, 'note' => $note],
88-
fn ($value) => $value !== null && $value !== '',
89-
),
82+
'payload' => ['user_ids' => $userIds],
9083
];
9184

9285
if ($threadId === null) {
93-
try {
94-
$thread = $service->newThread((string) $subject, $sender, $envelope, $userIds);
95-
} catch (InvalidEnvelopeException $e) {
96-
$this->error('Could not send ping: '.$this->renderEnvelopeError($e));
97-
98-
return self::FAILURE;
99-
}
100-
86+
$thread = $service->newThread((string) $subject, $sender, $envelope, $userIds);
10187
$this->info("Created thread {$thread->id} and pinged {$users->count()} user(s).");
10288

10389
return self::SUCCESS;
@@ -128,25 +114,9 @@ public function handle(MessageServiceInterface $service): int
128114
return self::FAILURE;
129115
}
130116

131-
try {
132-
$message = $service->newMessage($thread, $sender, $envelope);
133-
} catch (InvalidEnvelopeException $e) {
134-
$this->error('Could not send ping: '.$this->renderEnvelopeError($e));
135-
136-
return self::FAILURE;
137-
}
138-
117+
$message = $service->newMessage($thread, $sender, $envelope);
139118
$this->info("Pinged {$users->count()} user(s) in thread {$thread->id} (message {$message->id}).");
140119

141120
return self::SUCCESS;
142121
}
143-
144-
private function renderEnvelopeError(InvalidEnvelopeException $e): string
145-
{
146-
$response = $e->getResponse();
147-
148-
return $response instanceof JsonResponse
149-
? (string) $response->getContent()
150-
: $e->getMessage();
151-
}
152122
}

app/MessageTypes/PingType.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public function version(): string
1818

1919
public function purpose(): string
2020
{
21-
return 'Nudge specific participants of a thread. The payload lists the pinged user IDs (a targeted mention) so clients may highlight the message for those users. An optional short note may accompany the ping.';
21+
return 'Nudge specific participants of a thread. The payload lists the pinged user IDs (a targeted mention) so clients may highlight the message for those users.';
2222
}
2323

2424
/**
@@ -37,7 +37,6 @@ public function schema(): array
3737
'uniqueItems' => true,
3838
'items' => ['type' => 'string'],
3939
],
40-
'note' => ['type' => 'string', 'maxLength' => 280],
4140
],
4241
];
4342
}

docs/plans/2026-07-14-thread-ping-command-design.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
**Status:** Approved 2026-07-14
44
**Source:** ops/admin need to nudge specific participants of a thread from the CLI — either inside an existing thread or by spinning up a new thread whose opening message is the ping.
55

6+
> **Amendment 2026-07-15:** The optional free-text `note` field (and the `--note` option) described below were **removed** before merge. Every message type in this app deliberately forbids free text (e.g. `MoodType` "No free text", `StatusType` "not a notes field"), so a prose `note` violated that convention. The ping payload is now just `{ user_ids: [...] }`. Because that removed the only user-controlled value that could produce an invalid envelope, the `InvalidEnvelopeException` try/catch in the command was also dropped (the envelope is now valid by construction).
7+
68
## 1. Goal
79

810
Add an artisan command, `thread:ping`, that an operator/admin runs to "ping" a set of users. It works in two modes:

tests/Feature/PingThreadUsersCommandTest.php

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ public function test_creates_a_new_thread_and_pings_recipients(): void
9090
'--from' => $sender->id,
9191
'--subject' => 'Heads up',
9292
'--user' => [$a->id, $b->id],
93-
'--note' => 'please respond',
9493
])->assertSuccessful();
9594

9695
$thread = Thread::where('subject', 'Heads up')->firstOrFail();
@@ -103,29 +102,10 @@ public function test_creates_a_new_thread_and_pings_recipients(): void
103102
$ping = $this->pingMessage($thread);
104103
$this->assertNotNull($ping);
105104
$this->assertSame([$a->id, $b->id], Arr::get($ping->body, 'payload.user_ids'));
106-
$this->assertSame('please respond', Arr::get($ping->body, 'payload.note'));
107105

108106
Notification::assertSentTo($a, ThreadCreated::class);
109107
}
110108

111-
public function test_fails_gracefully_when_note_exceeds_max_length(): void
112-
{
113-
Notification::fake();
114-
115-
$sender = User::factory()->create(['notify_via' => ['broadcast']]);
116-
$recipient = User::factory()->create(['notify_via' => ['broadcast']]);
117-
$thread = $this->service->newThread('Subject', $sender, $this->envelope(), [$recipient->id]);
118-
119-
$this->runPing([
120-
'--thread' => $thread->id,
121-
'--from' => $sender->id,
122-
'--user' => [$recipient->id],
123-
'--note' => str_repeat('a', 281),
124-
])->assertFailed();
125-
126-
$this->assertNull($this->pingMessage($thread));
127-
}
128-
129109
public function test_fails_when_neither_thread_nor_subject_is_given(): void
130110
{
131111
$sender = User::factory()->create();
@@ -248,22 +228,6 @@ public function test_fails_when_a_pinged_user_does_not_exist(): void
248228
$this->assertNoPingMessagesExist();
249229
}
250230

251-
public function test_create_mode_fails_gracefully_when_note_exceeds_max_length(): void
252-
{
253-
$sender = User::factory()->create();
254-
$recipient = User::factory()->create();
255-
256-
$this->runPing([
257-
'--from' => $sender->id,
258-
'--subject' => 'Heads up',
259-
'--user' => [$recipient->id],
260-
'--note' => str_repeat('a', 281),
261-
])->assertFailed();
262-
263-
$this->assertNoPingMessagesExist();
264-
$this->assertDatabaseMissing('threads', ['subject' => 'Heads up']);
265-
}
266-
267231
private function assertNoPingMessagesExist(): void
268232
{
269233
$this->assertFalse(

tests/Unit/MessageTypeTest.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,10 @@ public function test_ping(): void
131131
$this->assertNotEmpty($type->purpose());
132132

133133
$this->assertTrue($this->accepts($type->schema(), ['user_ids' => ['u1']]));
134-
$this->assertTrue($this->accepts($type->schema(), ['user_ids' => ['u1', 'u2'], 'note' => 'please respond']));
134+
$this->assertTrue($this->accepts($type->schema(), ['user_ids' => ['u1', 'u2']]));
135135
$this->assertFalse($this->accepts($type->schema(), ['user_ids' => []])); // minItems
136136
$this->assertFalse($this->accepts($type->schema(), ['user_ids' => ['u1', 'u1']])); // uniqueItems
137-
$this->assertFalse($this->accepts($type->schema(), ['note' => 'x'])); // missing user_ids
138-
$this->assertFalse($this->accepts($type->schema(), ['user_ids' => ['u1'], 'x' => 1])); // additionalProperties
137+
$this->assertFalse($this->accepts($type->schema(), [])); // missing user_ids
138+
$this->assertFalse($this->accepts($type->schema(), ['user_ids' => ['u1'], 'note' => 'x'])); // additionalProperties (note no longer allowed)
139139
}
140140
}

0 commit comments

Comments
 (0)