Inhaltlich veraltete Events erkennen und dazugehörige Benachrichtigungen aufräumen
Wir wollen, dass man über Änderungen informiert wird und dafür die Benachrichtigungsseite als zentrale Anlaufstelle hat. Außerdem wollen wir, dass "alte" Benachrichtigungen automatisch verschwinden, ohne, dass man manuell aufräumen muss. "Alt" wird eine Nachricht nicht nur, wenn ihr Event lange her ist, sondern auch, wenn sie inhaltlich nicht mehr aktuell ist.
Z.B. wenn ein Task erst unassigned ist, dann an User A assigned wird und anschließend an User B, dann interessiert auf der Benachrichtigungsseite nur noch, dass der Task jetzt bei User B angekommen ist. Die alten Benachrichtigungen können weg.
Auf der Detailseite des Tasks ist natürlich die ganze Historie weiterhin interessant.
Regeln, welche Events andere welche anderen Events ablösen ("supersede")
Die Regeln gelten immer ausgehend von einem neuen (gerade erstellten) Event für andere Events desselben Objects die einen bestimmten Typ haben müssen.
- added_successor_workflow -> superseeded NICHTS
- assigned -> assigned, unassigned
- changed_summary -> changed_summary
- commented -> superseeded NICHTS
- completed_workflow -> superseeded NICHTS
- deleted -> alle events zu diesem Objekt außer kommentare und das aktuelle deleted-event
- removed_workflow -> removed_workflow, added_workflow: gleiche Maßnahme UND gleiches Ziel
- unassigned -> assigned, unassigned
- added_workflow -> removed_workflow, added_workflow: gleiche Maßnahme UND gleiches Ziel
- changed_due_date -> changed_due_date
- changed_title -> changed_title
- status changed -> alle anderen status changed
- skipped
- completed -> potenzial für mehr (workflow macht task-events (teilweise) outdated etc.; title, summary, ... changed outdated?) → NEIN, siehe unten
- created
- readied
- reopened
- started
Abgrenzung (vorerst?) was nicht abgelöst werden soll
Wir haben überlegt, ob z.B. das Abschließen einer Maßnahme
- die vorherigen Events der Maßnahme (title changed, summary changed, etc) ablöst: nein, die sollen bleiben
- die Events der enthaltenen Tasks ablöst: nein, die sollen bleiben
Umsetzung
"Pseudo"-Code
# Migration
add_reference :events, :superseder, index: true
class Event < ActiveRecord:Base
# Verknüpfung zur Nutzung
belongs_to :superseder, class_name: "Event", optional: true
has_many :supersedees, class_name: "Event", foreign_key: :superseder, inverse_of: :superseder
# Automatisches Eintragen der Verknüpfung
after_commit :apply_supersedees, on: :create
def apply_supersedees
# candidates siehe unten
supersedee_candidates.unsuperseded.update_all(superseder_id: id)
end
# Upgrade-Migration muss IDs aufsteigend sortiert durchgehen und apply_supersedees aufrufen.
# Events brauchen dann wiederum after-update callbacks um als erledigt markiert zu werden.
Der eigentlich interessante Teil, wie die veralteten Events ermittelt werden liegt der Methode bzw. im Scope supersedee_candidates
. Als Instanz-Methode ruft sie einfach den Scope mit Parameter auf. Dieser wiederum setzt eine Reihe von Sub-Scopes zusammen, die die Bedingungen verknüpfen.
scope :supersedee_candidates, ->(event) {
event.class
# die Subklassen-spezifischen Regeln, welche Event-Typen welche anderen ablösen
.supersede_candidates_by_type
# nur Events, die vor dem aktuell betrachteten erstellt worden (praktisch hauptsächlich für Migration relevant)
.before_event(event)
# Nur Events, die das selbe Objekt betreffen kommen überhaupt in Frage
.same_object_as(event)
# Für einige wenige (im Moment Added/RemovedWorkflow (to/from Ambition)), ansonsten ist das ein NO-OP.
.supersede_constrain_by_value(event)
}
Hierbei erscheint elegant, dass die Regeln für die Typen schön in den jeweiligen Klassen gekapselt sind, ähnlich den Notification-Receivers regeln. Das hat aber auch zwei Nachteile:
- Es gibt eine Warnung:
Creating scope :supersede_candidates_by_type. Overwriting existing method Events::CompletedEvent.supersede_candidates_by_type.
-- bei Scopes scheint das überschreiben in Subklassen nicht so üblich zu sein? Jedenfalls ist es notwendig, dasssupersede_candidates_by_type
direkt auf der konkreten Subklasse und nicht später in der Relation-Kette aufgerufen wird, damit die richtige Implementierung genommen wird. - Für
supersede_constrain_by_value
besteht praktisch das gleiche Problem, es müsste ebenfalls am Anfang der Kette stehen, damit es in der Subklasse funktioniert. Stattdessen einecase/when
-Umsetzung. Funktioniert ohne Warning und ohne Überraschung, aber halt alles nicht schön nach EventType getrennt.
Der unsuperseded
Scope ist hier bewusst nicht dabei, weil es viel leichter ist ihn hinzuzufügen als wegzunehmen.
Ich frage mich, ob die Fallstricke unter 1. (Warnung; muss vorne stehen) durch die elegante Verteilung auf die Klassen gerechtfertigt ist, oder ob die by-type Regeln auch in ein großes case-when Statement sollten. Dann wäre aller Code in der Event-Klasse und nichts mehr in den Subklassen.
Ich frage mich, ob das ganze in einen Concern wandern sollte. Generischer wird es dadurch leider nicht, da die Klassennamen explizit im Code stehen.
Habe jetzt doch noch die "Ich-frage-mich" Varianten umgesetzt... alles an einem Ort und unter 100 Zeilen ist doch ganz nett, auch wenn es beim großen Type-Matching-Statement etwas wüst aussieht.
Ggf. könnte man auch by_type und by_value einfach in konkrete Regeln zusammenfassen. Da beide auf Instanzspezifischen Regeln aufbauen (und nicht mehr typspezifischen).
Notizen über Fehlversuche mit supersede_constrain_by_value für die Erinnerung
Keiner von den untenstehenden Ansätzen hat funktioniert, wegen den jeweils genannten Gründen.
class Events::AddedWorkflowEvent < Event
# naive approach (obviously not working, would need explicit columns)
scope :supersede_constrain_by_value, ->(event) { Events::AddedWorkflowEvent.where(new_value: event.new_value).or(Events::RemovedWorkflowEvent.where(old_value: event.new_value)) }
# jsonb approach (but we have text column here)
scope :supersede_constrain_by_value, ->(event) { Event.where(Arel.sql("data->'added_workflow'->'uri'->>'model_id' = ?", event.added_workflow.id)).or(Event.where(Arel.sql("data->'removed_workflow'->'uri'->>'model_id' = ?", event.added_workflow.id))) }
# text approach (doesn't work because STI Relations can only be combined by scope.or within a single hierarchy)
scope :supersede_constrain_by_value, ->(event) { Events::AddedWorkflowEvent.where(Arel.sql("data like '%/Workflow/#{event.added_workflow.id}%'")).or(::Events::RemovedWorkflowEvent.where(Arel.sql("data like '%/Workflow/#{event.added_workflow.id}%'"))) }
# Starting the Scope with Event.where makes it forget the constraints set by earlier relation scopes
scope :supersede_constrain_by_value, ->(event) { Event.where(Arel.sql("data like '%:added_workflow:%/Workflow/#{event.added_workflow.id}%'")).or(::Event.where(Arel.sql("data like '%:removed_workflow:%/Workflow/#{event.added_workflow.id}%'"))) }
Vielleicht wäre es trotzdem eine gute Idee die data-Spalte von text auf jsonb zu migrieren?