From 01f7fa224f6dee639df2812a39bde74ab833f018 Mon Sep 17 00:00:00 2001 From: Chirag Chhatrala Date: Tue, 10 Dec 2024 20:06:23 +0530 Subject: [PATCH] Refactor integration validation rules to include form context - 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. --- .../Integration/FormIntegrationsRequest.php | 5 +- .../Handlers/AbstractIntegrationHandler.php | 2 +- .../Handlers/DiscordIntegration.php | 3 +- .../Handlers/EmailIntegration.php | 34 +++- .../Handlers/GoogleSheetsIntegration.php | 7 +- .../Handlers/SlackIntegration.php | 3 +- .../Handlers/WebhookIntegration.php | 4 +- .../Handlers/ZapierIntegration.php | 3 +- .../Email/EmailIntegrationTest.php | 175 ++++++++++++++++++ .../components/IntegrationModal.vue | 8 +- 10 files changed, 230 insertions(+), 14 deletions(-) create mode 100644 api/tests/Feature/Integrations/Email/EmailIntegrationTest.php diff --git a/api/app/Http/Requests/Integration/FormIntegrationsRequest.php b/api/app/Http/Requests/Integration/FormIntegrationsRequest.php index 128a4c0e0..7856548e4 100644 --- a/api/app/Http/Requests/Integration/FormIntegrationsRequest.php +++ b/api/app/Http/Requests/Integration/FormIntegrationsRequest.php @@ -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; @@ -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); @@ -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; } } diff --git a/api/app/Integrations/Handlers/AbstractIntegrationHandler.php b/api/app/Integrations/Handlers/AbstractIntegrationHandler.php index 47fc78cbd..cf90f267f 100644 --- a/api/app/Integrations/Handlers/AbstractIntegrationHandler.php +++ b/api/app/Integrations/Handlers/AbstractIntegrationHandler.php @@ -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 { diff --git a/api/app/Integrations/Handlers/DiscordIntegration.php b/api/app/Integrations/Handlers/DiscordIntegration.php index 1a3fbaf1a..e9977fd16 100644 --- a/api/app/Integrations/Handlers/DiscordIntegration.php +++ b/api/app/Integrations/Handlers/DiscordIntegration.php @@ -2,6 +2,7 @@ namespace App\Integrations\Handlers; +use App\Models\Forms\Form; use App\Open\MentionParser; use App\Service\Forms\FormSubmissionFormatter; use Illuminate\Support\Arr; @@ -9,7 +10,7 @@ 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', diff --git a/api/app/Integrations/Handlers/EmailIntegration.php b/api/app/Integrations/Handlers/EmailIntegration.php index b20a31e74..4655f3665 100644 --- a/api/app/Integrations/Handlers/EmailIntegration.php +++ b/api/app/Integrations/Handlers/EmailIntegration.php @@ -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', @@ -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 diff --git a/api/app/Integrations/Handlers/GoogleSheetsIntegration.php b/api/app/Integrations/Handlers/GoogleSheetsIntegration.php index d2fd3ff7e..c903a79f7 100644 --- a/api/app/Integrations/Handlers/GoogleSheetsIntegration.php +++ b/api/app/Integrations/Handlers/GoogleSheetsIntegration.php @@ -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; @@ -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 diff --git a/api/app/Integrations/Handlers/SlackIntegration.php b/api/app/Integrations/Handlers/SlackIntegration.php index c34b664fa..41978f08b 100644 --- a/api/app/Integrations/Handlers/SlackIntegration.php +++ b/api/app/Integrations/Handlers/SlackIntegration.php @@ -2,6 +2,7 @@ namespace App\Integrations\Handlers; +use App\Models\Forms\Form; use App\Open\MentionParser; use App\Service\Forms\FormSubmissionFormatter; use Illuminate\Support\Arr; @@ -9,7 +10,7 @@ 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/', diff --git a/api/app/Integrations/Handlers/WebhookIntegration.php b/api/app/Integrations/Handlers/WebhookIntegration.php index f5d98ec50..2d745743a 100644 --- a/api/app/Integrations/Handlers/WebhookIntegration.php +++ b/api/app/Integrations/Handlers/WebhookIntegration.php @@ -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' diff --git a/api/app/Integrations/Handlers/ZapierIntegration.php b/api/app/Integrations/Handlers/ZapierIntegration.php index 4a71ad40d..c3c218840 100644 --- a/api/app/Integrations/Handlers/ZapierIntegration.php +++ b/api/app/Integrations/Handlers/ZapierIntegration.php @@ -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; @@ -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 []; } diff --git a/api/tests/Feature/Integrations/Email/EmailIntegrationTest.php b/api/tests/Feature/Integrations/Email/EmailIntegrationTest.php new file mode 100644 index 000000000..241c56398 --- /dev/null +++ b/api/tests/Feature/Integrations/Email/EmailIntegrationTest.php @@ -0,0 +1,175 @@ +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' => 'test@example.com', + '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' => 'another@example.com', + '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' => 'test@example.com', + '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' => 'another@example.com', + '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' => "test@example.com\nanother@example.com", + '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' => "test@example.com\nanother@example.com\nthird@example.com", + '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('test@example.com'); + expect($integration->data->send_to)->toContain('another@example.com'); + expect($integration->data->send_to)->toContain('third@example.com'); +}); + +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' => 'test@example.com', + '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' => 'updated@example.com', + '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('updated@example.com'); + expect($integration->data->sender_name)->toBe('Updated Sender'); +}); diff --git a/client/components/open/integrations/components/IntegrationModal.vue b/client/components/open/integrations/components/IntegrationModal.vue index 3531861d8..ddd2bad80 100644 --- a/client/components/open/integrations/components/IntegrationModal.vue +++ b/client/components/open/integrations/components/IntegrationModal.vue @@ -27,6 +27,7 @@
Save @@ -55,6 +56,7 @@ const props = defineProps({ const alert = useAlert() const emit = defineEmits(["close"]) +const loading = ref(false) const formIntegrationsStore = useFormIntegrationsStore() const formIntegration = computed(() => @@ -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", @@ -117,5 +120,8 @@ const save = () => { alert.error("An error occurred while saving the integration") } }) + .finally(() => { + loading.value = false + }) }