Repository navigation
Add ability to block and subscribe Hashtags - #2180
blued-gear wants to merge 20 commits into
Conversation
This reverts commit 7ef7b3c.
# Conflicts: # src/Controller/User/Profile/UserBlockController.php # src/Repository/ContentRepository.php # src/Repository/Criteria.php # src/Utils/SqlHelpers.php
| </header> | ||
| </div> | ||
|
|
||
| {{ component('hashtag_sub', {hashtag: hashtag}) }} |
There was a problem hiding this comment.
Could we make sure every tag page passes a hashtag here? I can reproduce a 500 at /tag/blockedtag/threads/newest/all because TagEntryFrontController only passes tag. The posts, comments and people controllers have the same issue.
There was a problem hiding this comment.
I also still get an exception here, maybe because the tag is not in the DB, so the tag needs to get created
|
|
||
| public function block(User $user, Hashtag $hashtag): void | ||
| { | ||
| $user->blockHashtag($hashtag); |
There was a problem hiding this comment.
Blocking a hashtag should also remove an existing subscription, like domains and magazines do. I reproduced subscribing and then blocking the same hashtag, and it stays subscribed. Can we unsubscribe here before adding the block? Also, subscribing removes a block directly without dispatching HashtagBlockChangedEvent, so the cached block list can stay stale.
BentiGorlich
left a comment
There was a problem hiding this comment.
I am probably wrong about the persisting issue, but it looks weird to me, being used to having to do that manually 😅
|
|
||
| #[OneToMany(mappedBy: 'hashtag', targetEntity: HashtagSubscription::class, fetch: 'EXTRA_LAZY', cascade: [ | ||
| 'persist', | ||
| 'remove', |
There was a problem hiding this comment.
remove is not correct in this regard (I think). The doctrine removal is always really confusing for me, but it might be that it also removes the hashtag, when you're removing the subscription via $entiyManager->remove...
There was a problem hiding this comment.
According to a Stack Overflow answer, this should be correct
cascade={"remove"} meaning that removing entity A, Doctrine will also remove all B entities in the Collection.
Also from Doctrine Doc
Thanks to cascade: remove, you can easily delete a user and all linked comments without having to loop through them:
The inverse side HashtagSubscription has a onDelete: 'CASCADE' which will add a cascade to the foreign key constraint, so no hashtag-deletion by deleted subscription here too.
| $qb->andWhere( | ||
| 'NOT EXISTS (' | ||
| .'SELECT 1 FROM '.HashtagBlock::class.' hb INNER JOIN '.HashtagLink::class.' hbl ON hb.hashtag = hbl.hashtag ' | ||
| .'WHERE hbl.entry = e AND hb.user = :blocker' | ||
| .')' | ||
| ); |
There was a problem hiding this comment.
this also should let their own authored comments through
| 'NOT EXISTS (' | ||
| .'SELECT 1 FROM '.HashtagBlock::class.' hb INNER JOIN '.HashtagLink::class.' hbl ON hb.hashtag = hbl.hashtag ' | ||
| .'WHERE hbl.postComment = c AND hb.user = :blocker' | ||
| .')' |
There was a problem hiding this comment.
this also should let their own authored comments through
| 'NOT EXISTS (' | ||
| .'SELECT 1 FROM '.HashtagBlock::class.' hb INNER JOIN '.HashtagLink::class.' hbl ON hb.hashtag = hbl.hashtag ' | ||
| .'WHERE hbl.post = p AND hb.user = :blocker' | ||
| .')' |
There was a problem hiding this comment.
this also should let their own authored comments through
| </header> | ||
| </div> | ||
|
|
||
| {{ component('hashtag_sub', {hashtag: hashtag}) }} |
There was a problem hiding this comment.
I also still get an exception here, maybe because the tag is not in the DB, so the tag needs to get created
| @@ -0,0 +1,26 @@ | |||
| <aside{{ attributes.defaults({class: 'domain__subscribe', 'data-controller': 'subs'}) }}> | |||
There was a problem hiding this comment.
| <aside{{ attributes.defaults({class: 'domain__subscribe', 'data-controller': 'subs'}) }}> | |
| <aside{{ attributes.defaults({class: 'hashtag__subscribe', 'data-controller': 'subs'}) }}> |
# Conflicts: # src/Controller/Tag/TagOverviewController.php # src/Repository/ContentRepository.php
Should we create hashtags on the fly on request or return a 404? Creation on user-request can be abused to flood the DB. |
|
Yes I think you're right and it could be abused 🤔 |
|
Ad-hoc creation on subscription will not work, as the subscription controller expects an entity object in its args. So I will just throw a |
| 'NOT EXISTS (' | ||
| .'SELECT 1 FROM '.HashtagBlock::class.' hb INNER JOIN '.HashtagLink::class.' hbl ON hb.hashtag = hbl.hashtag ' | ||
| .'WHERE hbl.entryComment = c AND hb.user = :blocker' | ||
| .') OR c.user = :blocker' |
There was a problem hiding this comment.
This clause filters comments returned by findByCriteria(), but nested entry replies bypass it. EntrySingleController subsequently loads descendants through hydrateChildren(), and the default tree template renders them through EntryComment::getChildrenByCriteria(). That method checks banned hashtags and word filters, but does not check the viewer’s hashtag blocks.
A reproduction on this PR head returns another user’s blocked-tag reply beneath an unblocked top-level comment. For example, if the viewer blocks #blocked, an untagged parent remains visible and another user’s child containing #blocked still appears. The same gap occurs at deeper reply levels.
Please apply the viewer’s hashtag blocks when selecting nested replies, preserving the c.user = :blocker own-author exception implemented here. Also cover the classic view: templates/components/entry_comments_nested.html.twig iterates comment.nested directly and bypasses getChildrenByCriteria(), so fixing only that method would leave the classic rendering path exposed.
Please add regression coverage for blocked replies beneath an unblocked parent, deeper descendants, the viewer’s own replies, anonymous and other viewers, and visibility after unblocking. Instance-wide hashtag bans should continue to apply independently of the own-author exception. The equivalent post-comment path needs the same treatment.
There was a problem hiding this comment.
One possible solution is to add a viewer-specific helper to src/Entity/EntryComment.php and use it in getChildrenByCriteria(). This is the approach I implemented locally; it has not been committed or pushed.
public function containsBlockedHashtags(User $loggedInUser): bool
{
if ($this->isAuthor($loggedInUser)) {
return false;
}
foreach ($this->hashtags as /** @var HashtagLink $hashtag */ $hashtag) {
if ($loggedInUser->isBlockedHashtag($hashtag->hashtag)) {
return true;
}
}
return false;
}Then include it in the existing child filter:
$children = $this->children
->matching($criteria)
->filter(fn (EntryComment $comment) => !$comment->containsBannedHashtags()
&& (!$loggedInUser || (!$comment->containsBlockedHashtags($loggedInUser)
&& !$comment->containsFilteredWords($loggedInUser, $filterRealm))))
->toArray();The author exception is confined to viewer-specific hashtag blocks. The existing banned-hashtag and word filters continue to apply independently, and anonymous viewers do not acquire another user's block list.
For the classic view, wrap the existing component call in templates/components/entry_comments_nested.html.twig with the same check, since that view iterates the flat descendant collection:
{% if view is same as constant('App\\Controller\\User\\ThemeSettingsController::CLASSIC') %}
{% for reply in comment.nested %}
{% if not app.user or not reply.containsBlockedHashtags(app.user) %}
{{ component('entry_comment', {
comment: reply,
showNested: false,
level: 3,
showEntryTitle: false,
showMagazineName: false
}) }}
{% endif %}
{% endfor %}
{% endif %}I added tests/Unit/Entity/NestedCommentHashtagBlockTest.php locally to exercise both comment types, first-level and deeper replies, own-author replies, anonymous and other viewers, unblocking, instance-wide bans, and classic template rendering. The isolated regression run passed with 2 tests and 24 assertions.
| 'NOT EXISTS (' | ||
| .'SELECT 1 FROM '.HashtagBlock::class.' hb INNER JOIN '.HashtagLink::class.' hbl ON hb.hashtag = hbl.hashtag ' | ||
| .'WHERE hbl.postComment = c AND hb.user = :blocker' | ||
| .') OR c.user = :blocker' |
There was a problem hiding this comment.
The post-comment path has the same nested-reply gap as the entry-comment path. This clause applies to findByCriteria(), but PostSingleController subsequently loads descendants with hydrateChildren(). The default tree view calls PostComment::getChildrenByCriteria(), which checks banned hashtags and word filters but does not check the viewer’s blocked hashtags.
I reproduced another user’s reply containing a blocked hashtag remaining in the returned children beneath an unblocked parent on this PR head. The SQL own-author exception here is correct, but the corresponding viewer-block rule is missing from descendant filtering.
Please apply hashtag blocking in the nested post-comment path while keeping the viewer’s own replies visible. The classic view also needs coverage: templates/components/post_comments_nested.html.twig renders comment.nested directly, so it bypasses getChildrenByCriteria().
Add regression coverage for both tree and classic rendering, including deeper replies, own-author replies, anonymous and other viewers, and unblocking. Keep instance-wide banned-hashtag filtering independent of the viewer-specific block exception.
There was a problem hiding this comment.
One possible solution is to add a viewer-specific helper to src/Entity/PostComment.php and use it in getChildrenByCriteria(). This is the approach I implemented locally; it has not been committed or pushed.
public function containsBlockedHashtags(User $loggedInUser): bool
{
if ($this->isAuthor($loggedInUser)) {
return false;
}
foreach ($this->hashtags as /** @var HashtagLink $hashtag */ $hashtag) {
if ($loggedInUser->isBlockedHashtag($hashtag->hashtag)) {
return true;
}
}
return false;
}Then include it in the existing child filter:
$children = $this->children
->matching($criteria)
->filter(fn (PostComment $comment) => !$comment->containsBannedHashtags()
&& (!$loggedInUser || (!$comment->containsBlockedHashtags($loggedInUser)
&& !$comment->containsFilteredWords($loggedInUser, $filterRealm))))
->toArray();The author exception is confined to viewer-specific hashtag blocks. The existing banned-hashtag and word filters continue to apply independently, and anonymous viewers do not acquire another user's block list.
For the classic view, wrap the existing component call in templates/components/post_comments_nested.html.twig with the same check, since that view iterates the flat descendant collection:
{% if view is same as constant('App\\Controller\\User\\ThemeSettingsController::CLASSIC') %}
{% for reply in comment.nested %}
{% if not app.user or not reply.containsBlockedHashtags(app.user) %}
{{ component('post_comment', {
comment: reply,
showNested: false,
level: 3,
criteria: criteria,
}) }}
{% endif %}
{% endfor %}
{% endif %}I added tests/Unit/Entity/NestedCommentHashtagBlockTest.php locally to exercise both comment types, first-level and deeper replies, own-author replies, anonymous and other viewers, unblocking, instance-wide bans, and classic template rendering.
This PR enables users to block and subscribe to Hashtags and list their blocks / subscriptions (in UI and API equally).
Closes #1993 and #1452