From 147ae2c5be028fc02632804f60d29992019dc918 Mon Sep 17 00:00:00 2001 From: Xavier Vello Date: Wed, 10 Jul 2024 18:29:35 +0000 Subject: [PATCH] Fix opengraph meta for wiki pages (#4427) Fixes https://codeberg.org/forgejo/forgejo/issues/4417 by adding a conditional branch to the `head_opengraph` template to match wiki pages. I tried to be consistent with the other types: - `og:title` is the wiki page title - `og:url` is built via `{{AppUrl}}{{.Link}}` like it is done for commit and file views. This has the caveat of doubling the slash (see test below). Should we `{{trimSuffix "/" AppUrl}}` to remove this, if sprig is available? - `og:description` is the repository description to match GH behaviour. Also, the first sentences of the page might not be descriptive enough. Should we prefix the repo description with the repo name? - `og:type` and `og:image` are common Added a `TestOpenGraphProperties` integration test using existing fixtures. Coverage is not 100% but can be improved later. ## Output on a test repo ```html ``` Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/4427 Co-authored-by: Xavier Vello Co-committed-by: Xavier Vello --- release-notes/9.0.0/fix/4427.md | 1 + templates/base/head_opengraph.tmpl | 6 ++ tests/integration/opengraph_test.go | 150 ++++++++++++++++++++++++++++ 3 files changed, 157 insertions(+) create mode 100644 release-notes/9.0.0/fix/4427.md create mode 100644 tests/integration/opengraph_test.go diff --git a/release-notes/9.0.0/fix/4427.md b/release-notes/9.0.0/fix/4427.md new file mode 100644 index 0000000000..3556a8f13d --- /dev/null +++ b/release-notes/9.0.0/fix/4427.md @@ -0,0 +1 @@ +Fixed social media previews for links to wiki pages. diff --git a/templates/base/head_opengraph.tmpl b/templates/base/head_opengraph.tmpl index c02adabaab..292c3bdd92 100644 --- a/templates/base/head_opengraph.tmpl +++ b/templates/base/head_opengraph.tmpl @@ -24,6 +24,12 @@ {{- end -}} {{end}} + {{else if .Pages}} + + + {{if .Repository.Description}} + + {{end}} {{else}} diff --git a/tests/integration/opengraph_test.go b/tests/integration/opengraph_test.go new file mode 100644 index 0000000000..8d29e4548d --- /dev/null +++ b/tests/integration/opengraph_test.go @@ -0,0 +1,150 @@ +// Copyright 2024 The Forgejo Authors c/o Codeberg e.V.. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "net/http" + "testing" + + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/tests" + + "github.com/PuerkitoBio/goquery" + "github.com/stretchr/testify/assert" +) + +func TestOpenGraphProperties(t *testing.T) { + defer tests.PrepareTestEnv(t)() + siteName := "Forgejo: Beyond coding. We Forge." + + cases := []struct { + name string + url string + expected map[string]string + }{ + { + name: "website root", + url: "/", + expected: map[string]string{ + "og:title": siteName, + "og:url": setting.AppURL, + "og:description": "Forgejo is a self-hosted lightweight software forge. Easy to install and low maintenance, it just does the job.", + "og:type": "website", + "og:image": "/assets/img/logo.png", + "og:site_name": siteName, + }, + }, + { + name: "profile page without description", + url: "/user30", + expected: map[string]string{ + "og:title": "User Thirty", + "og:url": setting.AppURL + "user30", + "og:type": "profile", + "og:image": "https://secure.gravatar.com/avatar/eae1f44b34ff27284cb0792c7601c89c?d=identicon", + "og:site_name": siteName, + }, + }, + { + name: "profile page with description", + url: "/the_34-user.with.all.allowedchars", + expected: map[string]string{ + "og:title": "the_1-user.with.all.allowedChars", + "og:url": setting.AppURL + "the_34-user.with.all.allowedChars", + "og:description": "some [commonmark](https://commonmark.org/)!", + "og:type": "profile", + "og:image": setting.AppURL + "avatars/avatar34", + "og:site_name": siteName, + }, + }, + { + name: "issue", + url: "/user2/repo1/issues/1", + expected: map[string]string{ + "og:title": "issue1", + "og:url": setting.AppURL + "user2/repo1/issues/1", + "og:description": "content for the first issue", + "og:type": "object", + "og:image": "https://secure.gravatar.com/avatar/ab53a2911ddf9b4817ac01ddcd3d975f?d=identicon", + "og:site_name": siteName, + }, + }, + { + name: "pull request", + url: "/user2/repo1/pulls/2", + expected: map[string]string{ + "og:title": "issue2", + "og:url": setting.AppURL + "user2/repo1/pulls/2", + "og:description": "content for the second issue", + "og:type": "object", + "og:image": "https://secure.gravatar.com/avatar/ab53a2911ddf9b4817ac01ddcd3d975f?d=identicon", + "og:site_name": siteName, + }, + }, + { + name: "file in repo", + url: "/user27/repo49/src/branch/master/test/test.txt", + expected: map[string]string{ + "og:title": "repo49/test/test.txt at master", + "og:url": setting.AppURL + "/user27/repo49/src/branch/master/test/test.txt", + "og:type": "object", + "og:image": "https://secure.gravatar.com/avatar/7095710e927665f1bdd1ced94152f232?d=identicon", + "og:site_name": siteName, + }, + }, + { + name: "wiki page for repo without description", + url: "/user2/repo1/wiki/Page-With-Spaced-Name", + expected: map[string]string{ + "og:title": "Page With Spaced Name", + "og:url": setting.AppURL + "/user2/repo1/wiki/Page-With-Spaced-Name", + "og:type": "object", + "og:image": "https://secure.gravatar.com/avatar/ab53a2911ddf9b4817ac01ddcd3d975f?d=identicon", + "og:site_name": siteName, + }, + }, + { + name: "index page for repo without description", + url: "/user2/repo1", + expected: map[string]string{ + "og:title": "repo1", + "og:url": setting.AppURL + "user2/repo1", + "og:type": "object", + "og:image": "https://secure.gravatar.com/avatar/ab53a2911ddf9b4817ac01ddcd3d975f?d=identicon", + "og:site_name": siteName, + }, + }, + { + name: "index page for repo with description", + url: "/user27/repo49", + expected: map[string]string{ + "og:title": "repo49", + "og:url": setting.AppURL + "user27/repo49", + "og:description": "A wonderful repository with more than just a README.md", + "og:type": "object", + "og:image": "https://secure.gravatar.com/avatar/7095710e927665f1bdd1ced94152f232?d=identicon", + "og:site_name": siteName, + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + req := NewRequest(t, "GET", tc.url) + resp := MakeRequest(t, req, http.StatusOK) + doc := NewHTMLParser(t, resp.Body) + + foundProps := make(map[string]string) + doc.Find("head meta[property^=\"og:\"]").Each(func(_ int, selection *goquery.Selection) { + prop, foundProp := selection.Attr("property") + assert.True(t, foundProp) + content, foundContent := selection.Attr("content") + assert.True(t, foundContent, "opengraph meta tag without a content property") + foundProps[prop] = content + }) + + assert.EqualValues(t, tc.expected, foundProps, "mismatching opengraph properties") + }) + } +}