Ticket #13376 - Messages should have an expire flag

Django Internals
1

Hey all,

I am having a stab at completing the last current accepted messages ticket

https://code.djangoproject.com/ticket/13376

It’s definitely a non trivial change and a lot has happened in the 13 years since it was last active.

Broadly I would to establish the following:

  • a consensus that this feature is still a desirable change?
  • any new comments/patterns/features that could be used to finish the ticket (assuming there is a consensus to move forward)
1 Like
2

I think @freakboy3742’s opening remark from comment:1 still holds:

Getting the right API will be important here…

Do we need time based expiry, number of views, a manual dismiss, …? — This isn’t something we can realistically experiment with inside Django itself, so we need to settle it before we try and merge something.

There’s two standard ways forward there:

  • Try and spec it, which can work, but often ends in the long-grass. (Maybe it’s obvious, IDK?)
  • Build a third-party package which lets us experiment with the API, and see what stabilises. (This is “pave the cow-paths”.)
1 Like
3

Trying to see if there has been much chat about this as a feature, this is what I can find:

I think that means some people (more than just the ticket raiser) has a use case for customising the default behaviour and agreed that “getting the right API” will be key :+1:

1 Like
4

Thanks for the responses!

Having worked on the existing patch that was submitted before the proposed API consists of 3 parts. (This me doing a short spec before getting too much into the weeds)

  1. Passing in a list of Restrictions (new class) which defines how the message sticks around. This is optional to be backwards-compatible.
  2. With in this class there are 2 methods:
    a. on_display: this is called any time the message is iterated over and may perform an action for that restriction (eg decrement a counter)
    b. is_expired: this checks whether the message is still active. If it’s active then display the message otherwise don’t.

On the face of it, this API seems like it would be flexible enough to cover any possible use case you could reasonably throw at it.

That said it’s probably impossible to be 100% sure of my statement above, which brings us to the second way forward of creating a third-party package. If this is the consensus then I would suggest that the ticket is closed (or a new status indicating the requirement of a third-party package)

I do see a potential third middle path, which is based on the following observations:

  • Most of the changes in this patch are to the messages BaseStorage & Message classes. As a result most of these changes could live in a third-party package to test out it’s popularity etc…
  • A smaller subset of changes affect the external messages API. The minimum here would be to added the kwarg restrictions to the add_message function and passing that through to the __init__ method. This could be generalised to adding **kwargs to these functions.

This change would enable third-party packages to hook more natively into the messages API without Django itself requiring to spec out the API for this piece explicitly. This would also enable future changes to the API via third-party packages.

Finally I agree that the API for clearing a message is via iteration isn’t clear at all and being more explicit here would be a benefit.

5

I think I am planning to split the PR I have started into two (ie the middle path described above)

The first PR will add minimal changes to the messages API to allow easier extensions to the Storage backends and Message object.

The second PR will implement the existing patch of adding restrictions to Messages as per the ticket.

6

To add to the above here the first PR from my message above.

Reviews and opinions welcome! Fixed #13376: Added kwargs argument to messages API to allow extension of functionality to the Storage & Message class. by nanorepublica · Pull Request #18332 · django/django · GitHub

For comparison the initial PR for this ticket is here: Fixed #13376: Added restrictions to messages to control when they are hidden. by nanorepublica · Pull Request #18286 · django/django · GitHub

1 Like