Skip to content

add content id for attachments #30

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.

Sign up for GitHub

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

Open
farhan27 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add content id for attachments #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions actions/send_email.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ def run(self, email_from, email_to, subject, message,

accounts = self.config.get('smtp_accounts', None)
if accounts is None:
raise ValueError('"smtp_accounts" config value is required to send email.')
raise ValueError(
'"smtp_accounts" config value is required to send email.')
if not accounts:
raise ValueError('at least one account is required to send email.')

Expand All @@ -42,11 +43,14 @@ def run(self, email_from, email_to, subject, message,
email_to += email_cc

attachments = attachments or tuple()
for filepath in attachments:
filename = os.path.basename(filepath)
with open(filepath, 'rb') as f:
for el in attachments:
filename = os.path.basename(el['filePath'])
with open(el['filePath'], 'rb') as f:
Comment on lines +47 to +48
Copy link
Contributor

@blag blag Apr 13, 2021

Choose a reason for hiding this comment

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

Additionally, it should be possible to make this backwards compatible, so users of this pack do not have to update every single place they call this action. Please modify the Python to use the old code path if el is a string, and the new codepath if el is a dictionary.

Also, please use snake_case for dictionary keys in StackStorm, not camelCase.

part = MIMEApplication(f.read(), Name=filename)
part['Content-Disposition'] = 'attachment; filename="{}"'.format(filename)
part['Content-Disposition'] = 'attachment; filename="{}"'.format(
filename)
part['Content-ID'] = '<{}>'.format(el['fileId'])

msg.attach(part)

s = SMTP(account_data['server'], int(account_data['port']), timeout=20)
Expand Down
4 changes: 2 additions & 2 deletions actions/send_email.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,5 @@
attachments:
type: "array"
items:
type: "string"
description: "The absolute paths to the files to be included as attachments."
type: "Object"
Copy link
Contributor

@nmaludy nmaludy Feb 20, 2020

Choose a reason for hiding this comment

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

should "object" instead of uppercase "Object", this is why the test failed

Copy link
Contributor

@blag blag Apr 13, 2021

Choose a reason for hiding this comment

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

I don't like simply converting this over, because it would break this action for all users and all workflows that are currently using this action and passing in strings here.

We should be able to use oneOf here to preserve backwards compatibility for existing users of this pack. Please do so.

    attachments:
      type: array
      description: |
        An array or list of items, either strings or objects/dictionaries, that specifies
        each file to be included as attachments.
      items:
        oneOf:  # this should be camelCase, as defined by JSONSchema
          - type: string
            description: The absolute path to the file to be attached
          - type: object
            description: An object containing file_id and file_path keys
            properties:
              file_id:
                type: string
                description: A unique identifier for the attachment in the mail envelope
              file_path:
                type: string
                description:  The absolute path to the file to be attached

Note that oneOf should be camelCase, as it is defined in JSONSchema, but file_id and file_path should both be snake_case, as they are interpreted by Python in StackStorm.

description: "Array of object containing the absolute paths to the files and the file id"