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

Update SendGridService.php #70

Merged
merged 1 commit into from
Apr 14, 2014
Merged

Update SendGridService.php #70

merged 1 commit into from
Apr 14, 2014

Conversation

sb8244
Copy link
Contributor

@sb8244 sb8244 commented Apr 14, 2014

SendGrid should not attach a filetype

This is causing serious issues and serves no purpose. Remove the bug from the library! I use SendGrid and can tell you that it won't work. Most file readers are smart enough to detect a corrupted textual addition to the file, but then preview mode is disabled in Outlook.

SendGrid should not attach a filetype
@sb8244 sb8244 mentioned this pull request Apr 14, 2014
@bakura10
Copy link
Contributor

Although SendGrid recommends to send the filetype, there are also some examples where they do not. I can assume we can merge this fix then...

@juriansluiman thoughts?

@juriansluiman
Copy link
Contributor

I will test today and merge. I guess if you want to set the file type explicitly, we have to come up with another solution.

juriansluiman pushed a commit that referenced this pull request Apr 14, 2014
Update SendGridService.php
@juriansluiman juriansluiman merged commit dfff837 into Webador:master Apr 14, 2014
@juriansluiman
Copy link
Contributor

With the file type addition (old master) Sendgrid did work for me too, but I guess you are right Google is smart enough to remove the last bits and Outlook is a typical client which struggles with such addition.

Removing the type did not cause any problems with my GMail client :)

@juriansluiman
Copy link
Contributor

By the way, 1.4.1 is tagged now with this change!

@sb8244
Copy link
Contributor Author

sb8244 commented Apr 14, 2014

I wonder if SendGrid is unable to pick up the file type for binary files versus more basic files types? I might look into why this was failing to offer a healthier approach then tear it out. When it is tagged for 1.4.1, does that mean it will be in composer the next deploy?

@juriansluiman
Copy link
Contributor

I have tested it with .pdf and .txt files. There has to be a way to enforce the file type, but I now prefer to add this and ensure it works in Outlook too.

Just to confirm: the problem only exists in Outlook? I can only test in GMail (web/phone) and I have no other client available....

@sb8244
Copy link
Contributor Author

sb8244 commented Apr 15, 2014

Incorrect. The problem is in the file data itself. Excel is smart enough to detect that it is a string appended onto the end of the file and offer a prompt to fix it. Outlook does not do such a thing for excel files.

I opened my excel file as a binary and could see the physical text string tacked onto the end of the binary.

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.

3 participants