In some situations it could happen that `git annex p2phttp` needs some
kind of maintenance work resulting in a commit, but without a configured
git identity p2phttp would refuse to run. This could break p2phttp
support.
Setting `GIT_AUTHOR_{NAME,EMAIL}` should remedy this issue.
Fixes#69.
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests
- I added test coverage for Go changes...
- [ ] in their respective `*_test.go` for unit tests.
- [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
- [ ] in `web_src/js/*.test.js` if it can be unit tested.
- [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [x] I do not want this change to show in the release notes.
- [ ] I want the title to show in the release notes with a link to this pull request.
- [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.
Reviewed-on: https://codeberg.org/forgejo-aneksajo/forgejo-aneksajo/pulls/70
Co-authored-by: Matthias Riße <m.risse@fz-juelich.de>
Co-committed-by: Matthias Riße <m.risse@fz-juelich.de>
This makes it such that annexed files are treated like plain git files
in comparisons (e.g. the diff of a commit).
It also changes the image diff viewer to show a more reasonable error
message when one of the annexed files under comparison is missing.
Fixes#56.
Reviewed-on: https://codeberg.org/forgejo-aneksajo/forgejo-aneksajo/pulls/57
Co-authored-by: Matthias Riße <m.risse@fz-juelich.de>
Co-committed-by: Matthias Riße <m.risse@fz-juelich.de>
Otherwise, git-annex tries to use its own default port (9417) and fails.
Fixes#52.
Reviewed-on: https://codeberg.org/forgejo-aneksajo/forgejo-aneksajo/pulls/55
Co-authored-by: Matthias Riße <m.risse@fz-juelich.de>
Co-committed-by: Matthias Riße <m.risse@fz-juelich.de>
This adds a new endpoint under `/git-annex-p2phttp` which acts as an
authenticating proxy to git-annex' p2phttp server. This makes it
possible to set `annex+<server-url>/git-annex-p2phttp` as
`remote.<name>.annexurl` and use git-annex fully over http(s) with the
normal credentials and access tokens provided by Forgejo.
Fixes#25.
Reviewed-on: https://codeberg.org/matrss/forgejo-aneksajo/pulls/42
Co-authored-by: Matthias Riße <m.risse@fz-juelich.de>
Co-committed-by: Matthias Riße <m.risse@fz-juelich.de>
Git-annex emits HEAD requests for keys while trying to drop them from a
repository that was cloned via http. Forgejo asked for authentication
for these HEAD requests. This meant that cloning and getting files was
possible without authentication, but dropping was not.
Since the response to a HEAD request is a subset of the response to a
GET request it is safe to make those unauthenticated as well. That is
what this change does, although limited to the
:username/:reponame/annex/objects endpoint.
Fixes#40.
Reviewed-on: https://codeberg.org/matrss/forgejo-aneksajo/pulls/41
Co-authored-by: Matthias Riße <m.risse@fz-juelich.de>
Co-committed-by: Matthias Riße <m.risse@fz-juelich.de>
Previously, an external renderer that matched on an annexed file would
only see its content streamed via `STDIN`, or a temporary file with a copy
of its content would be generated and passed-by-filepath (with
`IS_INPUT_FILE=true`). Whether that happens, is also subject to
`MAX_DISPLAY_FILE_SIZE` (which defaults to 8MB).
This was problematic, because annexed files tend to be large. Moreover,
if present, they already exist as write-protected files on the
file-system. Creating a copy is both expensive and serves no particular
purpose.
This commit changes how external renderers are called.
1) With `IS_INPUT_FILE=true`, the renderer is passed the true location
of an annex key, if present, and an empty path, if not.
2) The original, repository-relative path of the rendering target is
made available to the renderer via the `GITEA_RELATIVE_PATH`
environment variable.
To achieve a lean implementation, the `Blob` of the rendering target
is passed on to the `RenderContext` (because the implementation of
the annex-related functionality is centered on this dtype.
This change makes it less costly to increase `MAX_DISPLAY_FILE_SIZE`,
in order to make large, annexed files eligible for markup rendering,
because no content copies will be made any longer.
External renderers can now use the original file path, with the full
original filename, including extensions, for decision making. For
example, to detect particular compression formats based in a file name
extension, or to alter the rendering based on contextual information
encoded in the file path (e.g., a multi-file data structure with a
particular organization pattern).
Apart from the additional environment variable, there is no change to
the handling of renderers that take their input via `STDIN` (i.e.,
`IS_INPUT_FILE=false`).
Fixes#35.
Reviewed-on: https://codeberg.org/matrss/forgejo-aneksajo/pulls/36
Reviewed-by: matrss <matrss@noreply.codeberg.org>
Co-authored-by: Michael Hanke <michael.hanke@gmail.com>
Co-committed-by: Michael Hanke <michael.hanke@gmail.com>
Previously, trying to view files that were annexed, but missing, just
led to an uninformative error 500. This was rather confusing.
With these changes it now shows the pointer target instead of the
(missing) content of the file, and also indicates this situation in the
"stored with git-annex" message. For semantic correctness views for
missing files return a 404 instead of a 200, as they would with the
content present.
Fixes#7, fixes#13.
Reviewed-on: https://codeberg.org/matrss/forgejo-aneksajo/pulls/28
Co-authored-by: Matthias Riße <m.risse@fz-juelich.de>
Co-committed-by: Matthias Riße <m.risse@fz-juelich.de>
This updates the repo index/file view endpoints so annex files match the way
LFS files are rendered, making annexed files accessible via the web instead of
being black boxes only accessible by git clone.
This mostly just duplicates the existing LFS logic. It doesn't try to combine itself
with the existing logic, to make merging with upstream easier. If upstream ever
decides to accept, I would like to try to merge the redundant logic.
The one bit that doesn't directly copy LFS is my choice to hide annex-symlinks.
LFS files are always _pointer files_ and therefore always render with the "file"
icon and no special label, but annex files come in two flavours: symlinks or
pointer files. I've conflated both kinds to try to give a consistent experience.
The tests in here ensure the correct download link (/media, from the last PR)
renders in both the toolbar and, if a binary file (like most annexed files will be),
in the main pane, but it also adds quite a bit of code to make sure text files
that happen to be annexed are dug out and rendered inline like LFS files are.
Previously, Gitea's LFS support allowed direct-downloads of LFS content,
via http://$HOSTNAME:$PORT/$USER/$REPO/media/branch/$BRANCH/$FILE
Expand that grace to git-annex too. Now /media should provide the
relevant *content* from the .git/annex/objects/ folder.
This adds tests too. And expands the tests to try symlink-based annexing,
since /media implicitly supports both that and pointer-file-based annexing.
Usage of `path` was replaced by `path/filepath` in upstream forgejo, and
it made sense to use that as well where `path` was previously used. The
`setHeaderCacheForever` function and the `sendFile` method had their
signature changed.
This makes HTTP symmetric with SSH clone URLs.
This gives us the fancy feature of _anonymous_ downloads,
so people can access datasets without having to set up an
account or manage ssh keys.
Previously, to access "open access" data shared this way,
users would need to:
1. Create an account on gitea.example.com
2. Create ssh keys
3. Upload ssh keys (and make sure to find and upload the correct file)
4. `git clone git@gitea.example.com:user/dataset.git`
5. `cd dataset`
6. `git annex get`
This cuts that down to just the last three steps:
1. `git clone https://gitea.example.com/user/dataset.git`
2. `cd dataset`
3. `git annex get`
This is significantly simpler for downstream users, especially for those
unfamiliar with the command line.
Unfortunately there's no uploading. While git-annex supports uploading
over HTTP to S3 and some other special remotes, it seems to fail on a
_plain_ HTTP remote. See https://github.com/neuropoly/gitea/issues/7
and https://git-annex.branchable.com/forum/HTTP_uploads/#comment-ce28adc128fdefe4c4c49628174d9b92.
This is not a major loss since no one wants uploading to be anonymous anyway.
To support private repos, I had to hunt down and patch a secret extra security
corner that Gitea only applies to HTTP for some reason (services/auth/basic.go).
This was guided by https://git-annex.branchable.com/tips/setup_a_public_repository_on_a_web_site/
Fixes https://github.com/neuropoly/gitea/issues/3
Co-authored-by: Mathieu Guay-Paquet <mathieu.guaypaquet@polymtl.ca>
**Backport:** https://codeberg.org/forgejo/forgejo/pulls/7167
- Replies to pending review comments no longer generate a notification, this was caused by an incomplete determination if the comment was part of the pending review or not.
- The logic was reworked to do the following if it's part of a pending review: It is not a single review and if it's a reply then the comment it is replying to is part of a pending review.
- Added integration test.
- Resolvesforgejo/forgejo#7151
Co-authored-by: Gusted <postmaster@gusted.xyz>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7178
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
Co-committed-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
**Backport:** https://codeberg.org/forgejo/forgejo/pulls/7004
- Add a new function `IsPushMirrorURLAllowed` that will allow `ssh://` url and make the existing `IsMigrateURLAllowed` not allow such URLs anymore.
- Resolvesforgejo/forgejo#6960
- Existing integration tests make sure that SSH urls are still allowed for the push mirror feature and added unit test to ensure that `IsMigrateURLAllowed` no longer allows SSH urls.
Co-authored-by: Gusted <postmaster@gusted.xyz>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7018
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
Co-committed-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
**Backport:** https://codeberg.org/forgejo/forgejo/pulls/6937
- 1ce33aa38d extended the LTA table with a purpose column so it could be extended to other tokens. However some are single-use tokens and should be deleted after use.
- This did not result in a good UX for activating user as they needed to also fill in their passwords and in the case that the password was incorrect the token would no longer be usable.
- This patch modifies the code to allow for a little delay before deleting authorization tokens to do additional verification such as the password check. This cannot be done before the authorization token check as that the authorization token determines who the user is.
- Resolvesforgejo/forgejo#6912
- Adjusted existing unit test.
Co-authored-by: Gusted <postmaster@gusted.xyz>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6976
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
Co-committed-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
**Backport:** https://codeberg.org/forgejo/forgejo/pulls/6680
- Make it such that `[service].ENABLE_INTERNAL_SIGNIN = false` disables the forgotten password prompt on the login page.
Co-authored-by: davrot <davrot@noreply.codeberg.org>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6930
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
Co-committed-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
- Do an access check when loading issues for a project column, currently
this is not done and exposes the title, labels and existence of a
private issue that the viewer of the project board may not have access
to.
- The number of issues cannot be calculated in a efficient manner
and stored in the database because their number may vary depending on
the visibility of the repositories participating in the project. The
previous implementation used the pre-calculated numbers stored in each
project, which did not reflect that potential variation.
- The code is derived from https://github.com/go-gitea/gitea/pull/22865
(cherry picked from commit 2193afaeb9954a5778f5a47aafd0e6fbbf48d000)
The web route to update and delete variables of runners did not check if
the ID that was given belonged to the context it was requested in, this
made it possible to update and delete every existing runner variable of
a instance for any authenticated user.
The code has been reworked to always take into account the context of
the request (owner and repository ID).
The web route to delete action runners did not check if the ID that was
given belonged to the context it was requested in, this made it possible
to delete every existing runner of a instance by a authenticated user.
The code was reworked to ensure that the caller of the delete
runner function retrieved the runner by ID and then checks if it belongs
to the context it was requested in, although this is not an optimal
solution it is consistent with the context checking of other code for
runners.
**Backport:** https://codeberg.org/forgejo/forgejo/pulls/6708
- Currently, the explore/organizations page always defaults to using "newest" as its sort. Instead, use the pre-existing config option (`setting.UI.ExploreDefaultSort`) so server administrators can change the default sort order.
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests
(Sorry, not sure how to add a test for this change)
- I added test coverage for Go changes...
- [ ] in their respective `*_test.go` for unit tests.
- [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.
### Release notes
I don't mind either way.
- [ ] I do not want this change to show in the release notes.
- [x] I want the title to show in the release notes with a link to this pull request.
- [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.
Co-authored-by: John Moon <john.moon@vts-i.com>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6749
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
Co-committed-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
**Backport:** https://codeberg.org/forgejo/forgejo/pulls/6488
This is no longer possible in future go-git versions, so lets hardcode it
Co-authored-by: Gusted <postmaster@gusted.xyz>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6492
Co-authored-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
Co-committed-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
This commit allows chaning default branch update style through global
and repository settings. The setting affects "Update branch" button
in PR view (button shows when some commits are ahead of master branch).
When default update style is set to "rebase", dropdown button updates branch
by rebase by default. When update style is set to other value, dropdown button
updates branch by merge. Any of these actions may be selected using dropdown
in any case.
Signed-off-by: George Bartolomey <george@bh4.ru>
Added support for searching content in a specific directory or file.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6143
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
Co-authored-by: Shiny Nematoda <snematoda.751k2@aleeas.com>
Co-committed-by: Shiny Nematoda <snematoda.751k2@aleeas.com>
This commit has a fundamental flaw, in order to syncronize if external
users are still active the commit checks if the refresh token is
accepted by the OAuth provider, if that is not the case it sees that as
the user is disabled and sets the is active field to `false` to signal
that. Because it might be possible (this commit makes this a highly
likelyhood) that the OAuth provider still recognizes this user the
commit introduces code to allow users to re-active themselves via the
oauth flow if they were disabled because of this. However this code
makes no distinction in why the user was disabled and always re-actives
the user.
Thus the reactivation via the OAuth flow allows users to bypass the
manually activation setting (`[service].REGISTER_MANUAL_CONFIRM`) or if
the admin for other reasons disabled the user.
This reverts commit 21fdd28f08.
(cherry picked from commit b32f0cdfa05c3a0e34425e1b8a5dfa8b63914a01)
Conflicts:
tests/integration/wiki_test.go
"Long-Page" is missing as well as the tests package
- I made a mistake when specifying the `FOR` clause for the index hint,
I read it as being an required argument by XORM. The [MariaDB
documention](https://mariadb.com/kb/en/use-index/) tells that it
defaults to the `FOR JOIN` clause hence why I specified `JOIN` (As can
be seen in the previous PR's SQL analyze I didn't specify the `FOR`
clause). However apparently there seems to be some wizardy going on as
we need to tell MariaDB to use this index for the `ORDER BY` clause to
actually force MariaDB to use this index over the `updated_unix` index.
However because it's not actually required by XORM to specify this
value I leave this empty as mariadb is apparently smart enough to figure
out for which type we want to use this index.
- TL;DR make this index hint actually effective for MariaDB.
- Ref: #6146
- For the notifications page the unread and pinned notifications are
gathered for doer those that and are ordered by the updated unix.
MariaDB makes a bad decision (sometimes, for most users it does not make
this decision) with this query, it uses the index for the `updated_unix`
column to speed up this query, however this is not the correct index to
be taking, if the doer does not have more than 20 (the
page size) unread and pinned notifications combined MariaDB will
traverse the whole notifications table before it realizes that there are
no more notifications to be gathered. It instead should use the index
for the `user_id` column (this is what MariaDB already does for most
users), so the list that has to be traversed is limited to the doer's
notifications which is significantly less than the whole notifications
table.
- This is a different approach than what Gitea has taken to solve this
problem, which is to add a index to the (status, userid, updated_unix)
tuple (Ref: https://github.com/go-gitea/gitea/pull/32395). Adding more
and more indexes is not a good way if we can use existing indexes to get
a query to a acceptable performance.
- The code cannot use `db.Find` as it's hard to add a index hint option
specifically for this query and not for the other instances that uses
`activities_model.FindNotificationOptions`.
- Only add a index hint for MySQL as I have not been able to test if
SQLite or PostgreSQL are smart enough to use the better index (as you
need a large enough dataset to test this meaningfully).
- Integration test added to ensure the SQL is run by all databases.
---
Performance numbers (from Codeberg's database - MariaDB
10.11.6-MariaDB-0+deb12u1):
Currently:
```sql
SELECT * FROM `notification` WHERE notification.user_id=26734 AND (notification.status=3 OR notification.status=1) ORDER BY notification.updated_unix DESC LIMIT 20;
(5.731 sec)
+------+-------------+--------------+-------+--------------------------------------------------+-------------------------------+---------+-------+---------+------------+----------+------------+-------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+--------------+-------+--------------------------------------------------+-------------------------------+---------+-------+---------+------------+----------+------------+-------------+
| 1 | SIMPLE | notification | index | IDX_notification_status,IDX_notification_user_id | IDX_notification_updated_unix | 8 | const | 1376836 | 1474066.00 | 50.03 | 0.00 | Using where |
+------+-------------+--------------+-------+--------------------------------------------------+-------------------------------+---------+-------+---------+------------+----------+------------+-------------+
```
Using the better index:
```sql
SELECT * FROM `notification` USE INDEX (IDX_notification_user_id) WHERE notification.user_id=26734 AND (notification.status=3 OR notification.status=1) ORDER BY notification.updated_unix DESC LIMIT 20;
(0.834 sec)
+------+-------------+--------------+--------+----------------------------------------------------------+--------------------------+---------+----------------------------------+-------+----------+----------+------------+----------------------------------------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+--------------+--------+----------------------------------------------------------+--------------------------+---------+----------------------------------+-------+----------+----------+------------+----------------------------------------------+
| 1 | PRIMARY | notification | ref | PRIMARY,IDX_notification_status,IDX_notification_user_id | IDX_notification_user_id | 8 | const | 22042 | 10756.00 | 50.03 | 0.02 | Using where; Using temporary; Using filesort |
| 1 | PRIMARY | notification | eq_ref | PRIMARY | PRIMARY | 8 | gitea_production.notification.id | 1 | 1.00 | 100.00 | 100.00 | |
+------+-------------+--------------+--------+----------------------------------------------------------+--------------------------+---------+----------------------------------+-------+----------+----------+------------+----------------------------------------------+
```
This fixes a TODO in the code to validate the RedirectURIs when adding
or editing an OAuth application in user settings.
This also includes a refactor of the user settings tests to only create
the DB once per top-level test to avoid reloading fixtures.
(cherry picked from commit 16a7d343d78807e39df124756e5d43a69a2203a3)
Conflicts:
services/forms/user_form.go
tests/integration/user_settings_test.go
simple conflicts
Fix#29654Fix#32481
(cherry picked from commit 703be6bf307ed19ce8dc8cd311d24aeb6e5b9861)
Conflicts:
routers/api/v1/repo/file.go
routers/web/repo/repo.go
services/repository/archiver/archiver.go
services/repository/archiver/archiver_test.go
trivial context conflicts
add missing function PathParam skipped in a very large refactor
- Although sorting can be used to make the doer the first user of the
list, this isn't optimal and can be instead done with a linear search,
remove that entry and add the doer to the front of the slice.
- Extra unit test added.
Setting ENABLE_INTERNAL_SIGNIN to false will disable the built-in
signin form, should the administrator prefer to limit users to SSO.
Continuation of forgejo/forgejo#6076
## Overview
Hi all, I'm a first-time contributor to Forgejo. I was looking for something interesting to contribute and the first thing that caught my attention was https://codeberg.org/forgejo/forgejo/issues/6043, a request for an enhancement to include "issue previews" when publishing links to social media platforms. As a bit of background, the way these platforms work is that they search for meta tags in the posted link's content, and if they find a meta `og:image` (along with other meta tags) they'll pull the image to include in the social media post. Forgejo currently provides an `og:image` tag but it just renders the repository or repository-owner's avatar.
This PR will render `og:image` for an issue or PR into a link to `{...}/summary-card`, which is a dynamically generated image that contains a summary of the issue.
## Design Notes
### Rendering / Rasterization
The tricky part of solving this problem is rendering an image that combines some text, some images, and some layout elements. To address this, I've created a `card` module which allows for a handful of operations:
- Create a new rendered image (a "Card")
- Add a margin to a card
- Split the card, horizontally or vertically, into two pieces with a proportional layout (eg. 70%/30%, as desired), each of which are "Cards" that render into the same root image
- Render text into a card, with line-wrapping and text-alignment capabilities
- Render an image onto a card
- Fetches an external image as safely as possible (for server-side fetch of Gravatar, etc.)
The card module can be reused to create `og:image` summary cards for any object in the future, although obviously it's capabilities are limited. The current implementation is on issues/PRs.
I considered a few alternative approaches before taking this approach, and here's why I rejected those options:
- Provide the summary card as an SVG object which could be rendered much more easily with a template file -- however, support for SVG isn't defined as positive for OpenGraph, and a quick look through some existing implementations suggest that it is not widely supported, if at all
- Rendering as HTML/CSS, or SVG, and then using an external tool to convert into a PNG (or other static) image -- this would be much nicer and easier to implement, but would require tying in some very heavy-weight dependencies
- Rendering using a more sophisticated graphics library, eg. cairo -- also would be nicer and easier to implement, but again a heavy dependency for a small functionality
As a result of the limited capabilities of the new card module, summary cards don't have icons on them (which would require SVG rasterization) or pretty status badges with colors and rounded rects. In the future if better drawing capabilities were added, the graphics could be improved, but it doesn't seem too important.
### External Avatars
In order to rasterize a user's avatar onto the summary card, it might have to be retrieved by the server from the external source (eg. Gravatar). A `fetchExternalImage` routine attempts to do this in the safest way possible to protect the server from any possible security exposure from this; (a) verifying that the content-types are acceptable, (b) ensuring that the file-size and image-size are within the safe bounds that are used for custom avatars, (c) using a very-short timeout to avoid stalling the server if an external dependency is offline.
### Caching
Summary cards are cached after rendered. This has the downside of causing updates to statuses, avatars, titles, etc. being stale on the summary card for the cache TTL. However, during testing I found that some social media engines like Mastodon will cause the summary card to be accessed a significant number of times after being referenced by a post, causing a mini-tornado of requests. The cache compensates for this to avoid server load in this situation.
### Scope
I'm considering out-of-scope:
- Summary cards on other objects (eg. repos, users) can be left for future implementation
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).
### Tests
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
- [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- ~~I added test coverage for JavaScript changes...~~ n/a, no JS changes
- [x] ~~in `web_src/js/*.test.js` if it can be unit tested.~~
- [x] ~~in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).~~
- Manual testing
- [x] Access & attach screenshots of both an issue and a pull-request's summary card; see below
- [x] Ensure reasonable (non-crash) behavior of rendering text with glyphs outside the font -- correctly rendered as replacement unicode chars
- [x] Using a public test instance, verify that og:image behavior looks good on platforms like Mastodon and BlueSky
- [x] Bluesky: ✅
- [x] Mastodon: ✅ (Note that the summary card will be requested many times as the post is federated; either each server, or each client, will fetch it itself)
- [x] OpenGraph test site (https://www.opengraph.xyz/): ✅
- [x] Discord: Looks OK ✅; needs "twitter:card" to be set to "summary_large_image" to display the large-scale image, but (a) that's probably annoying to use, (b) probably wrong because it doesn't match Twitter Card's spec for a "photographic image", and (c) don't want to encourage/continue use of vendor-specific tag
- [x] Verify cases with user avatar missing (or autogen), and repo avatar missing (falls back to repo owner avatar)
Pull request summary card:
Issue summary card:
(images to the right are the custom repo avatar, w/ fallback to the repo owner avatar)
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.
- OpenGraph capabilities are expected to work in the background without user awareness, and so there is no need for documentation to explain the capabilities for users.
### Release notes
- [ ] I do not want this change to show in the release notes.
- [x] I want the title to show in the release notes with a link to this pull request.
- [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6053
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
