Refactor & upgrade the event system

[ErisDS]

Recently, we've wired a few new things up to the event system and as we touch these pieces we're starting to find issues with the events and the data available to them.

Currently:

• There are custom events for key model changes such as user.activated or post.published as well as crud actions like post.added (a new draft).
• Each event gets provided with the full model at the time the event was fired

In many cases, we then want to answer the question "what changed", but this is where things get tricky because when deleting a model, the model is empty, and you have to use Bookshelf's `previousAttributes as we do here.

In all other cases, Bookshelf's previousAttributes is only for pre-save changes, not telling you what changed in the last update, as explained here - so Ghost has it's own thing called updatedAttributes

So far in reviewing all usages of events, we don't actually use any model functionality other than attributes / .toJSON() and either previousAttributes or updatedAttributes. We are therefore passing huge Model objects around which is unnecessary, because if we ever do want a real Model object we use forge().fetch() to get the model.

At some point, we want to expose events to Apps, and exposing full model objects would be horrible in terms of permissions.

Therefore, I think what we need to do is change all of our event.emit() calls to send JSON objects rather than models:

• for create: send only the .toJSON version as the first arg
• for updates: send the .toJSON() version as the first arg, and updatedAttributes as the second arg
• for destroy: send the previousAttributes as the first arg

It would be great to figure out a way to make these objects consistent - at the moment it's not possible to do toJSON() for updated/previous attributes. Our .toJSON() methods have been customised to filter out data like passwords and add extra fields like urls onto records.

In addition, to make our events more performant and to get access to the event name inside of handlers, I think it would be a good idea to swap out Ghost's event system to use https://github.com/asyncly/EventEmitter2.

Finally, it's also apparent that we actually send way too many edited/updated events for models because we update them whenever there's a call to update regardless of whether something changed or not. I recently changed this for the settings model and this same change is needed for other models.

Summary of tasks:

• Events should emit JSON not full models (Refactor & upgrade the event system #9248 (comment))
• Upgrade to EventEmitter2 (Switch to EventEmitter2 #10463)
• Don't send events when nothing has changed
• Don't send events when importing

Note: In #9178 it is spec'd that events should live in lib/common.

[ErisDS]

I think this needs a review as part of webhooks. We should identify what is actually needed and skip the rest.

[kirrg001]

In all other cases, Bookshelf's previousAttributes is only for pre-save changes, not telling you what changed in the last update, as explained here - so Ghost has it's own thing called updatedAttributes

This was fixed, see.

[kirrg001]

I agree that internal Ghost events should not receive models.

I think this needs a review as part of webhooks. We should identify what is actually needed and skip the rest.

Webhooks need to access current and previous attributes, but we have a lot of places in Ghost where we need to access both depending on the use case.

[kirrg001]

Will look into this issue closer to see what we should adopt right now depending on the full webhook spec.

[kirrg001]

Don't send events when nothing has changed

Definitely will sort this out, but i assume it's not trivial to solve, because relations will be always marked as changed if you send them in the request even if they have not changed. This will be tricky.

[kirrg001]

Don't send events when importing

We have to send events on import right now, because the url service needs to generate the urls and cache the resources.

[kirrg001]

Events should emit JSON not full models

The API output serializer was designed to receive models from the controller.

This gives us:

A) full access to the model e.g example
B) It reduces the need of executing toJSON in each API version
C) A centralised place to call toJSON before sending the request to the user
D) A centralised place to ensure we pass the correct model options

We could potentially make the API output serializers flexible and either accept JSON or a model, but I am not sure about this direction currently. If the API output serializers rely on models, we could lose potential functionality (see example of A). toJSON is IMO the final output for requests.

---

Regarding Webhooks. They don't suffer from sending models around. The code will be clean.

Pseudo in webhooks service:

// serializes a model's current attributes
api[webhook.api_version].serializers.output(model, options)

// serialize a model's previous attributes
api[webhook.api_version].serializers.output(model, {previous: true})

// API output serializes will ensure we transform a model into JSON format
model.toJSON(options)
model.toJSON({previous: true})

It could be that the big picture becomes a little bit more clearer this year. But for now i will close this issue.

[kirrg001]

Don't send events when nothing has changed

Definitely will sort this out, but i assume it's not trivial to solve, because relations will be always marked as changed if you send them in the request even if they have not changed. This will be tricky.

This was done.