Rework the tag system #654
Conversation
- move column `tags` from `entry`, `entry_comment`, `post` and `post_comment` to its own table `hashtag` with a linking table `hashtag_link` which references the 4 tables. Migrating up and down should be possible without any hiccups via the migration - Adjust a lot of code to use the new code to get the linked hashtags - the hashtag table uses the `citext` type of psql. For it to work `citext` was added as a type and a mapping type to the doctrine configuration - add hashtag banning as an admin option. Posts which already exists with the banned hashtag are not shown anymore (they are in the tag timeline) and new posts which contain a banned hashtag throw an exception when created - add a hashtag panel, similar to the magazine panel. In the future subscriber count, subscribe button and block button can be added there as well - remove the formrow for defining hashtags which are not part of the body. This may be reverted, but the row always confused me - remove the hard coded related hashtags, it is an empty panel at the moment - made some adjustments in the `SearchRepository` to filter out banned hashtags and use the new `NativeQueryAdapter` which should improve the performance of it
BentiGorlich
added
enhancement
- put the extract method and its dependencies in a class which can be constructed in our unit tests. The `TagManager` now depends on symfony classes...
|
I have it running on <gehirneimer.de> if anyone wants to take a peek at that :) |
- use the internal count method instead of iterating over the array
|
I marked is as ready again, because the the microblog pages being slow has bothing to do with this PR, just with my server needing more RAM 😅 |
Please don't cause more regression. We still try to fix the regression issues from the delete pr. 😔 |
...I hope you're not being serious, I highly doubt @BentiGorlich is intentionally causing regression. |
|
I'm actually pretty serious here. Yes I know he doesn't do it on purpose. Nobody does. But I would first fix the regression issues before adding more features or refactoring like this PR. There need to be focus on stability and regression fixes first, before continuing with more. |
|
I'm gonna respond in a longer post later, because you're kindakilling motivation with rhis kind of statement, but for now: you guys already fixed the regressions we know of and the problems occurred while I was working on this one, so I don't get where your attitude is coming from... |
I didn't sleep well. But it was also still on my mind. |
|
@BentiGorlich @nobodyatroot again sorry how I reacted again. I have some bad time recently. I already talked to Benti about it. Hopefully it will all settle again. For now I have too much Tinnitus, so I take some time off. |
|
@melroy89 Thanks for apologizing publicly, I mean it :) |
This comment was marked as resolved.
This comment was marked as resolved.
|
Thanks for testing. How do I test it? |
Someone is actually using the API on my site, lol. Maybe point the interstellar app at your instance and try to monkey with the site functions? Kind of a brute force testing solution until we get proper API unit tests in place. |
|
I added a commit. We still need to figure out what to do when an entry with a banned hashtag is created |
Would a generic "post/entry is not allowed" flash message be sufficient? |
ghost
mentioned this pull request
|
@BentiGorlich fyi, i did see this in
|
|
No I didn't notice it, but I will definitely fix it. |
|
Fix lint. |
ghost
self-requested a review
|
The question is: is "Thread could not be created. The content is not allowed." a good enough error message when a banned hashtag is used? |
I think so, if they really want to know why it's banned they can ask/message the admin.... might turn into a difficult conversation :-) |
The most integral thing I've done here is to rework the db schema for hashtags. We now have a table called
hashtagwhich contains the string tag it self as well as other info (atm onlybanned) andhashtag_linkwhich links posts of any kind to hashtags if they contain them.The migration I wrote contains sql code to transition between the new and old schema.
Changing this causes a lot of necessary adjustments.
I have not tested the API methods, because I don't know how 😅That should definitely be done.
The reason I did it is that the hashtag queries are one of the slowest queries we have at the moment and it is not possible to reasonably implement banning, blocking or subscribing to hashtags with the old system.
One cool thing I did in the process is to write the
NativeQueryAdapterfor pagination. It allows to put any SQL query in there and get a pagination interface for it. I replaces some of the queries in theSearchRepositorywith it.Visible things I changed:
1. [new] Hashtag panel:
2. [new] Hashtag Admin Panel Options:
There is now a button to ban/unban a hashtag (see screenshot above). The ban button is a danger button, the unban button is not.
3. [removed] Related Hashtags:
I decided to remove the hard coded related hashtags, but keep the box they were in (see screenshot above). I tried removing the box as well, but it just looked weird. We can add the usual filter options to it in the future
4. [removed] "tags" Input:
This can be reverted, but the input always just confused me anyways, so removed it for ease of use.
Behaviour of banned hashtags:
I went for a more temporary approach of the ban: keep posts that already exists, but block new posts from being created.
Local
If a user tries to create a post with a banned hashtag in them they see this error message:
I did not add an error message that specifically said "Tag x is banned" so the user can't circumvent the ban that easily. If this doesn't make any sense to anyone we can of course change it.
Remote
When a post is created with a banned hashtag in the body a
TagBannedExceptiongets thrown. This is caught in theCreateHandlerand theChainActivityHandlerand the post is discarded (these are the only two points where posts get created that I could find). So when we get a remote post which contains a banned hashtag we do not create it at all.visibility
When a post contains a banned hashtag it is hidden in most places: search, timeline (thread and microblog). The one thing where they still show up is on the hashtag page itself. My idea was that the admin should still be able to see all the posts that acutally exists containing the hashtag. I considered making the hashtag side admin only if a hashtag is banned, but I didn't do it, yet. I am counting on your feedback, how we should handle it.