Opened 6 years ago

Last modified 5 years ago

#13376 new New feature

Messages should have an "expire" flag

Reported by: ryanshow@… Owned by:
Component: contrib.messages Version: 1.2-beta
Severity: Normal Keywords: messages, expire
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no


Messages should have a flag which sets their expiration behavior. In the current messages framework, messages are automatically expired as soon as they are iterated over. This can be overridden if you explicitly set their "used" flag to false after they have been iterated. It would be nice to be able to set a flag which tells the message never to expire unless explicitly told to do so.

Attachments (4)

messages_expirity_capabilities.1.diff (36.9 KB) - added by marekw2143 6 years ago.
messages_expirity_capabilities.2.diff (35.8 KB) - added by marekw2143 5 years ago.
messages_expirity_capabilities.3.diff (37.3 KB) - added by marekw2143 5 years ago.
messages_expirity_capabilities.4.diff (35.7 KB) - added by marekw2143 5 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 6 years ago by russellm

  • milestone 1.3 deleted
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Getting the right API will be important here, but I can see how the idea could be useful. Expiry based on time, number of displays, or some other request/user based flag could all be useful.

I've removed the 1.3 milestone because it's a feature request without a patch.

comment:2 Changed 6 years ago by marekw2143

  • Owner changed from nobody to marekw2143
  • Status changed from new to assigned

Changed 6 years ago by marekw2143

comment:3 Changed 6 years ago by bpeschier

  • Has patch set

Pretty substantial patch, looks good at first sight, might take a deeper look soon.

comment:4 Changed 6 years ago by bpeschier

  • Needs documentation set
  • Patch needs improvement set

After inspection I have a couple of notes on the (otherwise good) patch:

  • I do not completely agree with the chosen api-functions on restrictions; on_iter and to_remove are too vague, I would use something along the lines of on_display since iteration could be happening at a lot of different points of time and is_expired to put more emphasis on the conceptual state (restriction expired) instead of the procedural state (we have to remove it).
  • The changes in the API must be documented so people can actually make use of restrictions without having to read code.
  • There are quite a few spelling errors and typos in the patch which give it a rushed feeling

comment:5 Changed 5 years ago by bpeschier

After going through the "storage" discussion on the mailing list, I looked a bit further into the patch and spotted something related which should be improved: at the moment the encoding is done centrally in storage/ with hardcoded checks for types. Maintaining different types of restrictions becomes a real pain in the backside this way. It should be de-centralised to the actual restriction class (which brings the problem of determining which restriction should be loaded for the decode).

Changed 5 years ago by marekw2143

Changed 5 years ago by marekw2143

comment:6 Changed 5 years ago by gabrielhurley

  • Component changed from Contrib apps to contrib.messages

comment:7 Changed 5 years ago by julien

  • Severity set to Normal
  • Type set to New feature

Changed 5 years ago by marekw2143

comment:8 Changed 5 years ago by marekw2143

  • Easy pickings unset
  • Owner marekw2143 deleted
  • Status changed from assigned to new
  • UI/UX unset
Note: See TracTickets for help on using tickets.
Back to Top