lamby / django-slack

Slack integration for Django, using the templating engine to generate messages
https://django-slack.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
241 stars 57 forks source link

Accept Template along with template path #112

Open ayushin opened 2 years ago

ayushin commented 2 years ago

Hi,

Very nice library, in our case we load templates from the database it would be great if we could pass Template object to slack_message() instead of just a pathname to the template.

lamby commented 2 years ago

Would you be able to work on a patch for this? I don't typically pass around Template instances in my projects, so I don't know the common/best practices.

odigity commented 2 years ago

I came here to ask for the same feature - not because I load templates from my database, but because when you're first trying out the library, or when you're doing development in general, it'd be convenient to pass a test string first and then promote it to a template file later. I think only accepting body content in the form of template files is unnecessarily restrictive.

lamby commented 2 years ago

Can you folks try https://github.com/lamby/django-slack/commit/ecdaad7545d3d6efb0a2f21fd56091e9c86c20fd ? Thanks. :)

odigity commented 2 years ago

I looked at the commit, but I didn't understand the change. I installed branch issue-112 locally and tried passing slack_message a message string instead of a template name string and it threw a TemplateDoesNotExist error, as usual.

I don't understand what has changed or how to use that change.

lamby commented 2 years ago

The change adds support for Template objects... but, wait, when you say "message string", what exactly do you mean by that? What is the Python data type of that?

odigity commented 2 years ago

I would like the ability to simply hand slack_message() a string to use as the message instead of having to create a separate file that gets loaded as a template, or having to wrap my string in a Django template object. Example:

slack_message( "Hello there." )

The simplest and most intuitive use case.

lamby commented 2 years ago

By what mechanism would the method determine whether this str would be a template string instead of a path though? (Hence supporting Template objects)

odigity commented 2 years ago

I don't have a good answer. The fact that the existing API expects a string containing a template name + the need to maintain backwards compatibility kind of kills the opportunity for the most obvious interface, which would be to simply pass a string containing the actual message.

You could branch on whether the string ends with ".slack", but that's fragile.

You could accept a flag in kwargs indicating the first arg is a message rather than template name, but that's ugly.

Perhaps it would make sense to export an alternative function like from_string() - though that's also ugly.


If this request doesn't fit in with your design or plans, feel free to ignore it. I've already started using my own slack messaging library based on your RequestsBackend which meets my specific needs.

lamby commented 2 years ago

Noted - thanks for the background and context. However, thinking about it for a bit, believe that the most obvious interface (as you put it) is the template path, not a string, ie. a conscious analogy or parallel to Django's regular rendering methods which take template paths precisely to avoid all of the ugliness of formatting potentially quite complicated strings in Python code... and to gain all the advantages of template filters and suchforth.