From c131e471764c4e4316bd68df45c5f212a4bdfc15 Mon Sep 17 00:00:00 2001 From: Hugo Sales Date: Tue, 7 Dec 2021 19:53:56 +0000 Subject: [PATCH] [COMPONENT][Tag] Remove wrong canonicalization of tags in tag feed controller. Fix display of original tags --- components/Tag/Controller/Tag.php | 49 ++++++++----------- components/Tag/Tag.php | 23 +++++++-- .../Tag/templates/actor_tag_feed.html.twig | 31 +++++++----- .../Tag/templates/note_tag_feed.html.twig | 35 +++++++------ plugins/RelatedTags/RelatedTags.php | 10 ++-- src/Entity/NoteTag.php | 2 +- 6 files changed, 86 insertions(+), 64 deletions(-) diff --git a/components/Tag/Controller/Tag.php b/components/Tag/Controller/Tag.php index 172a91f83e..eea9eed65d 100644 --- a/components/Tag/Controller/Tag.php +++ b/components/Tag/Controller/Tag.php @@ -8,77 +8,70 @@ use App\Core\Cache; use App\Core\Controller; use App\Util\Common; use Component\Tag\Tag as CompTag; -use Functional as F; class Tag extends Controller { - private function process(string|array $tag_or_tags, callable $key, string $query, string $template) + private function process(string|array $canon_single_or_multi, null|string|array $tag_single_or_multi, string $key, string $query, string $template) { $actor = Common::actor(); $page = $this->int('page') ?: 1; $lang = $this->string('lang'); - if (\is_null($lang)) { - $langs = $actor->getPreferredLanguageChoices(); - $lang = $langs[array_key_first($langs)]; - } - if (\is_string($tag_or_tags)) { - $canonical = CompTag::canonicalTag($tag_or_tags, $lang); - } else { - $canonical = F\map($tag_or_tags, fn ($t) => CompTag::canonicalTag($t, $lang)); - } + $results = Cache::pagedStream( - key: $key($canonical), + key: $key, query: $query, - query_args: ['canon' => $canonical], + query_args: ['canon' => $canon_single_or_multi], actor: $actor, page: $page, ); return [ '_template' => $template, - 'tag_name' => $canonical, + 'tag_name' => $tag_single_or_multi, 'results' => $results, 'page' => $page, ]; } - public function single_note_tag(string $tag) + public function single_note_tag(string $canon) { return $this->process( - tag_or_tags: $tag, - key: fn ($canonical) => "note-tag-feed-{$canonical}", + canon_single_or_multi: $canon, + tag_single_or_multi: $this->string('tag'), + key: CompTag::cacheKeys($canon)['note_single'], query: 'select n from note n join note_tag nt with n.id = nt.note_id where nt.canonical = :canon order by nt.created DESC, nt.note_id DESC', template: 'note_tag_feed.html.twig', ); } - public function multi_note_tags(string $tags) + public function multi_note_tags(string $canons) { - $tags = explode(',', $tags); return $this->process( - tag_or_tags: $tags, - key: fn ($canonical) => 'note-tags-feed-' . implode('-', $canonical), + canon_single_or_multi: explode(',', $canons), + tag_single_or_multi: !\is_null($this->string('tags')) ? explode(',', $this->string('tags')) : null, + key: CompTag::cacheKeys(str_replace(',', '-', $canons))['note_multi'], query: 'select n from note n join note_tag nt with n.id = nt.note_id where nt.canonical in (:canon) order by nt.created DESC, nt.note_id DESC', template: 'note_tag_feed.html.twig', ); } - public function single_actor_tag(string $tag) + public function single_actor_tag(string $canon) { return $this->process( - tag_or_tags: $tag, - key: fn ($canonical) => "actor-tag-feed-{$canonical}", + canon_single_or_multi: $canon, + tag_single_or_multi: $this->string('tag'), + key: CompTag::cacheKeys($canon)['actor_single'], query: 'select a from actor a join actor_tag at with a.id = at.tagged where at.canonical = :canon order by at.modified DESC', template: 'actor_tag_feed.html.twig', ); } - public function multi_actor_tag(string $tags) + public function multi_actor_tag(string $canons) { - $tags = explode(',', $tags); return $this->process( - tag_or_tags: $tags, - key: fn ($canonical) => 'actor-tags-feed-' . implode('-', $canonical), + canon_single_or_multi: explode(',', $canons), + tag_single_or_multi: !\is_null($this->string('tags')) ? explode(',', $this->string('tags')) : null, + key: CompTag::cacheKeys(str_replace(',', '-', $canons))['actor_multi'], query: 'select a from actor a join actor_tag at with a.id = at.tagged where at.canonical = :canon order by at.modified DESC', template: 'actor_tag_feed.html.twig', ); diff --git a/components/Tag/Tag.php b/components/Tag/Tag.php index d5abfff274..63b5d64013 100644 --- a/components/Tag/Tag.php +++ b/components/Tag/Tag.php @@ -57,10 +57,10 @@ class Tag extends Component public function onAddRoute($r): bool { - $r->connect('single_note_tag', '/note-tag/{tag<' . self::TAG_SLUG_REGEX . '>}', [Controller\Tag::class, 'single_note_tag']); - $r->connect('multiple_note_tags', '/note-tags/{tags<(' . self::TAG_SLUG_REGEX . ',)+' . self::TAG_SLUG_REGEX . '>}', [Controller\Tag::class, 'multi_note_tags']); - $r->connect('single_actor_tag', '/actor-tag/{tag<' . self::TAG_SLUG_REGEX . '>}', [Controller\Tag::class, 'single_actor_tag']); - $r->connect('multiple_actor_tags', '/actor-tags/{tags<(' . self::TAG_SLUG_REGEX . ',)+' . self::TAG_SLUG_REGEX . '>}', [Controller\Tag::class, 'multi_actor_tags']); + $r->connect('single_note_tag', '/note-tag/{canon<' . self::TAG_SLUG_REGEX . '>}', [Controller\Tag::class, 'single_note_tag']); + $r->connect('multi_note_tags', '/note-tags/{canons<(' . self::TAG_SLUG_REGEX . ',)+' . self::TAG_SLUG_REGEX . '>}', [Controller\Tag::class, 'multi_note_tags']); + $r->connect('single_actor_tag', '/actor-tag/{canon<' . self::TAG_SLUG_REGEX . '>}', [Controller\Tag::class, 'single_actor_tag']); + $r->connect('multi_actor_tags', '/actor-tags/{canons<(' . self::TAG_SLUG_REGEX . ',)+' . self::TAG_SLUG_REGEX . '>}', [Controller\Tag::class, 'multi_actor_tags']); return Event::next; } @@ -83,6 +83,9 @@ class Tag extends Component ])); Cache::pushList("tag-{$canonical_tag}", $note); $processed_tags = true; + foreach (self::cacheKeys($canonical_tag) as $key) { + Cache::delete($key); + } } if ($processed_tags) { DB::flush(); @@ -96,11 +99,21 @@ class Tag extends Component return Event::next; } + public static function cacheKeys(string $canon_single_or_multi): array + { + return [ + 'note_single' => "note-tag-feed-{$canon_single_or_multi}", + 'note_multi' => "note-tags-feed-{$canon_single_or_multi}", + 'actor_single' => "actor-tag-feed-{$canon_single_or_multi}", + 'actor_multi' => "actor-tags-feed-{$canon_single_or_multi}", + ]; + } + private static function tagLink(string $tag, ?string $language): string { $tag = self::ensureLength($tag); $canonical = self::canonicalTag($tag, $language); - $url = Router::url('single_note_tag', !\is_null($language) ? ['tag' => $canonical, 'lang' => $language] : ['tag' => $canonical]); + $url = Router::url('single_note_tag', !\is_null($language) ? ['canon' => $canonical, 'lang' => $language, 'tag' => $tag] : ['canon' => $canonical, 'tag' => $tag]); return HTML::html(['a' => ['attrs' => ['href' => $url, 'title' => $tag, 'rel' => 'tag'], $tag]], options: ['indent' => false]); } diff --git a/components/Tag/templates/actor_tag_feed.html.twig b/components/Tag/templates/actor_tag_feed.html.twig index b7c06eebfd..54d001927f 100644 --- a/components/Tag/templates/actor_tag_feed.html.twig +++ b/components/Tag/templates/actor_tag_feed.html.twig @@ -1,22 +1,29 @@ {% extends 'base.html.twig' %} {% block stylesheets %} - {{ parent() }} - + {{ parent() }} + {% endblock stylesheets %} {% block body %} -

{{ 'Actors containing tag:' | trans }} {{ tag_name }}

+ {% if tag_name is defined and tag_name is not null %} + {% if tag_name is instanceof('string') %} +

{% trans %}Notes with tag: %tag_name%{% endtrans %}

+ {% else %} + {% set tags = tag_name|join(',') %} {# TODO Not ideal, hard to translate #} +

{% trans %}Notes with tags: %tags%{% endtrans %}

+ {% endif %} + {% endif %} - {% for pinned in handle_event('AddPinnedFeedContent', app.request) %} - {% include pinned['template'] with { 'actor_tags': pinned['vars']} only %} - {% endfor %} + {% for pinned in handle_event('AddPinnedFeedContent', app.request) %} + {% include pinned['template'] with { 'actor_tags': pinned['vars']} only %} + {% endfor %} - {% for actor in results %} - {% block profile_view %}{% include 'cards/profile/view.html.twig' %}{% endblock profile_view %} - {% endfor %} + {% for actor in results %} + {% block profile_view %}{% include 'cards/profile/view.html.twig' %}{% endblock profile_view %} + {% endfor %} -
- {{ "Page: " ~ page }} -
+
+ {{ "Page: " ~ page }} +
{% endblock %} diff --git a/components/Tag/templates/note_tag_feed.html.twig b/components/Tag/templates/note_tag_feed.html.twig index 61644da68c..9a4788de02 100644 --- a/components/Tag/templates/note_tag_feed.html.twig +++ b/components/Tag/templates/note_tag_feed.html.twig @@ -2,24 +2,31 @@ {% import '/cards/note/view.html.twig' as noteView %} {% block stylesheets %} - {{ parent() }} - + {{ parent() }} + {% endblock stylesheets %} {% block body %} -

{{ 'Notes containing tag:' | trans }} {{ tag_name }}

+ {% if tag_name is defined and tag_name is not null %} + {% if tag_name is instanceof('string') %} +

{% trans %}People with tag: %tag_name%{% endtrans %}

+ {% else %} + {% set tags = tag_name|join(', ') %} {# TODO Not ideal, hard to translate #} +

{% trans %}People with tags: %tags%{% endtrans %}

+ {% endif %} + {% endif %} - {% for pinned in handle_event('AddPinnedFeedContent', app.request) %} - {% include pinned['template'] with { 'note_tags': pinned['vars']} only %} - {% endfor %} + {% for pinned in handle_event('AddPinnedFeedContent', app.request) %} + {% include pinned['template'] with { 'note_tags': pinned['vars']} only %} + {% endfor %} - {% for note in results %} - {% block current_note %} - {{ noteView.macro_note(note) }} - {% endblock current_note %} - {% endfor %} + {% for note in results %} + {% block current_note %} + {{ noteView.macro_note(note) }} + {% endblock current_note %} + {% endfor %} -
- {{ "Page " ~ page }} -
+
+ {{ "Page " ~ page }} +
{% endblock %} diff --git a/plugins/RelatedTags/RelatedTags.php b/plugins/RelatedTags/RelatedTags.php index 272ebd34d1..7bc532a998 100644 --- a/plugins/RelatedTags/RelatedTags.php +++ b/plugins/RelatedTags/RelatedTags.php @@ -37,12 +37,13 @@ class RelatedTags extends Plugin */ public function onAddPinnedFeedContent(Request $request, array &$pinned) { - $tags = $request->attributes->get('tags'); - $tags = !\is_null($tags) ? explode('-', $tags) : [$request->attributes->get('tag')]; + $tags = $request->attributes->get('canons'); + $tags = !\is_null($tags) ? explode(',', $tags) : [$request->attributes->get('canon')]; + switch ($request->attributes->get('_route')) { case 'single_note_tag': // fall-through - case 'multi_note_tag': + case 'multi_note_tags': $related = Cache::getList( 'related-note-tags-' . implode('-', $tags), fn () => DB::sql( @@ -59,9 +60,10 @@ class RelatedTags extends Plugin ); $pinned[] = ['template' => 'related_tags/note_tags.html.twig', 'vars' => $related]; break; + case 'single_actor_tag': // fall-through - case 'multi_actor_tag': + case 'multi_actor_tags': $related = Cache::getList( 'related-actor-tags-' . implode('-', $tags), fn () => DB::sql( diff --git a/src/Entity/NoteTag.php b/src/Entity/NoteTag.php index a99ebc140a..0cd1e9adea 100644 --- a/src/Entity/NoteTag.php +++ b/src/Entity/NoteTag.php @@ -125,7 +125,7 @@ class NoteTag extends Entity public function getUrl(?Actor $actor = null): string { - $params = ['tag' => $this->getCanonical()]; + $params = ['canon' => $this->getCanonical(), 'tag' => $this->getTag()]; if (!\is_null($actor)) { $params['lang'] = $actor->getTopLanguage()->getLocale(); }