Skip to content

Commit

Permalink
Refactor integration validation rules to include form context
Browse files Browse the repository at this point in the history
- Updated the `getValidationRules` method in various integration handlers (Discord, Email, Google Sheets, Slack, Webhook, Zapier) to accept an optional `Form` parameter, allowing for context-aware validation.
- Enhanced the `EmailIntegration` handler to enforce restrictions based on user plans, ensuring free users can only create one email integration per form and can only send to a single email address.
- Added a new test suite for `EmailIntegration` to validate the new restrictions and ensure proper functionality for both free and pro users.
- Introduced loading state management in the `IntegrationModal` component to improve user experience during save operations.

These changes improve the flexibility and user experience of form integrations, particularly for email handling.
  • Loading branch information
chiragchhatrala committed Dec 10, 2024
1 parent c63571e commit 01f7fa2
Show file tree
Hide file tree
Showing 10 changed files with 230 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace App\Http\Requests\Integration;

use App\Models\Forms\Form;
use App\Models\Integration\FormIntegration;
use App\Rules\IntegrationLogicRule;
use Illuminate\Foundation\Http\FormRequest;
Expand All @@ -14,9 +15,11 @@ class FormIntegrationsRequest extends FormRequest
public array $integrationRules = [];

private ?string $integrationClassName = null;
private ?Form $form = null;

public function __construct(Request $request)
{
$this->form = Form::findOrFail(request()->route('id'));
if ($request->integration_id) {
// Load integration class, and get rules
$integration = FormIntegration::getIntegration($request->integration_id);
Expand Down Expand Up @@ -77,7 +80,7 @@ protected function isOAuthRequired(): bool

private function loadIntegrationRules()
{
foreach ($this->integrationClassName::getValidationRules() as $key => $value) {
foreach ($this->integrationClassName::getValidationRules($this->form) as $key => $value) {
$this->integrationRules['settings.' . $key] = $value;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public function handle(): void
Http::throw()->post($this->getWebhookUrl(), $this->getWebhookData());
}

abstract public static function getValidationRules(): array;
abstract public static function getValidationRules(?Form $form): array;

public static function isOAuthRequired(): bool
{
Expand Down
3 changes: 2 additions & 1 deletion api/app/Integrations/Handlers/DiscordIntegration.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@

namespace App\Integrations\Handlers;

use App\Models\Forms\Form;
use App\Open\MentionParser;
use App\Service\Forms\FormSubmissionFormatter;
use Illuminate\Support\Arr;
use Vinkla\Hashids\Facades\Hashids;

class DiscordIntegration extends AbstractIntegrationHandler
{
public static function getValidationRules(): array
public static function getValidationRules(?Form $form): array
{
return [
'discord_webhook_url' => 'required|url|starts_with:https://discord.com/api/webhooks',
Expand Down
34 changes: 31 additions & 3 deletions api/app/Integrations/Handlers/EmailIntegration.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,23 @@

namespace App\Integrations\Handlers;

use App\Models\Forms\Form;
use App\Models\Integration\FormIntegration;
use App\Notifications\Forms\FormEmailNotification;
use Illuminate\Support\Facades\Log;
use Illuminate\Support\Facades\Notification;
use App\Open\MentionParser;
use App\Service\Forms\FormSubmissionFormatter;
use Illuminate\Validation\ValidationException;

class EmailIntegration extends AbstractEmailIntegrationHandler
{
public const RISKY_USERS_LIMIT = 120;

public static function getValidationRules(): array
public static function getValidationRules(?Form $form): array
{
return [
'send_to' => 'required',
$rules = [
'send_to' => ['required'],
'sender_name' => 'required',
'sender_email' => 'email|nullable',
'subject' => 'required',
Expand All @@ -24,6 +27,31 @@ public static function getValidationRules(): array
'include_hidden_fields_submission_data' => ['nullable', 'boolean'],
'reply_to' => 'nullable',
];

if ($form->is_pro) {
return $rules;
}

// Free plan users can only send to a single email address (avoid spam)
$rules['send_to'][] = function ($attribute, $value, $fail) use ($form) {
if (count(explode("\n", trim($value))) > 1 || count(explode(',', $value)) > 1) {
$fail('You can only send to a single email address on the free plan. Please upgrade to the Pro plan to create a new integration.');
}
};

// Free plan users can only have a single email integration per form (avoid spam)
if (!request()->route('integrationid')) {
$existingEmailIntegrations = FormIntegration::where('form_id', $form->id)
->where('integration_id', 'email')
->count();
if ($existingEmailIntegrations > 0) {
throw ValidationException::withMessages([
'settings.send_to' => ['Free users are limited to 1 email integration per form.']
]);
}
}

return $rules;
}

protected function shouldRun(): bool
Expand Down
7 changes: 3 additions & 4 deletions api/app/Integrations/Handlers/GoogleSheetsIntegration.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use App\Events\Forms\FormSubmitted;
use App\Integrations\Google\Google;
use App\Models\Forms\Form;
use App\Models\Integration\FormIntegration;
use Exception;
use Illuminate\Support\Facades\Log;
Expand All @@ -22,11 +23,9 @@ public function __construct(
$this->client = new Google($formIntegration);
}

public static function getValidationRules(): array
public static function getValidationRules(?Form $form): array
{
return [

];
return [];
}

public static function isOAuthRequired(): bool
Expand Down
3 changes: 2 additions & 1 deletion api/app/Integrations/Handlers/SlackIntegration.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@

namespace App\Integrations\Handlers;

use App\Models\Forms\Form;
use App\Open\MentionParser;
use App\Service\Forms\FormSubmissionFormatter;
use Illuminate\Support\Arr;
use Vinkla\Hashids\Facades\Hashids;

class SlackIntegration extends AbstractIntegrationHandler
{
public static function getValidationRules(): array
public static function getValidationRules(?Form $form): array
{
return [
'slack_webhook_url' => 'required|url|starts_with:https://hooks.slack.com/',
Expand Down
4 changes: 3 additions & 1 deletion api/app/Integrations/Handlers/WebhookIntegration.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@

namespace App\Integrations\Handlers;

use App\Models\Forms\Form;

class WebhookIntegration extends AbstractIntegrationHandler
{
public static function getValidationRules(): array
public static function getValidationRules(?Form $form): array
{
return [
'webhook_url' => 'required|url'
Expand Down
3 changes: 2 additions & 1 deletion api/app/Integrations/Handlers/ZapierIntegration.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace App\Integrations\Handlers;

use App\Events\Forms\FormSubmitted;
use App\Models\Forms\Form;
use App\Models\Integration\FormIntegration;
use Exception;

Expand All @@ -16,7 +17,7 @@ public function __construct(
parent::__construct($event, $formIntegration, $integration);
}

public static function getValidationRules(): array
public static function getValidationRules(?Form $form): array
{
return [];
}
Expand Down
175 changes: 175 additions & 0 deletions api/tests/Feature/Integrations/Email/EmailIntegrationTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
<?php

use App\Models\Integration\FormIntegration;

test('free user can create one email integration', function () {
$user = $this->actingAsUser();
$workspace = $this->createUserWorkspace($user);
$form = $this->createForm($user, $workspace);

// First email integration should succeed
$response = $this->postJson(route('open.forms.integration.create', $form), [
'integration_id' => 'email',
'status' => true,
'settings' => [
'send_to' => '[email protected]',
'sender_name' => 'Test Sender',
'subject' => 'Test Subject',
'email_content' => 'Test Content',
'include_submission_data' => true
]
]);

$response->assertSuccessful();
expect(FormIntegration::where('form_id', $form->id)->count())->toBe(1);

// Second email integration should fail
$response = $this->postJson(route('open.forms.integration.create', $form), [
'integration_id' => 'email',
'status' => true,
'settings' => [
'send_to' => '[email protected]',
'sender_name' => 'Test Sender',
'subject' => 'Test Subject',
'email_content' => 'Test Content',
'include_submission_data' => true
]
]);

$response->assertStatus(422)
->assertJson([
'errors' => [
'settings.send_to' => ['Free users are limited to 1 email integration per form.']
]
]);
});

test('pro user can create multiple email integrations', function () {
$user = $this->actingAsProUser();
$workspace = $this->createUserWorkspace($user);
$form = $this->createForm($user, $workspace);

// First email integration
$response = $this->postJson(route('open.forms.integration.create', $form), [
'integration_id' => 'email',
'status' => true,
'settings' => [
'send_to' => '[email protected]',
'sender_name' => 'Test Sender',
'subject' => 'Test Subject',
'email_content' => 'Test Content',
'include_submission_data' => true
]
]);

$response->assertSuccessful();

// Second email integration should also succeed for pro users
$response = $this->postJson(route('open.forms.integration.create', $form), [
'integration_id' => 'email',
'status' => true,
'settings' => [
'send_to' => '[email protected]',
'sender_name' => 'Test Sender',
'subject' => 'Test Subject',
'email_content' => 'Test Content',
'include_submission_data' => true
]
]);

$response->assertSuccessful();
expect(FormIntegration::where('form_id', $form->id)->count())->toBe(2);
});

test('free user cannot add multiple emails', function () {
$user = $this->actingAsUser();
$workspace = $this->createUserWorkspace($user);
$form = $this->createForm($user, $workspace);

$response = $this->postJson(route('open.forms.integration.create', $form), [
'integration_id' => 'email',
'status' => true,
'settings' => [
'send_to' => "[email protected]\n[email protected]",
'sender_name' => 'Test Sender',
'subject' => 'Test Subject',
'email_content' => 'Test Content',
'include_submission_data' => true
]
]);

$response->assertStatus(422)
->assertJsonValidationErrors(['settings.send_to'])
->assertJson([
'errors' => [
'settings.send_to' => ['You can only send to a single email address on the free plan. Please upgrade to the Pro plan to create a new integration.']
]
]);
});

test('pro user can add multiple emails', function () {
$user = $this->actingAsProUser();
$workspace = $this->createUserWorkspace($user);
$form = $this->createForm($user, $workspace);

$response = $this->postJson(route('open.forms.integration.create', $form), [
'integration_id' => 'email',
'status' => true,
'settings' => [
'send_to' => "[email protected]\n[email protected]\n[email protected]",
'sender_name' => 'Test Sender',
'subject' => 'Test Subject',
'email_content' => 'Test Content',
'include_submission_data' => true
]
]);

$response->assertSuccessful();

$integration = FormIntegration::where('form_id', $form->id)->first();
expect($integration)->not->toBeNull();
expect($integration->data->send_to)->toContain('[email protected]');
expect($integration->data->send_to)->toContain('[email protected]');
expect($integration->data->send_to)->toContain('[email protected]');
});

test('free user can update their single email integration', function () {
$user = $this->actingAsUser();
$workspace = $this->createUserWorkspace($user);
$form = $this->createForm($user, $workspace);

// Create initial integration
$response = $this->postJson(route('open.forms.integration.create', $form), [
'integration_id' => 'email',
'status' => true,
'settings' => [
'send_to' => '[email protected]',
'sender_name' => 'Test Sender',
'subject' => 'Test Subject',
'email_content' => 'Test Content',
'include_submission_data' => true
]
]);

$response->assertSuccessful();
$integrationId = $response->json('form_integration.id');

// Update the integration
$response = $this->putJson(route('open.forms.integration.update', [$form, $integrationId]), [
'integration_id' => 'email',
'status' => true,
'settings' => [
'send_to' => '[email protected]',
'sender_name' => 'Updated Sender',
'subject' => 'Updated Subject',
'email_content' => 'Updated Content',
'include_submission_data' => true
]
]);

$response->assertSuccessful();

$integration = FormIntegration::find($integrationId);
expect($integration->data->send_to)->toBe('[email protected]');
expect($integration->data->sender_name)->toBe('Updated Sender');
});
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
<div class="flex justify-center gap-x-2">
<v-button
class="px-8"
:loading="loading"
@click.prevent="save"
>
Save
Expand Down Expand Up @@ -55,6 +56,7 @@ const props = defineProps({
const alert = useAlert()
const emit = defineEmits(["close"])
const loading = ref(false)
const formIntegrationsStore = useFormIntegrationsStore()
const formIntegration = computed(() =>
Expand Down Expand Up @@ -98,7 +100,8 @@ const initIntegrationData = () => {
initIntegrationData()
const save = () => {
if (!integrationData.value) return
if (!integrationData.value || loading.value) return
loading.value = true
integrationData.value
.submit(
props.formIntegrationId ? "PUT" : "POST",
Expand All @@ -117,5 +120,8 @@ const save = () => {
alert.error("An error occurred while saving the integration")
}
})
.finally(() => {
loading.value = false
})
}
</script>

0 comments on commit 01f7fa2

Please sign in to comment.