Skip to content

add blank signature template file, added to all email notifications#39

Closed
qwertiko wants to merge 1 commit intofiveai:2.5from
qwertiko:notifications-signature
Closed

add blank signature template file, added to all email notifications#39
qwertiko wants to merge 1 commit intofiveai:2.5from
qwertiko:notifications-signature

Conversation

@qwertiko
Copy link
Copy Markdown

This allows for a simple method to include an email signature to all outgoing notifications. the blank file will avoid local customizations to be overridden.

@sedan07
Copy link
Copy Markdown

sedan07 commented Feb 19, 2021

@qwertiko thanks for this.

How do you go about setting the content of this signature file? At the moment the signature.blade.php file is tracked by git and so it will notice any local modifications and want them to be committed. Or am i missing something obvious?

Would it make more sense to make this an option in the dashboard the same way you can set "Custom Header HTML" under the "customization" section? Then the value is stored in the DB and injected into Blade?

@qwertiko
Copy link
Copy Markdown
Author

2 thoughts:
Why bother a database with something that does not change?
If a tracked file never receives a change, can others not do with it what they want? I believe it would never interfere, they could commit or stash and it would not matter. (I may be wrong here).

@sedan07
Copy link
Copy Markdown

sedan07 commented Feb 22, 2021

Why bother a database with something that does not change?

  • It may change, not often and not necessarily. But I don't think we can assume it won't ever need changing
    • The "Site Name" is also set in the DB but again rarely if ever changes
  • All the other site configuration options are in the DB and set via the dashboard so this would be inconsistent
    • Also the separation of concerns, the DB worries about the per-instance/deployment customisation whilst the code base worries about the general look of the site
  • Would need to be documented for others to find without reading the code base. Other settings are self discoverable by viewing the dashboard settings screen.
  • Requires slightly more technical knowledge to update a Blade file rather than some MD on a web page. (Minor)

If a tracked file never receives a change, can others not do with it what they want?

  • This would require the maintainers to keep this internal file unchanged. Generally changing your internal implementation is OK as long as you don't break your API(s). But this would force this internal file to be considered a part of the API. - Admittedly unlikely to change but 2 years from now who can say.
  • If deployed via git then if we ever refactored the views and this file got moved it would require more work from the end user.
    • You'd also have a dirty local checkout on a server which may be taken as an issue by someone reacting to an operational issue
  • If deployed as a Container Image then would need an extra file mounted into the image on deployment
    • More of an issue when setting up when you just go through settings on the dashboard. But then you want to change the signature now you need to modify the deployment and re-deploy.
  • If deployed by getting packaged up (say as an RPM) then would require either patching the codebase once deployed (may lead to brief downtime) or modifying RPM build to also inject extra files making the RPM less generic.
  • This would also add complexity if running the codebase from the read-only mount on production as you'd need to inject this file in prior to mounting read-only.

@qwertiko qwertiko closed this Feb 22, 2021
@qwertiko
Copy link
Copy Markdown
Author

@sedan07 when placing this html into the database, eloquent is quite happy to turn
<span style="color: #706e78;">--</span> into <span xss=removed>--</span>

do you have any idea where that can be stopped? Else I see no way to support valid useful css in html emails for the signature.
Ideas?

@qwertiko
Copy link
Copy Markdown
Author

unless i include the custom stylesheet into the email also

@qwertiko qwertiko deleted the notifications-signature branch February 24, 2021 15:20
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