Extract NotificationList state

[askvortsov1]

Refs #1821 #2144

Changes proposed in this pull request:

• Extract NotificationListState into its own state class

Reviewers should focus on:

• Anything I missed?

Confirmed

• Frontend changes: tested on a local Flarum installation.
• Backend changes: tests are green (run composer test).

[dsevillamartin]

Why are we passing the app global? I don't understand the purpose of this. If it is to allow for extensibility, wouldn't that require the extension developer to create their own whole Application class anyway with the data that this state requires?

[askvortsov1]

@datitisev As per the discussion in #2150 (comment)

[dsevillamartin]

Hm I see. It doesn't seem right to me.

[franzliedke]

@datitisev Would you prefer to see the actual dependencies (e.g. the store and the session) being passed in instead of the entire app?

[dsevillamartin]

I think so, yeah.

[franzliedke]

As far as I can see, this one is a bit different than other "state" PRs. The notification state does not seem to affect (or be relevant to) other parts of our application, so I do not see a reason to move it up to the application.

Instead, moving it from the NotificationList to the NotificationsDropdown would be enough - the dropdown component is the "highest" component that needs this data (no siblings or parents need it), and it could pass relevant data down to children (the NotificationList component) as props where they need it.

What do you think? Am I missing something?

[askvortsov1]

Instead, moving it from the NotificationList to the NotificationsDropdown would be enough - the dropdown component is the "highest" component that needs this data (no siblings or parents need it), and it could pass relevant data down to children (the NotificationList component) as props where they need it.

I would agree with this.

[askvortsov1]

I would agree with this.

Actually, scratch that: NotificationsPage also uses it.

[franzliedke]

Approved in a three-people call 😃

[dsevillamartin]

Breaking changes:

• app.cache.notifications is now app.notifications.getNotificationPages().
• methods for modifying app.cache.notifications are now available through app.notifications instead of the component