Skip to content
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

Support for Tags and Metadata? #20

Closed
guicapanema opened this issue Mar 27, 2019 · 10 comments
Closed

Support for Tags and Metadata? #20

guicapanema opened this issue Mar 27, 2019 · 10 comments

Comments

@guicapanema
Copy link

Hello there!

I'm using Laravel and Postmark using this adapter. I'm trying to send a message and add metadata/tags to it. Reading through the code & docs, I figured my best bet would be to add X-PM headers as per the SMTP documentation. I tried doing that:

public function build()
    {
		$this->withSwiftMessage(function (\Swift_Message $message) {
			$message->getHeaders()->addTextHeader('X-PM-Metadata-document-id', $this->document->id);
		});

		$client_name = $this->document->client->name;

		return $this->markdown('emails.documents.review')
					->attach($this->document->download_path, ['as' => $this->document->file_name])
					->subject('Document for review - ' . $client_name);
    }

The headers do get added to the message (I can see them when clicking the "raw source" tab in the UI) but Postmark doesn't seem to care for such headers when using the API.

Maybe it's possible to handle these special headers in the adapter? I see that there is some kind of precedent here.

Thank you!
Guilherme

@drakakisgeo
Copy link

Just made a pull request #21

@AndreasHerss
Copy link

This would be amazing! :D

@IzioDev
Copy link

IzioDev commented Oct 8, 2019

We just fork the repo and change the Transport.php file.

Line 285:

private function processHeaders(&$payload, $message)
	{
		$headers = [];
		foreach ($message->getHeaders()->getAll() as $key => $value) {
			$fieldName = $value->getFieldName();
			$excludedHeaders = ['Subject', 'Content-Type', 'MIME-Version', 'Date'];
			if (!in_array($fieldName, $excludedHeaders)) {
				if (
					$value instanceof \Swift_Mime_Headers_UnstructuredHeader ||
					$value instanceof \Swift_Mime_Headers_OpenDKIMHeader
				) {
					if ($fieldName != 'X-PM-Tag') {
						$metadataField = 'X-PM-Metadata-';
						if (substr($fieldName, 0, strlen($metadataField)) == $metadataField) {
							$payload["Metadata"][explode($metadataField, $fieldName)[1]] = $value->getValue();
						}

						array_push($headers, [
							"Name" => $fieldName,
							"Value" => $value->getValue(),
						]);
					} else {
						$payload["Tag"] = $value->getValue();
					}
				} else if (
					$value instanceof \Swift_Mime_Headers_DateHeader ||
					$value instanceof \Swift_Mime_Headers_IdentificationHeader ||
					$value instanceof \Swift_Mime_Headers_ParameterizedHeader ||
					$value instanceof \Swift_Mime_Headers_PathHeader
				) {
					array_push($headers, [
						"Name" => $fieldName,
						"Value" => $value->getFieldBody(),
					]);
					if ($value->getFieldName() == 'Message-ID') {
						array_push($headers, [
							"Name" => 'X-PM-KeepID',
							"Value" => 'true',
						]);
					}
				}
			}
		}
		$payload['Headers'] = $headers;
	}

This can handle multiple Metadata since the opened pull request only support one (the last entered)

@atheken
Copy link
Contributor

atheken commented Nov 16, 2019

Hi there. Thanks for the PR. We are actually in the process of correcting this in the API so that no updates will be required as we continue to add X-PM-* header support for new Postmark features. You should expect this to automatically start working in the next week or two.

@IzioDev
Copy link

IzioDev commented Nov 16, 2019

@atheken Will you inform postmark users with an email at least or something before you pushed you fix into production?

@atheken
Copy link
Contributor

atheken commented Nov 17, 2019

@izio38 - We announce changes in the API, and this will be no different. However, the change is additive, and really won't break anything, even your patch, here's why:

This transport translates SMTP requests into API calls. An oversight in our API endpoints was that there are cases like this where X-PM-* headers drive behavior/set properties of sends in SMTP, but not in normal API requests (such as Tags, Metadata, etc.).

We are updating our API so that X-PM-* headers will be used when present, but their corresponding request payload properties are absent in API requests (i.e. respect the explicit API payload properties when present, but set them if they are not present, but corresponding X-PM-* headers are included in the request).

Since we are making this the responsibility of the API, all future X-PM-* headers that we support will automatically be supported by any library that translates SMTP messages to API requests, even if that library does not explicitly handle those new headers.

In the case of this patch, if you map the metadata headers, this will set the API payload property, and the X-PM-Metadata-* headers are not needed, but as I said, this will be handled automatically by the API in either case.

We are obviously assuming the user intent was for X-PM-Metadata-* headers to be mapped as metadata in Postmark messages, but we believe this is a reasonable assumption, given the historic use of X-PM-* headers in Postmark.

Let me know if this alleviates your concerns, and if not, feel free to contact our support team with additional questions.

@AndreasHerss
Copy link

@atheken Do you know anything about when to expect it to be implemented in your api?
i really need this feature 😅

@atheken
Copy link
Contributor

atheken commented Nov 26, 2019

@AndreasHerss - we deployed this late last week, you should be all set.

@atheken atheken closed this as completed Nov 26, 2019
@atheken atheken reopened this Nov 26, 2019
@atheken
Copy link
Contributor

atheken commented Nov 26, 2019

Thanks again for the PR @izio38 - we do appreciate these enhancements, but we're trying to defer most of this work to the API so that customers don't need to update their clients as frequently/when we add new functionality. As I mentioned, this is now built in to the API, so no special handling is required by this transport.

@atheken atheken closed this as completed Nov 26, 2019
@AndreasHerss
Copy link

@atheken Sweet! just hadn't tried it since this issue was still open :) thank you!

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 a pull request may close this issue.

5 participants