feat/validate video type before uploading to S3 by nolramaf · Pull Request #1897 · EvolutionAPI/evolution-api · GitHub
Skip to content

feat/validate video type before uploading to S3#1897

Merged
DavidsonGomes merged 4 commits intoEvolutionAPI:developfrom
nolramaf:feat/validate-video-type-before-uploading-to-s3
Sep 17, 2025
Merged

feat/validate video type before uploading to S3#1897
DavidsonGomes merged 4 commits intoEvolutionAPI:developfrom
nolramaf:feat/validate-video-type-before-uploading-to-s3

Conversation

@nolramaf
Copy link
Copy Markdown
Contributor

@nolramaf nolramaf commented Sep 1, 2025

  • Introduce SAVE_VIDEO in S3 config
  • Added check for video messages before attempting S3 upload
  • Prevents unnecessary uploads when SAVE_VIDEO is disabled

Summary by Sourcery

Introduce a new SAVE_VIDEO configuration flag to prevent unnecessary S3 uploads for video messages when disabled, and enforce this check across message handling services and utilities.

New Features:

  • Add SAVE_VIDEO boolean to S3 configuration settings

Enhancements:

  • Skip S3 upload for video messages in Baileys and Business WhatsApp services when SAVE_VIDEO is false
  • Adjust getConversationMessage utility to exclude video media URLs if SAVE_VIDEO is disabled

@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented Sep 1, 2025

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes - here's some feedback:

  • The if (isVideo && !SAVE_VIDEO) block is repeated in multiple places; consider extracting that logic into a shared helper or middleware to DRY up the upload flow.
  • The conditional logic in getConversationMessage around SAVE_VIDEO and media types is becoming complex—refactor or simplify it into smaller functions for readability and easier maintenance.
  • Instead of throwing a generic Error when video uploads are disabled, use a custom error class or error code so downstream logic can handle this case more explicitly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `if (isVideo && !SAVE_VIDEO)` block is repeated in multiple places; consider extracting that logic into a shared helper or middleware to DRY up the upload flow.
- The conditional logic in getConversationMessage around SAVE_VIDEO and media types is becoming complex—refactor or simplify it into smaller functions for readability and easier maintenance.
- Instead of throwing a generic Error when video uploads are disabled, use a custom error class or error code so downstream logic can handle this case more explicitly.

## Individual Comments

### Comment 1
<location> `src/api/integrations/channel/whatsapp/whatsapp.baileys.service.ts:1263` </location>
<code_context>
             if (isMedia) {
               if (this.configService.get<S3>('S3').ENABLE) {
                 try {
+                  if (isVideo && !this.configService.get<S3>('S3').SAVE_VIDEO) {
+                    throw new Error('Video upload is disabled.');
+                  }
+
</code_context>

<issue_to_address>
Throwing an error for disabled video upload may disrupt flow.

Instead of raising an exception, consider skipping video uploads and logging a warning to prevent service interruption for other media types.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
                  if (isVideo && !this.configService.get<S3>('S3').SAVE_VIDEO) {
                    throw new Error('Video upload is disabled.');
                  }
=======
                  if (isVideo && !this.configService.get<S3>('S3').SAVE_VIDEO) {
                    this.logger.warn('Video upload is disabled. Skipping video upload.');
                    // Skip video upload by returning early from this block
                    return;
                  }
>>>>>>> REPLACE

</suggested_fix>

### Comment 2
<location> `src/api/integrations/channel/meta/whatsapp.business.service.ts:462` </location>
<code_context>
                   mediaType = 'video';
                 }

+                if (mediaType == 'video' && !this.configService.get<S3>('S3').SAVE_VIDEO) {
+                  throw new Error('Video upload is disabled.');
+                }
+
</code_context>

<issue_to_address>
Error thrown for disabled video upload may affect user experience.

Instead of raising an exception, consider returning a user-friendly message or logging the event to avoid disrupting the user experience.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
                if (mediaType == 'video' && !this.configService.get<S3>('S3').SAVE_VIDEO) {
                  throw new Error('Video upload is disabled.');
                }
=======
                if (mediaType == 'video' && !this.configService.get<S3>('S3').SAVE_VIDEO) {
                  this.logger?.info?.('Video upload attempted but is disabled by configuration.');
                  return {
                    success: false,
                    message: 'Video upload is currently disabled. Please contact support if you need this feature enabled.',
                  };
                }
>>>>>>> REPLACE

</suggested_fix>

### Comment 3
<location> `src/config/env.config.ts:285` </location>
<code_context>
   USE_SSL?: boolean;
   REGION?: string;
   SKIP_POLICY?: boolean;
+  SAVE_VIDEO: boolean;
 };

</code_context>

<issue_to_address>
SAVE_VIDEO type should be optional for consistency.

Marking SAVE_VIDEO as optional aligns with the other S3 config properties and helps prevent issues with undefined values.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
  SAVE_VIDEO: boolean;
=======
  SAVE_VIDEO?: boolean;
>>>>>>> REPLACE

</suggested_fix>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/api/integrations/channel/whatsapp/whatsapp.baileys.service.ts
Comment thread src/api/integrations/channel/meta/whatsapp.business.service.ts
Comment thread src/config/env.config.ts Outdated
nolramaf and others added 3 commits September 1, 2025 19:49
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
@DavidsonGomes DavidsonGomes changed the base branch from main to develop September 2, 2025 13:02
@DavidsonGomes
Copy link
Copy Markdown
Collaborator

@DavidsonGomes DavidsonGomes merged commit 7ba8787 into EvolutionAPI:develop Sep 17, 2025
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants