-
Notifications
You must be signed in to change notification settings - Fork 327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
variables and mentions #590
Changes from all commits
6c056f2
70804ef
4640b26
1b5dc99
3773313
f8c2b30
1e96b46
92dc063
4c19d66
5ffec25
e864d41
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
|
||
use App\Events\Forms\FormSubmitted; | ||
use App\Mail\OpenFormMail; | ||
use App\Open\MentionParser; | ||
use App\Service\Forms\FormSubmissionFormatter; | ||
use Illuminate\Bus\Queueable; | ||
use Illuminate\Contracts\Queue\ShouldQueue; | ||
|
@@ -16,13 +17,21 @@ class SubmissionConfirmationMail extends OpenFormMail implements ShouldQueue | |
use Queueable; | ||
use SerializesModels; | ||
|
||
private $formattedData; | ||
|
||
/** | ||
* Create a new message instance. | ||
* | ||
* @return void | ||
*/ | ||
public function __construct(private FormSubmitted $event, private $integrationData) | ||
{ | ||
$formatter = (new FormSubmissionFormatter($event->form, $event->data)) | ||
->createLinks() | ||
->outputStringsOnly() | ||
->useSignedUrlForFiles(); | ||
|
||
$this->formattedData = $formatter->getFieldsWithValue(); | ||
} | ||
|
||
/** | ||
|
@@ -34,17 +43,14 @@ public function build() | |
{ | ||
$form = $this->event->form; | ||
|
||
$formatter = (new FormSubmissionFormatter($form, $this->event->data)) | ||
->createLinks() | ||
->outputStringsOnly() | ||
->useSignedUrlForFiles(); | ||
|
||
return $this | ||
->replyTo($this->getReplyToEmail($form->creator->email)) | ||
->from($this->getFromEmail(), $this->integrationData->notification_sender) | ||
->subject($this->integrationData->notification_subject) | ||
->subject($this->getSubject()) | ||
->markdown('mail.form.confirmation-submission-notification', [ | ||
'fields' => $formatter->getFieldsWithValue(), | ||
'notificationBody' => $this->getNotificationBody(), | ||
'fields' => $this->formattedData, | ||
'form' => $form, | ||
'integrationData' => $this->integrationData, | ||
'noBranding' => $form->no_branding, | ||
|
@@ -67,9 +73,25 @@ private function getReplyToEmail($default) | |
{ | ||
$replyTo = $this->integrationData->confirmation_reply_to ?? null; | ||
|
||
if ($replyTo && filter_var($replyTo, FILTER_VALIDATE_EMAIL)) { | ||
return $replyTo; | ||
if ($replyTo) { | ||
$parser = new MentionParser($replyTo, $this->formattedData); | ||
$parsedReplyTo = $parser->parse(); | ||
if ($parsedReplyTo && filter_var($parsedReplyTo, FILTER_VALIDATE_EMAIL)) { | ||
return $parsedReplyTo; | ||
} | ||
Comment on lines
+76
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider Refactoring Repetitive Parsing Logic The parsing logic using Here's how you might refactor the parsing logic: private function parseIntegrationDataField($field)
{
$value = $this->integrationData->$field ?? null;
if ($value) {
$parser = new MentionParser($value, $this->formattedData);
return $parser->parse();
}
return null;
} Update the methods accordingly: private function getReplyToEmail($default)
{
$parsedReplyTo = $this->parseIntegrationDataField('confirmation_reply_to');
if ($parsedReplyTo && filter_var($parsedReplyTo, FILTER_VALIDATE_EMAIL)) {
return $parsedReplyTo;
}
return $default;
}
private function getSubject()
{
- $parser = new MentionParser($this->integrationData->notification_subject, $this->formattedData);
- return $parser->parse();
+ return $this->parseIntegrationDataField('notification_subject') ?? '';
}
private function getNotificationBody()
{
- $parser = new MentionParser($this->integrationData->notification_body, $this->formattedData);
- return $parser->parse();
+ return $this->parseIntegrationDataField('notification_body') ?? '';
} |
||
} | ||
return $default; | ||
} | ||
|
||
private function getSubject() | ||
{ | ||
$parser = new MentionParser($this->integrationData->notification_subject, $this->formattedData); | ||
return $parser->parse(); | ||
} | ||
|
||
private function getNotificationBody() | ||
{ | ||
$parser = new MentionParser($this->integrationData->notification_body, $this->formattedData); | ||
return $parser->parse(); | ||
} | ||
Comment on lines
+92
to
+96
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Review the Usage of The |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
<?php | ||
|
||
namespace App\Open; | ||
|
||
use DOMDocument; | ||
use DOMXPath; | ||
|
||
class MentionParser | ||
{ | ||
private $content; | ||
private $data; | ||
|
||
public function __construct($content, $data) | ||
{ | ||
$this->content = $content; | ||
$this->data = $data; | ||
} | ||
Comment on lines
+13
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add type hints and return types for improved code clarity Adding parameter and return type hints enhances readability and helps catch type-related bugs. Consider updating your methods with type declarations: -public function __construct($content, $data)
+public function __construct(string $content, array $data)
-public function parse()
+public function parse(): string
-private function replaceMentions()
+private function replaceMentions(): string
-private function getData($fieldId)
+private function getData(string $fieldId) Also applies to: 16-20, 21-41, 43-54 |
||
|
||
public function parse() | ||
{ | ||
$doc = new DOMDocument(); | ||
// Disable libxml errors and use internal errors | ||
$internalErrors = libxml_use_internal_errors(true); | ||
|
||
// Wrap the content in a root element to ensure it's valid XML | ||
$wrappedContent = '<root>' . $this->content . '</root>'; | ||
|
||
// Load HTML, using UTF-8 encoding | ||
$doc->loadHTML(mb_convert_encoding($wrappedContent, 'HTML-ENTITIES', 'UTF-8'), LIBXML_HTML_NOIMPLIED | LIBXML_HTML_NODEFDTD); | ||
|
||
// Restore libxml error handling | ||
libxml_use_internal_errors($internalErrors); | ||
|
||
$xpath = new DOMXPath($doc); | ||
$mentionElements = $xpath->query("//span[@mention]"); | ||
|
||
foreach ($mentionElements as $element) { | ||
$fieldId = $element->getAttribute('mention-field-id'); | ||
$fallback = $element->getAttribute('mention-fallback'); | ||
$value = $this->getData($fieldId); | ||
|
||
if ($value !== null) { | ||
$textNode = $doc->createTextNode(is_array($value) ? implode(', ', $value) : $value); | ||
$element->parentNode->replaceChild($textNode, $element); | ||
} elseif ($fallback) { | ||
$textNode = $doc->createTextNode($fallback); | ||
$element->parentNode->replaceChild($textNode, $element); | ||
} else { | ||
$element->parentNode->removeChild($element); | ||
} | ||
} | ||
|
||
// Extract and return the processed HTML content | ||
$result = $doc->saveHTML($doc->getElementsByTagName('root')->item(0)); | ||
|
||
// Remove the root tags we added | ||
$result = preg_replace('/<\/?root>/', '', $result); | ||
|
||
// Trim whitespace and convert HTML entities back to UTF-8 characters | ||
$result = trim(html_entity_decode($result, ENT_QUOTES | ENT_HTML5, 'UTF-8')); | ||
|
||
return $result; | ||
} | ||
Comment on lines
+19
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider refactoring and adding error handling. The For example:
Also, consider wrapping the DOM operations in a try-catch block to handle any potential exceptions. |
||
|
||
private function replaceMentions() | ||
{ | ||
$pattern = '/<span[^>]*mention-field-id="([^"]*)"[^>]*mention-fallback="([^"]*)"[^>]*>.*?<\/span>/'; | ||
return preg_replace_callback($pattern, function ($matches) { | ||
$fieldId = $matches[1]; | ||
$fallback = $matches[2]; | ||
$value = $this->getData($fieldId); | ||
|
||
if ($value !== null) { | ||
if (is_array($value)) { | ||
return implode(' ', array_map(function ($v) { | ||
return $v; | ||
}, $value)); | ||
} | ||
return $value; | ||
} elseif ($fallback) { | ||
return $fallback; | ||
} | ||
return ''; | ||
}, $this->content); | ||
} | ||
Comment on lines
+65
to
+85
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clarify the purpose of the unused The
Keeping unused methods can lead to confusion and maintenance issues in the future. |
||
|
||
private function getData($fieldId) | ||
{ | ||
$value = collect($this->data)->firstWhere('id', $fieldId)['value'] ?? null; | ||
|
||
if (is_object($value)) { | ||
return (array) $value; | ||
} | ||
|
||
return $value; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
<?php | ||
|
||
namespace App\Service\HtmlPurifier; | ||
|
||
use HTMLPurifier_HTMLDefinition; | ||
use Stevebauman\Purify\Definitions\Definition; | ||
use Stevebauman\Purify\Definitions\Html5Definition; | ||
|
||
class OpenFormsHtmlDefinition implements Definition | ||
{ | ||
public static function apply(HTMLPurifier_HTMLDefinition $definition) | ||
{ | ||
Html5Definition::apply($definition); | ||
|
||
$definition->addAttribute('span', 'mention-field-id', 'Text'); | ||
$definition->addAttribute('span', 'mention-field-name', 'Text'); | ||
$definition->addAttribute('span', 'mention-fallback', 'Text'); | ||
$definition->addAttribute('span', 'mention', 'Bool'); | ||
$definition->addAttribute('span', 'contenteditable', 'Bool'); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle Potential Exceptions from
FormSubmissionFormatter
While constructing the
FormSubmissionFormatter
, exceptions could be thrown if the formatting fails. Consider adding exception handling to manage potential errors gracefully and provide fallback mechanisms if necessary.Example: