feat(open-banking): enable manage bank accounts for everyone (#572)
## Summary Removes the `ManageBankAccounts` Pennant feature flag that gated the per-connection "Manage Accounts" surface to the admin user, making the flow available to all users. ## Changes - Delete the `App\Features\ManageBankAccounts` feature class. - `ConnectionAccountController`: drop the `ensureFeatureEnabled()` gate (3 call sites + method) and the `Feature`/`ManageBankAccounts` imports. Access stays guarded by `authorizeConnection()` (connection ownership). - `HandleInertiaRequests`: stop sharing the `manageBankAccounts` Inertia flag. - Frontend: remove the flag from the `Features` type, the `features.manageBankAccounts` conditional in `connections.tsx` (now gated solely by `canManageAccounts`), and the now-unused `features` destructure. - Tests: update `InertiaSharedDataTest` and the `connections.test.tsx` mock; simplify the `adminUser()` helper to `onboardedUser()` in `ConnectionAccountTest` and remove the "forbidden when feature disabled" test. ## Testing - `vendor/bin/pint --dirty` — pass - `php artisan test tests/Feature/OpenBanking/ConnectionAccountTest.php tests/Feature/InertiaSharedDataTest.php` — 17 passed - `bunx vitest run resources/js/pages/settings/connections.test.tsx` — pass - `bun run lint` / `bun run format` — clean
This commit is contained in:
parent
83a5e9657e
commit
0f3cdd41aa
|
|
@ -1,23 +0,0 @@
|
|||
<?php
|
||||
|
||||
namespace App\Features;
|
||||
|
||||
use App\Models\User;
|
||||
|
||||
/**
|
||||
* Gates the per-connection "Manage Accounts" surface while it is being rolled
|
||||
* out. For now it is limited to the admin user (ADMIN_EMAIL) so the flow can be
|
||||
* dogfooded before a wider release.
|
||||
*
|
||||
* @api
|
||||
*/
|
||||
class ManageBankAccounts
|
||||
{
|
||||
/**
|
||||
* Resolve the feature's initial value.
|
||||
*/
|
||||
public function resolve(?User $user): bool
|
||||
{
|
||||
return $user?->isAdmin() ?? false;
|
||||
}
|
||||
}
|
||||
|
|
@ -3,7 +3,6 @@
|
|||
namespace App\Http\Controllers\OpenBanking;
|
||||
|
||||
use App\Contracts\BankingProviderInterface;
|
||||
use App\Features\ManageBankAccounts;
|
||||
use App\Http\Controllers\Controller;
|
||||
use App\Http\Requests\OpenBanking\MapConnectionAccountRequest;
|
||||
use App\Jobs\SyncBankingConnectionJob;
|
||||
|
|
@ -16,7 +15,6 @@ use Illuminate\Http\Request;
|
|||
use Illuminate\Support\Facades\Log;
|
||||
use Inertia\Inertia;
|
||||
use Inertia\Response;
|
||||
use Laravel\Pennant\Feature;
|
||||
|
||||
class ConnectionAccountController extends Controller
|
||||
{
|
||||
|
|
@ -24,7 +22,6 @@ class ConnectionAccountController extends Controller
|
|||
|
||||
public function index(Request $request, BankingConnection $connection): Response
|
||||
{
|
||||
$this->ensureFeatureEnabled();
|
||||
$this->authorizeConnection($connection);
|
||||
|
||||
$user = $request->user();
|
||||
|
|
@ -44,8 +41,6 @@ class ConnectionAccountController extends Controller
|
|||
|
||||
public function map(MapConnectionAccountRequest $request, BankingConnection $connection, AccountUserCurrencyService $accountUserCurrencyService): RedirectResponse
|
||||
{
|
||||
$this->ensureFeatureEnabled();
|
||||
|
||||
$validated = $request->validated();
|
||||
$uid = $validated['bank_account_uid'];
|
||||
|
||||
|
|
@ -109,7 +104,6 @@ class ConnectionAccountController extends Controller
|
|||
|
||||
public function unlink(BankingConnection $connection, Account $account): RedirectResponse
|
||||
{
|
||||
$this->ensureFeatureEnabled();
|
||||
$this->authorizeConnection($connection);
|
||||
|
||||
if ($account->banking_connection_id !== $connection->id) {
|
||||
|
|
@ -124,11 +118,6 @@ class ConnectionAccountController extends Controller
|
|||
return back()->with('success', __('Account is no longer syncing. It is now a manual account.'));
|
||||
}
|
||||
|
||||
private function ensureFeatureEnabled(): void
|
||||
{
|
||||
abort_unless(Feature::for(auth()->user())->active(ManageBankAccounts::class), 403);
|
||||
}
|
||||
|
||||
private function authorizeConnection(BankingConnection $connection): void
|
||||
{
|
||||
if ($connection->user_id !== auth()->id()) {
|
||||
|
|
|
|||
|
|
@ -5,7 +5,6 @@ namespace App\Http\Middleware;
|
|||
use App\Enums\BankingConnectionStatus;
|
||||
use App\Enums\BankingProvider;
|
||||
use App\Features\CalculateBalancesOnImport;
|
||||
use App\Features\ManageBankAccounts;
|
||||
use App\Features\TransactionAnalysis;
|
||||
use App\Models\BankingConnection;
|
||||
use App\Services\CurrencyOptions;
|
||||
|
|
@ -180,21 +179,18 @@ class HandleInertiaRequests extends Middleware
|
|||
'cashflow' => true,
|
||||
'calculateBalancesOnImport' => false,
|
||||
'transactionAnalysis' => false,
|
||||
'manageBankAccounts' => false,
|
||||
];
|
||||
}
|
||||
|
||||
$features = Feature::for($user)->values([
|
||||
CalculateBalancesOnImport::class,
|
||||
TransactionAnalysis::class,
|
||||
ManageBankAccounts::class,
|
||||
]);
|
||||
|
||||
return [
|
||||
'cashflow' => true,
|
||||
'calculateBalancesOnImport' => $features[CalculateBalancesOnImport::class] !== false,
|
||||
'transactionAnalysis' => $features[TransactionAnalysis::class] !== false,
|
||||
'manageBankAccounts' => $features[ManageBankAccounts::class] !== false,
|
||||
];
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -29,7 +29,6 @@ vi.mock('@inertiajs/react', () => ({
|
|||
cashflow: true,
|
||||
calculateBalancesOnImport: false,
|
||||
transactionAnalysis: false,
|
||||
manageBankAccounts: true,
|
||||
},
|
||||
},
|
||||
}),
|
||||
|
|
|
|||
|
|
@ -44,8 +44,7 @@ interface Props {
|
|||
}
|
||||
|
||||
export default function ConnectionsPage({ connections }: Props) {
|
||||
const { auth, flash, subscriptionsEnabled, features } =
|
||||
usePage<SharedData>().props;
|
||||
const { auth, flash, subscriptionsEnabled } = usePage<SharedData>().props;
|
||||
const isDemoAccount = auth?.isDemoAccount ?? false;
|
||||
const isFreePlan = subscriptionsEnabled && !auth?.hasProPlan;
|
||||
const [connectDialogOpen, setConnectDialogOpen] = useState(false);
|
||||
|
|
@ -287,23 +286,22 @@ export default function ConnectionsPage({ connections }: Props) {
|
|||
{__('Map Accounts')}
|
||||
</DropdownMenuItem>
|
||||
)}
|
||||
{features.manageBankAccounts &&
|
||||
canManageAccounts(
|
||||
connection,
|
||||
) && (
|
||||
<DropdownMenuItem
|
||||
onClick={() =>
|
||||
router.visit(
|
||||
`/open-banking/connections/${connection.id}/accounts`,
|
||||
)
|
||||
}
|
||||
>
|
||||
<Wallet className="mr-2 h-4 w-4" />
|
||||
{__(
|
||||
'Manage Accounts',
|
||||
)}
|
||||
</DropdownMenuItem>
|
||||
)}
|
||||
{canManageAccounts(
|
||||
connection,
|
||||
) && (
|
||||
<DropdownMenuItem
|
||||
onClick={() =>
|
||||
router.visit(
|
||||
`/open-banking/connections/${connection.id}/accounts`,
|
||||
)
|
||||
}
|
||||
>
|
||||
<Wallet className="mr-2 h-4 w-4" />
|
||||
{__(
|
||||
'Manage Accounts',
|
||||
)}
|
||||
</DropdownMenuItem>
|
||||
)}
|
||||
{hasAuthError(
|
||||
connection,
|
||||
) && (
|
||||
|
|
|
|||
|
|
@ -43,7 +43,6 @@ export interface Features {
|
|||
cashflow: boolean;
|
||||
calculateBalancesOnImport: boolean;
|
||||
transactionAnalysis: boolean;
|
||||
manageBankAccounts: boolean;
|
||||
}
|
||||
|
||||
export interface ExpiredBankingConnectionNotification {
|
||||
|
|
|
|||
|
|
@ -44,7 +44,6 @@ test('shared feature flags do not include coinbase flag', function () {
|
|||
'cashflow' => true,
|
||||
'calculateBalancesOnImport' => false,
|
||||
'transactionAnalysis' => false,
|
||||
'manageBankAccounts' => false,
|
||||
]);
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -17,21 +17,13 @@ beforeEach(function () {
|
|||
]);
|
||||
});
|
||||
|
||||
/**
|
||||
* The manage-accounts surface is gated behind the ManageBankAccounts feature,
|
||||
* which is only active for the admin user. Mark the created user as the admin so
|
||||
* the gate lets the request through.
|
||||
*/
|
||||
function adminUser(): User
|
||||
function onboardedUser(): User
|
||||
{
|
||||
$user = User::factory()->onboarded()->create();
|
||||
config(['mail.admin_email' => $user->email]);
|
||||
|
||||
return $user;
|
||||
return User::factory()->onboarded()->create();
|
||||
}
|
||||
|
||||
test('index renders the manage page with synced and available accounts', function () {
|
||||
$user = adminUser();
|
||||
$user = onboardedUser();
|
||||
$connection = BankingConnection::factory()->create(['user_id' => $user->id]);
|
||||
|
||||
Account::factory()->for($user)->create([
|
||||
|
|
@ -56,7 +48,7 @@ test('index renders the manage page with synced and available accounts', functio
|
|||
});
|
||||
|
||||
test('refresh discovers bank accounts that are not yet synced', function () {
|
||||
$user = adminUser();
|
||||
$user = onboardedUser();
|
||||
$connection = BankingConnection::factory()->create(['user_id' => $user->id]);
|
||||
|
||||
Account::factory()->for($user)->create([
|
||||
|
|
@ -94,7 +86,7 @@ test('refresh discovers bank accounts that are not yet synced', function () {
|
|||
test('map create adds a new synced account and dispatches a sync', function () {
|
||||
Queue::fake();
|
||||
|
||||
$user = adminUser();
|
||||
$user = onboardedUser();
|
||||
$connection = BankingConnection::factory()->create([
|
||||
'user_id' => $user->id,
|
||||
'aspsp_name' => 'Test Bank',
|
||||
|
|
@ -125,7 +117,7 @@ test('map create adds a new synced account and dispatches a sync', function () {
|
|||
test('map link moves syncing to another account and unlinks the previous one', function () {
|
||||
Queue::fake();
|
||||
|
||||
$user = adminUser();
|
||||
$user = onboardedUser();
|
||||
$connection = BankingConnection::factory()->create(['user_id' => $user->id]);
|
||||
|
||||
$source = Account::factory()->for($user)->create([
|
||||
|
|
@ -160,7 +152,7 @@ test('map link moves syncing to another account and unlinks the previous one', f
|
|||
});
|
||||
|
||||
test('unlink stops syncing but keeps the account and its transactions', function () {
|
||||
$user = adminUser();
|
||||
$user = onboardedUser();
|
||||
$connection = BankingConnection::factory()->create(['user_id' => $user->id]);
|
||||
|
||||
$account = Account::factory()->for($user)->create([
|
||||
|
|
@ -182,7 +174,7 @@ test('unlink stops syncing but keeps the account and its transactions', function
|
|||
});
|
||||
|
||||
test('index is forbidden for another user\'s connection', function () {
|
||||
$user = adminUser();
|
||||
$user = onboardedUser();
|
||||
$connection = BankingConnection::factory()->create([
|
||||
'user_id' => User::factory()->onboarded()->create()->id,
|
||||
]);
|
||||
|
|
@ -193,7 +185,7 @@ test('index is forbidden for another user\'s connection', function () {
|
|||
});
|
||||
|
||||
test('unlink rejects an account that does not belong to the connection', function () {
|
||||
$user = adminUser();
|
||||
$user = onboardedUser();
|
||||
$connection = BankingConnection::factory()->create(['user_id' => $user->id]);
|
||||
$account = Account::factory()->for($user)->create([
|
||||
'banking_connection_id' => null,
|
||||
|
|
@ -204,18 +196,8 @@ test('unlink rejects an account that does not belong to the connection', functio
|
|||
->assertNotFound();
|
||||
});
|
||||
|
||||
test('manage accounts is forbidden when the feature is disabled', function () {
|
||||
$user = User::factory()->onboarded()->create();
|
||||
config(['mail.admin_email' => 'someone-else@example.com']);
|
||||
$connection = BankingConnection::factory()->create(['user_id' => $user->id]);
|
||||
|
||||
$this->actingAs($user)
|
||||
->get(route('open-banking.connection-accounts.index', $connection))
|
||||
->assertForbidden();
|
||||
});
|
||||
|
||||
test('map link rejects a non-transactional target account', function () {
|
||||
$user = adminUser();
|
||||
$user = onboardedUser();
|
||||
$connection = BankingConnection::factory()->create(['user_id' => $user->id]);
|
||||
$loan = Account::factory()->for($user)->create([
|
||||
'banking_connection_id' => null,
|
||||
|
|
|
|||
Loading…
Reference in New Issue