feat(integration-requests): add not-doable status with a public comment (#552)
## Summary Adds a `not_doable` status to integration requests for cases we've reviewed but can't implement (e.g. no public API). - **Applied with a comment** — the `integration-requests:review` command now offers a `not doable` choice that requires a comment explaining why. The comment is stored on the request and shown to users. - **Shown at the end** — not-doable requests stay visible to everyone but sink to the bottom of the board regardless of their votes, with the comment rendered below the name. - **Not votable** — voting is blocked both in the UI (disabled button) and the backend (404), even for the author. - **Rejected stays hidden** — rejected requests never appear in the list (unchanged behaviour, now covered by a test). ## Changes - `IntegrationRequestStatus` enum: new `NotDoable` case + label. - Migration: nullable `comment` text column on `integration_requests`. - `IntegrationRequestController`: list includes not-doable for everyone, ordered last; vote guard rejects not-doable. - `ReviewIntegrationRequestsCommand`: `not doable` option with a required comment prompt. - Board component: not-doable badge, comment, disabled voting. - Factory `notDoable()` state, `es.json` translation, feature tests. ## Testing - `php artisan test tests/Feature/IntegrationRequestTest.php` — 21 passed - `vendor/bin/pint --dirty`, `bun run format`, `bun run lint` — clean
This commit is contained in:
parent
7e9122eaf4
commit
801f6c7cd4
|
|
@ -46,11 +46,12 @@ class ReviewIntegrationRequestsCommand extends Command
|
|||
|
||||
$approved = 0;
|
||||
$rejected = 0;
|
||||
$notDoable = 0;
|
||||
|
||||
foreach ($pending as $request) {
|
||||
$decision = $this->choice(
|
||||
"Review \"{$request->name}\" ({$request->url})",
|
||||
['approve', 'reject', 'skip'],
|
||||
['approve', 'reject', 'not doable', 'skip'],
|
||||
'skip',
|
||||
);
|
||||
|
||||
|
|
@ -60,11 +61,23 @@ class ReviewIntegrationRequestsCommand extends Command
|
|||
} elseif ($decision === 'reject') {
|
||||
$request->update(['status' => IntegrationRequestStatus::Rejected]);
|
||||
$rejected++;
|
||||
} elseif ($decision === 'not doable') {
|
||||
$comment = $this->ask('Why is this integration not doable? (shown to users)');
|
||||
|
||||
while (blank($comment)) {
|
||||
$comment = $this->ask('A comment is required to mark an integration as not doable');
|
||||
}
|
||||
|
||||
$request->update([
|
||||
'status' => IntegrationRequestStatus::NotDoable,
|
||||
'comment' => $comment,
|
||||
]);
|
||||
$notDoable++;
|
||||
}
|
||||
}
|
||||
|
||||
$skipped = $pending->count() - $approved - $rejected;
|
||||
$this->info("Done. Approved: {$approved}, rejected: {$rejected}, skipped: {$skipped}.");
|
||||
$skipped = $pending->count() - $approved - $rejected - $notDoable;
|
||||
$this->info("Done. Approved: {$approved}, rejected: {$rejected}, not doable: {$notDoable}, skipped: {$skipped}.");
|
||||
|
||||
return self::SUCCESS;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ enum IntegrationRequestStatus: string
|
|||
case Pending = 'pending';
|
||||
case Approved = 'approved';
|
||||
case Rejected = 'rejected';
|
||||
case NotDoable = 'not_doable';
|
||||
|
||||
public function label(): string
|
||||
{
|
||||
|
|
@ -14,6 +15,7 @@ enum IntegrationRequestStatus: string
|
|||
self::Pending => 'Pending',
|
||||
self::Approved => 'Approved',
|
||||
self::Rejected => 'Rejected',
|
||||
self::NotDoable => 'Not doable',
|
||||
};
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -54,8 +54,11 @@ class IntegrationRequestController extends Controller
|
|||
{
|
||||
$user = $request->user();
|
||||
|
||||
if ($integrationRequest->status !== IntegrationRequestStatus::Approved
|
||||
&& $integrationRequest->user_id !== $user->id) {
|
||||
$canVote = $integrationRequest->status === IntegrationRequestStatus::Approved
|
||||
|| ($integrationRequest->status === IntegrationRequestStatus::Pending
|
||||
&& $integrationRequest->user_id === $user->id);
|
||||
|
||||
if (! $canVote) {
|
||||
abort(404);
|
||||
}
|
||||
|
||||
|
|
@ -85,7 +88,7 @@ class IntegrationRequestController extends Controller
|
|||
{
|
||||
return IntegrationRequest::query()
|
||||
->where(function ($query) use ($user) {
|
||||
$query->where('status', IntegrationRequestStatus::Approved)
|
||||
$query->whereIn('status', [IntegrationRequestStatus::Approved, IntegrationRequestStatus::NotDoable])
|
||||
->orWhere(function ($inner) use ($user) {
|
||||
$inner->where('status', IntegrationRequestStatus::Pending)
|
||||
->where('user_id', $user->id);
|
||||
|
|
@ -93,6 +96,8 @@ class IntegrationRequestController extends Controller
|
|||
})
|
||||
->withCount('votes')
|
||||
->withExists(['votes as has_voted' => fn ($query) => $query->where('user_id', $user->id)])
|
||||
// Not-doable requests sink to the bottom regardless of their votes.
|
||||
->orderByRaw('CASE WHEN status = ? THEN 1 ELSE 0 END', [IntegrationRequestStatus::NotDoable->value])
|
||||
->orderByDesc('votes_count')
|
||||
->orderByDesc('created_at')
|
||||
->get();
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ use Illuminate\Database\Eloquent\Relations\HasMany;
|
|||
* @property string $name
|
||||
* @property string $url
|
||||
* @property IntegrationRequestStatus $status
|
||||
* @property ?string $comment
|
||||
* @property string $user_id
|
||||
*/
|
||||
class IntegrationRequest extends Model
|
||||
|
|
@ -26,6 +27,7 @@ class IntegrationRequest extends Model
|
|||
'name',
|
||||
'url',
|
||||
'status',
|
||||
'comment',
|
||||
'user_id',
|
||||
];
|
||||
|
||||
|
|
|
|||
|
|
@ -34,4 +34,12 @@ class IntegrationRequestFactory extends Factory
|
|||
{
|
||||
return $this->state(['status' => IntegrationRequestStatus::Rejected]);
|
||||
}
|
||||
|
||||
public function notDoable(): static
|
||||
{
|
||||
return $this->state([
|
||||
'status' => IntegrationRequestStatus::NotDoable,
|
||||
'comment' => fake()->sentence(),
|
||||
]);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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('integration_requests', function (Blueprint $table) {
|
||||
$table->text('comment')->nullable()->after('status');
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Reverse the migrations.
|
||||
*/
|
||||
public function down(): void
|
||||
{
|
||||
Schema::table('integration_requests', function (Blueprint $table) {
|
||||
$table->dropColumn('comment');
|
||||
});
|
||||
}
|
||||
};
|
||||
|
|
@ -2,6 +2,7 @@
|
|||
"Link": "Enlace",
|
||||
"Submit": "Enviar",
|
||||
"Pending review": "Pendiente de revisión",
|
||||
"Not doable": "No viable",
|
||||
"Something went wrong.": "Algo salió mal.",
|
||||
"Integration requests": "Solicitudes de integración",
|
||||
"Request integration": "Solicitar integración",
|
||||
|
|
|
|||
|
|
@ -18,7 +18,8 @@ export interface IntegrationRequestItem {
|
|||
id: string;
|
||||
name: string;
|
||||
url: string;
|
||||
status: 'pending' | 'approved' | 'rejected';
|
||||
status: 'pending' | 'approved' | 'rejected' | 'not_doable';
|
||||
comment: string | null;
|
||||
votes_count: number;
|
||||
has_voted: boolean;
|
||||
created_at: string;
|
||||
|
|
@ -110,7 +111,11 @@ export function IntegrationRequestsBoard({
|
|||
};
|
||||
|
||||
const handleVote = async (item: IntegrationRequestItem) => {
|
||||
if (busy || (!item.has_voted && actionsRemaining <= 0)) {
|
||||
if (
|
||||
busy ||
|
||||
item.status === 'not_doable' ||
|
||||
(!item.has_voted && actionsRemaining <= 0)
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -219,37 +224,50 @@ export function IntegrationRequestsBoard({
|
|||
) : (
|
||||
<ul className="space-y-2">
|
||||
{requests.map((item) => (
|
||||
<li
|
||||
key={item.id}
|
||||
className="flex items-center justify-between gap-3 rounded-lg border p-3"
|
||||
>
|
||||
<div className="flex min-w-0 items-center gap-2">
|
||||
<a
|
||||
href={item.url}
|
||||
target="_blank"
|
||||
rel="noopener noreferrer"
|
||||
className="truncate font-medium hover:underline"
|
||||
<li key={item.id} className="rounded-lg border p-3">
|
||||
<div className="flex items-center justify-between gap-3">
|
||||
<div className="flex min-w-0 items-center gap-2">
|
||||
<a
|
||||
href={item.url}
|
||||
target="_blank"
|
||||
rel="noopener noreferrer"
|
||||
className="truncate font-medium hover:underline"
|
||||
>
|
||||
{item.name}
|
||||
</a>
|
||||
{item.status === 'pending' && (
|
||||
<Badge variant="secondary">
|
||||
{__('Pending review')}
|
||||
</Badge>
|
||||
)}
|
||||
{item.status === 'not_doable' && (
|
||||
<Badge variant="outline">
|
||||
{__('Not doable')}
|
||||
</Badge>
|
||||
)}
|
||||
</div>
|
||||
<Button
|
||||
variant={
|
||||
item.has_voted ? 'default' : 'outline'
|
||||
}
|
||||
size="sm"
|
||||
disabled={
|
||||
busy ||
|
||||
item.status === 'not_doable' ||
|
||||
(!item.has_voted && outOfActions)
|
||||
}
|
||||
onClick={() => handleVote(item)}
|
||||
aria-pressed={item.has_voted}
|
||||
>
|
||||
{item.name}
|
||||
</a>
|
||||
{item.status === 'pending' && (
|
||||
<Badge variant="secondary">
|
||||
{__('Pending review')}
|
||||
</Badge>
|
||||
)}
|
||||
<ChevronUp className="h-4 w-4" />
|
||||
{item.votes_count}
|
||||
</Button>
|
||||
</div>
|
||||
<Button
|
||||
variant={item.has_voted ? 'default' : 'outline'}
|
||||
size="sm"
|
||||
disabled={
|
||||
busy || (!item.has_voted && outOfActions)
|
||||
}
|
||||
onClick={() => handleVote(item)}
|
||||
aria-pressed={item.has_voted}
|
||||
>
|
||||
<ChevronUp className="h-4 w-4" />
|
||||
{item.votes_count}
|
||||
</Button>
|
||||
{item.comment && (
|
||||
<p className="mt-2 text-sm text-muted-foreground">
|
||||
{item.comment}
|
||||
</p>
|
||||
)}
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
|
|
|
|||
|
|
@ -185,7 +185,7 @@ test('the review command approves a pending request', function () {
|
|||
->expectsChoice(
|
||||
"Review \"{$request->name}\" ({$request->url})",
|
||||
'approve',
|
||||
['approve', 'reject', 'skip'],
|
||||
['approve', 'reject', 'not doable', 'skip'],
|
||||
)
|
||||
->assertSuccessful();
|
||||
|
||||
|
|
@ -199,7 +199,7 @@ test('the review command rejects a pending request', function () {
|
|||
->expectsChoice(
|
||||
"Review \"{$request->name}\" ({$request->url})",
|
||||
'reject',
|
||||
['approve', 'reject', 'skip'],
|
||||
['approve', 'reject', 'not doable', 'skip'],
|
||||
)
|
||||
->assertSuccessful();
|
||||
|
||||
|
|
@ -211,3 +211,69 @@ test('the review command reports when there is nothing to review', function () {
|
|||
->expectsOutput('No pending integration requests.')
|
||||
->assertSuccessful();
|
||||
});
|
||||
|
||||
test('not doable requests are visible to everyone with their comment', function () {
|
||||
IntegrationRequest::factory()->notDoable()->create([
|
||||
'name' => 'Impossible Bank',
|
||||
'comment' => 'They have no public API.',
|
||||
]);
|
||||
|
||||
$this->actingAs(User::factory()->create())
|
||||
->getJson('/integration-requests/data')
|
||||
->assertOk()
|
||||
->assertJsonCount(1, 'requests')
|
||||
->assertJsonPath('requests.0.status', 'not_doable')
|
||||
->assertJsonPath('requests.0.comment', 'They have no public API.');
|
||||
});
|
||||
|
||||
test('not doable requests are listed after the rest', function () {
|
||||
IntegrationRequest::factory()->notDoable()->create(['name' => 'Not doable']);
|
||||
$approved = IntegrationRequest::factory()->approved()->create(['name' => 'Approved']);
|
||||
|
||||
$this->actingAs(User::factory()->create())
|
||||
->getJson('/integration-requests/data')
|
||||
->assertOk()
|
||||
->assertJsonPath('requests.0.id', $approved->id)
|
||||
->assertJsonPath('requests.1.name', 'Not doable');
|
||||
});
|
||||
|
||||
test('rejected requests never appear in the list', function () {
|
||||
IntegrationRequest::factory()->rejected()->create();
|
||||
|
||||
$this->actingAs(User::factory()->create())
|
||||
->getJson('/integration-requests/data')
|
||||
->assertOk()
|
||||
->assertJsonCount(0, 'requests');
|
||||
});
|
||||
|
||||
test('nobody can vote on a not doable request', function () {
|
||||
$request = IntegrationRequest::factory()->notDoable()->create();
|
||||
|
||||
$this->actingAs(User::factory()->create())
|
||||
->postJson("/integration-requests/{$request->id}/vote")
|
||||
->assertNotFound();
|
||||
|
||||
$owner = User::factory()->create();
|
||||
$ownRequest = IntegrationRequest::factory()->notDoable()->create(['user_id' => $owner->id]);
|
||||
|
||||
$this->actingAs($owner)
|
||||
->postJson("/integration-requests/{$ownRequest->id}/vote")
|
||||
->assertNotFound();
|
||||
});
|
||||
|
||||
test('the review command marks a request as not doable with a comment', function () {
|
||||
$request = IntegrationRequest::factory()->create(['name' => 'Hard Bank']);
|
||||
|
||||
$this->artisan('integration-requests:review')
|
||||
->expectsChoice(
|
||||
"Review \"{$request->name}\" ({$request->url})",
|
||||
'not doable',
|
||||
['approve', 'reject', 'not doable', 'skip'],
|
||||
)
|
||||
->expectsQuestion('Why is this integration not doable? (shown to users)', 'No public API available.')
|
||||
->assertSuccessful();
|
||||
|
||||
$request->refresh();
|
||||
expect($request->status)->toBe(IntegrationRequestStatus::NotDoable);
|
||||
expect($request->comment)->toBe('No public API available.');
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue