On slack,discord add support for Include hidden fields #654
Conversation
WalkthroughThis pull request introduces a new feature for Discord and Slack integrations that allows optional inclusion of hidden fields in form submission notifications. The changes span across three files: Changes
Sequence DiagramsequenceDiagram
participant User
participant NotificationsUI
participant Integration
participant FormSubmissionFormatter
User->>NotificationsUI: Configure hidden fields option
NotificationsUI->>Integration: Set include_hidden_fields_submission_data
Integration->>FormSubmissionFormatter: Call showHiddenFields() if enabled
FormSubmissionFormatter-->>Integration: Return submission data with hidden fields
Integration->>User: Send webhook with optional hidden fields
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
client/components/open/integrations/components/NotificationsMessageActions.vue (1)
105-106: Consider adding a warning message for hidden fieldsWhile the implementation is correct, consider adding a warning message when enabling hidden fields, as they might contain sensitive information.
} else if (['include_hidden_fields_submission_data'].includes(keyname)) { this.compVal[keyname] = false + if (this.compVal[keyname]) { + this.$emit('warning', 'Hidden fields may contain sensitive information. Please review your form\'s hidden fields before enabling this option.') + } }api/app/Integrations/Handlers/SlackIntegration.php (1)
18-18: Consider abstracting common integration logicWhile the implementation is correct and consistent with DiscordIntegration, there's duplicate code for handling hidden fields. Consider moving this logic to the AbstractIntegrationHandler.
// In AbstractIntegrationHandler.php + protected function getFormattedSubmissionData(): array { + $settings = (array) $this->integrationData ?? []; + $formatter = (new FormSubmissionFormatter($this->form, $this->submissionData)) + ->outputStringsOnly(); + + if (Arr::get($settings, 'include_hidden_fields_submission_data', false)) { + $formatter->showHiddenFields(); + } + + return $formatter->getFieldsWithValue(); + } // In DiscordIntegration.php and SlackIntegration.php - $settings = (array) $this->integrationData ?? []; - $formatter = (new FormSubmissionFormatter($this->form, $this->submissionData))->outputStringsOnly(); - if (Arr::get($settings, 'include_hidden_fields_submission_data', false)) { - $formatter->showHiddenFields(); - } - $formattedData = $formatter->getFieldsWithValue(); + $formattedData = $this->getFormattedSubmissionData();Also applies to: 38-43
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
api/app/Integrations/Handlers/DiscordIntegration.php(2 hunks)api/app/Integrations/Handlers/SlackIntegration.php(2 hunks)client/components/open/integrations/components/NotificationsMessageActions.vue(3 hunks)
🔇 Additional comments (4)
client/components/open/integrations/components/NotificationsMessageActions.vue (1)
18-25: LGTM! Clear and consistent UI implementation
The toggle switch for hidden fields is well-implemented with:
- Clear labeling and helpful description
- Proper conditional rendering based on include_submission_data
- Consistent styling with other toggle switches
api/app/Integrations/Handlers/DiscordIntegration.php (2)
18-18: LGTM! Proper validation rule implementation
The validation rule correctly allows for nullable boolean values, maintaining consistency with the frontend implementation.
38-43: Verify hidden fields handling in FormSubmissionFormatter
The implementation looks correct, but we should verify the FormSubmissionFormatter's handling of hidden fields.
✅ Verification successful
Hidden fields handling in FormSubmissionFormatter is properly implemented
The implementation is correct and thorough. The FormSubmissionFormatter class:
- Has a
showHiddenFieldsflag that defaults to false - Properly checks this flag in both
getCleanKeyValue()andgetFieldsWithValue()methods - Uses
FormLogicPropertyResolver::isHidden()to determine if fields should be hidden - Skips hidden fields when the flag is false and includes them when true
- Maintains chainable method pattern for configuration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for FormSubmissionFormatter's showHiddenFields implementation
ast-grep --pattern 'class FormSubmissionFormatter {
$$$
public function showHiddenFields() {
$$$
}
$$$
}'
Length of output: 119
Script:
#!/bin/bash
# Let's try a broader search for FormSubmissionFormatter class and its implementation
rg -l "class FormSubmissionFormatter"
# Then search for showHiddenFields method across the codebase
rg "showHiddenFields"
# Also search for hidden fields related functionality
rg "hidden.*fields" -i
Length of output: 68267
Script:
#!/bin/bash
# Let's get the content of FormSubmissionFormatter.php to verify the implementation
cat api/app/Service/Forms/FormSubmissionFormatter.php
Length of output: 10583
api/app/Integrations/Handlers/SlackIntegration.php (1)
38-43: Verify consistent behavior across integrations
Let's verify that both Discord and Slack integrations handle hidden fields identically.
✅ Verification successful
Hidden fields handling is consistent across Discord and Slack integrations
Both Discord and Slack integrations handle hidden fields in exactly the same way:
- Both use the same
FormSubmissionFormatterclass - Both check the
include_hidden_fields_submission_datasetting usingArr::get($settings, 'include_hidden_fields_submission_data', false) - Both call
showHiddenFields()on the formatter when the setting is enabled - Both use
getFieldsWithValue()to retrieve the formatted data
The implementations are identical in terms of hidden fields handling, ensuring consistent behavior across both integrations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Compare the implementation of hidden fields handling
diff <(rg -A 5 'include_hidden_fields_submission_data' api/app/Integrations/Handlers/DiscordIntegration.php) <(rg -A 5 'include_hidden_fields_submission_data' api/app/Integrations/Handlers/SlackIntegration.php)
Length of output: 318
Script:
#!/bin/bash
# Let's first check if both integration files exist and examine their content
echo "=== Checking Discord Integration ==="
rg -A 10 "FormSubmissionFormatter" api/app/Integrations/Handlers/DiscordIntegration.php || echo "No matches found"
echo -e "\n=== Checking Slack Integration ==="
rg -A 10 "FormSubmissionFormatter" api/app/Integrations/Handlers/SlackIntegration.php || echo "No matches found"
Length of output: 2254
JhumanJ
deleted the
161a6-on-integration-add-option-for-include-hidden-fields
branch
Summary by CodeRabbit
New Features
Bug Fixes
Documentation