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

Add support for image attachment #49

Merged
merged 1 commit into from
Apr 10, 2023

Conversation

boryn
Copy link
Contributor

@boryn boryn commented Aug 8, 2021

  • Message accepts now image() method.
  • The message is sent using the multipart/form-data form (with or without the image).
  • If there is any error with the image, it is silently omitted and the message is sent without the attachment.

@boryn boryn marked this pull request as ready for review August 8, 2021 16:10
@boryn boryn mentioned this pull request Aug 8, 2021
@boryn boryn force-pushed the image-support branch 2 times, most recently from 39fc5d3 to 871676c Compare August 9, 2021 05:27
Copy link
Collaborator

@Kovah Kovah left a comment

Choose a reason for hiding this comment

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

I have tested this locally and it work's without issues. Maybe after merging the error handling could be improved by logging issues instead of silently discarding them.
One thing missing would be tests. At least the basic ones like in PushoverMessageTest for setting and retrieving the image path.

@atymic atymic merged commit 4c9606b into laravel-notification-channels:master Apr 10, 2023
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