Also disallow the first post in a discussion to be deleted or hidden
(thus preventing discussions with zero posts)
closesflarum/core#90closesflarum/core#92
We need to fire the PostWasDeleted event for every post when a
discussion is deleted. This means deleting big discussions will be an
intensive process, but that’s OK because it’s very rare.
All routes are now stored in a RouteCollection, which is then used
for dispatching by the (reusable) RouterMiddleware.
This change also entails moving all routes to the service providers.
This may be changed again later, and is done for convenience reasons
right now.
Get rid of Permissible - too complex and inefficient. Replace with:
- a “Locked” trait which works similarly but only evaluates logic on
hydrated models.
- a “VisibleScope” trait which also works similarly but only scopes
queries
This is all we need, Permissible is overkill. There is only one
instance where we have to duplicate some logic
(Discussion::scopeVisiblePosts and Post::allow(‘view’, …)) but it’s
barely anything.
Haven’t decoupled for now, we can definitely look at doing that later.
Permissions table seeder slightly updated.
Also did a bit of a query audit, there’s still a lot to be done but
it’s much better than it was. Some relatively low-hanging fruit
detailed in EloquentPostRepository.
Ditched the idea of having language packs as extensions. Reasoning:
1. Because we use machine keys for translations (rather than English
keys), extensions need to be able to define default translations. If
English translations are to be included in extensions and not in a
language pack extension, then it doesn’t make sense to have other
languages as language pack extensions. Inconsistency → complexity.
2. Translations should maintain version parity with their respective
extensions. There’s no way to do this if extension translations are
external to the extension.
Instead, localisation will be a core effort, as well as a per-extension
effort. Translators will be encouraged to send PRs to core + extensions.
In core, each locale has a directory containing three files:
- translations.yml
- config.js: contains pluralisation logic for the JS app, as well as
moment.js localisation if necessary
- config.php: contains pluralisation logic for the PHP app
Extensions can use the Flarum\Extend\Locale extender to add/override
translations/config to a locale.
Asset compilation has been completely refactored with a better
architecture. Translations + config.js are compiled and cached for the
currently active locale.
It works but it’s not the most pretty thing in the world. @franzliedke
Would be great if you could take a look at the whole formatting API and
work your magic on it sometime… my brain is fried!
After a morning of searching, it seems there is no PHP Markdown library
that has built-in XSS/sanitization support. The recommended solution is
to use HTMLPurifier.
This actually works out OK, though, as it’s probably a good idea to
enforce sanitization regardless of which formatters are enabled, and to
not leave them with the responsibility of sanitization (it’s a big
responsibility). Since we cache rendered posts, the slow speed of
HTMLPurifier isn’t a concern.
Note that HTMLPurifier requires a file to be loaded by Composer, but
Studio does not yet support this, so for now I have included it
manually.
Originally the user activity feed was implemented using UNIONs. I was
looking at make an API to add activity “sources”, or extra UNION
queries (select from posts, mentions, etc.) but quickly realised that
this is too slow and there’s no way to make it scale.
So I’ve implemented an API which is very similar to how notifications
work (see previous commit). The `activity` table is an aggregation of
stuff that happens, and it’s kept in sync by an ActivitySyncer which is
used whenever a post it created/edited/deleted, a user is
mentioned/unmentioned, etc.
Again, the API is very simple (see Core\Activity\PostedActivity +
Core\Handlers\Events\UserActivitySyncer)
It turns out that the idea of “sending” a notification is flawed. (What
happens if the notification subject is deleted shortly after? The
notified user would end up with a dud notification which would be
confusing. What about if a post is edited to mention an extra user? If
you sent out notifications, the users who’ve already been mentioned
would get a duplicate notification.)
Instead, I’ve introduced the idea of notification “syncing”. Whenever a
change is made to a piece of data (e.g. a post is created, edited, or
deleted), you make a common notification and “sync” it to a set of
users. The users who’ve received this notification before won’t get it
again. It will be sent out to new users, and hidden from users who’ve
received it before but are no longer recipients (e.g. users who’ve been
“unmentioned” in a post).
To keep track of this, we use the existing notifications database
table, with an added `is_deleted` column. The syncing/diffing is
handled all behind the scenes; the API is extremely simple (see
Core\Notifications\DiscussionRenamedNotification +
Core\Events\Handlers\DiscussionRenamedNotifier)
For now I’ve chucked it on Flarum\Core as a static method, but
ultimately I think we will need a ConfigRepository abstraction (whether
it replaces or sits underneath the Flarum\Core static method I’m not
sure).
Also starting to think about multisite scenarios, I think this is
important. The Forum model could actually end up with a database table
behind it, and each forum would have its own config settings? Haven’t
really thought about it too hard yet…
- The recipient(s) are the concern of the notifier/sender, not the
notification itself
- Allow “retraction” of notifications (e.g. if a discussion is
stickied, but then it is unstickied)
- Misc. cleanup
Perhaps also in user activity stream. They are used in the mentions
extension.
In order to generate the excerpt, each formatter can implement a
“strip” method which basically converts block formatting into inline
formatting.
For example: when you rename a discussion, DiscussionRenamedPost is
created. If you rename it again immediately afterwards, then a new
DiscussionRenamedPost can be merged into the old one. This will either
result in the old one being updated with the new title, or it being
deleted all together if it was renamed back to the old title.
Also make some tweaks:
- Merge SerializeAction::$include and
SerializeAction::$includeAvailable into a keyed boolean array
- Set defaults for SerializeAction::$limit and $limitMax
- Rename SerializeAction::$sortAvailable to $sortFields
This is useful for both the Sticky and Categories extensions, where if
you sticky a discussion and then immediately unsticky it, or if you
move it to a category and then immediately move it back, the last
“activity” post will be removed.
- In order to be consistent with the Ember/LESS naming scheme, renamed
Flarum\Web to Flarum\Forum.
- Moved common classes into Flarum\Support so that Flarum\Admin doesn’t
depend on Flarum\Forum. Also moved Actor into Flarum\Support as it
doesn’t belong in the domain.
- Extract shared Ember components into a “flarum-common” ember-cli
addon. This can be used by both the forum + admin Ember apps, keeping
things DRY
- Move LESS styles into their own top-level directory and do a similar
thing (extract common styles)
- Add LESS/JS compilation and versioning to PHP (AssetManager)
- Set up admin entry point
(Theoretical) upgrade instructions:
- Delete everything in [app_root]/public
- Set up tooling in forum/admin Ember apps (npm install/update, bower
install/update) and then build them (ember build)
- php artisan vendor:publish
- Upgrade flarum/flarum repo (slight change in a config file)
- If you need to trigger a LESS/JS recompile, delete the .css/.js files
in [app_root]/public/flarum. I set up LiveReload to do this for me when
I change files in less/ or ember/
Todo:
- Start writing admin app!
- Remove bootstrap/font-awesome from repo and instead depend on their
composer packages? Maybe? (Bower is not an option here)
- Notifications can be delivered in multiple ways (alert, email)
- Different notification types can implement interfaces to allow
themselves to be delivered in these various ways
- User preferences for each notification type/method combination are
automatically registered
This is so that if an extension adds a post type, and the database gets
populated with posts of that type, but then if the extension is
disabled, we wouldn’t want those posts to display because we would have
no knowledge about how to deal with/render them.
Addresses the following error when using pqsql.
[PDOException]
SQLSTATE[42703]: Undefined column: 7 ERROR: column "comment" does not exist
LINE 1: ...d) FROM posts WHERE user_id = users.id and type = "comment")
Perhaps this should be an extension, but it is pretty essential and I
can’t think of many instances where it wouldn’t be wanted. Would be
very easy to extract later on if need be.
New stuff:
- Signup + email confirmation.
- Updated authentication strategy with remember cookies. closes#5
- New search system with some example gambits! This is cool - check out
the source. Fulltext drivers will be implemented as decorators
overriding the EloquentPostRepository’s findByContent method.
- Lay down the foundation for bootstrapping the Ember app.
- Update Web layer’s asset manager to properly publish CSS/JS files.
- Console commands to run installation migrations and seeds.
Refactoring:
- New structure: move models, repositories, commands, and events into
their own namespaces, rather than grouping by entity.
- All events are classes.
- Use L5 middleware and command bus implementations.
- Clearer use of repositories and the Active Record pattern.
Repositories are used only for retrieval of ActiveRecord objects, and
then save/delete operations are called directly on those ActiveRecords.
This way, we don’t over-abstract at the cost of Eloquent magic, but
testing is still easy.
- Refactor of Web layer so that it uses the Actions routing
architecture.
- “Actor” concept instead of depending on Laravel’s Auth.
- General cleanup!
