- 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 | |
+------+-------------+--------------+--------+----------------------------------------------------------+--------------------------+---------+----------------------------------+-------+----------+----------+------------+----------------------------------------------+
```
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
- `GetSubModules` already solely stores the URL of the submodule and not
a `*SubModule` entry, so don't try to type assert it to be a struct.
- I am not able to pinpoint when this was regressed but if I had to
guess it might be #4941.
- Added integration test.
## Scope
Intended to fix#6099; and moves related tests to integration tests (per. request https://codeberg.org/forgejo/forgejo/pulls/6105#issuecomment-2486228)
## 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. **not applicable**
- [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes... **not applicable**
- [ ] 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.
- Rationale: bug-fix for a change that hasn't been released yet.
- [ ] 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/6113
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
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>
- Currently the TOTP secrets are stored using the `secrets` module with
as key the MD5 hash of the Secretkey, the `secrets` module uses general
bad practices. This patch migrates the secrets to use the `keying`
module (#5041) which is easier to use and use better practices to store
secrets in databases.
- Migration test added.
- Remove the Forgejo migration databases, and let the gitea migration
databases also run forgejo migration databases. This is required as the
Forgejo migration is now also touching tables that the forgejo migration
didn't create itself.
Confine the search menu to be at most the width of the page, or 80% of
the viewport width, whichever is smaller. To do this, introduce a new
`--container-width` variable for the descendant elements of
`.ui.container` to be able to access.
Also update the relevant e2e test: add a long 'lorem ipsum' page, add a
search for it, parameterize the width.
When running e2e tests on flaky networks, gravatar can cause a timeout
and test failures. Turn off, and populate avatars on e2e test suite run
to make them reliable.
(cherry picked from commit 9ac74a1a408136455a9e0586fb8e65163048597b)
Conflicts:
models/fixtures/user.yml
services/repository/contributors_graph_test.go
trivial context conflicts
Extend API MarkupOptions to contain branch path.
The `api.MarkupOptions{}` to have `BranchPath` which contains the
current branch. The `RenderMarkup` function utilizes a struct since there
are too many variables passed as arguments and that is not a good sign
for readability.
And `repo-editor.js` will contain a new form data which is `branch-path`
which will then be utilized by `edit.tmpl` as `data-branch-path`.
Closes: #4510
## 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.
- [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)).
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [ ] I did not document these changes and I do not expect someone else to do it.
### 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/5781
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Nirmal Kumar R <tildezero@gmail.com>
Co-committed-by: Nirmal Kumar R <tildezero@gmail.com>
It is possible to set a Email for a Organization. This Email is optional and only used to be displayed on the profile page. However, once you set an EMail, you can no longer remove it. This PR fixes that.
While working on the tests, I found out, that the API returns a 500 when trying to set an invalid EMail. I fixed that too. It returns a 422 now.
Fixes#4567
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/5517
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Reviewed-by: Otto <otto@codeberg.org>
Co-authored-by: JakobDev <jakobdev@gmx.de>
Co-committed-by: JakobDev <jakobdev@gmx.de>
Git has a cool feature called git notes. It allows adding a text to a commit without changing the commit itself. Forgejo already displays git notes. With this PR you can also now change git notes.
<details>
<summary>Screenshots</summary>
</details>
## Checklist
The [developer guide](https://forgejo.org/docs/next/developer/) 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.
- [x] 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
- [ ] 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.
<!--start release-notes-assistant-->
## Release notes
<!--URL:https://codeberg.org/forgejo/forgejo-->
- Features
- [PR](https://codeberg.org/forgejo/forgejo/pulls/4753): <!--number 4753 --><!--line 0 --><!--description QWxsb3cgY2hhbmdpbmcgZ2l0IG5vdGVz-->Allow changing git notes<!--description-->
<!--end release-notes-assistant-->
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/4753
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: JakobDev <jakobdev@gmx.de>
Co-committed-by: JakobDev <jakobdev@gmx.de>
Refactoring of #32211
this move the PublicOnly() filter calcuation next to the DB querys and
let it be decided by the Doer
---
*Sponsored by Kithara Software GmbH*
(cherry picked from commit 43c252dfeaf9ab03c4db3e7ac5169bc0d69901ac)
Conflicts:
models/organization/org_test.go
models/organization/org_user_test.go
routers/web/org/home.go
rather simple conflict resolution but not trivial
tests/integration/user_count_test.go had to be adapted (simple)
because it does not exist in Gitea and uses the modified model
- Only send a review request based on the code owner file if the code
owner user has read permissions to the pull requests of that repository.
- This avoids leaking title of PRs from private repository when a
CODEOWNER file is present which contains users that do not have access
to the private repository.
- Found by @oliverpool.
- Integration test added.
- Remember if fuzzy was set or not for the open/close/all states.
- Use `fuzzy=false` for test, as `fuzzy=true` is the default (this is
the opposite of all the other values).
- Remove `ctx.Link` prefix for open/close states, this makes them
suspectible to the existing tests (the other filter links are also in
the format of simply having `?xx=xx&yy=yy`).
- Fix typo in test name.
Fix#32427
(cherry picked from commit 35bcd667b23de29a7b0d0bf1090fb10961d3aca3)
Conflicts:
- tests/integration/repo_tag_test.go
Resolved by manually copying the added test, and also manually
adjusting the imported Go modules.
- Ensure that the specified push mirror ID belongs to the requested
repository, otherwise it is possible to modify the intervals of the push
mirrors that do not belong to the requested repository.
- Integration test added.
- If a repository is forked to a private or limited user/organization,
the fork should not be visible in the list of forks depending on the
doer requesting the list of forks.
- Added integration testing for web and API route.
- The RSS and atom feed for branches exposes details about the code, it
therefore should be guarded by the requirement that the doer has access
to the code of that repository.
- Added integration testing.
- This unifies the security behavior of enrolling security keys with
enrolling TOTP as a 2FA method. When TOTP is enrolled, you cannot use
basic authorization (user:password) to make API request on behalf of the
user, this is now also the case when you enroll security keys.
- The usage of access tokens are the only method to make API requests on
behalf of the user when a 2FA method is enrolled for the user.
- Integration test added.
- If the incoming mail feature is enabled, tokens are being sent with
outgoing mails. These tokens contains information about what type of
action is allow with such token (such as replying to a certain issue
ID), to verify these tokens the code uses the HMAC-SHA256 construction.
- The output of the HMAC is truncated to 80 bits, because this is
recommended by RFC2104, but RFC2104 actually doesn't recommend this. It
recommends, if truncation should need to take place, it should use
max(80, hash_len/2) of the leftmost bits. For HMAC-SHA256 this works out
to 128 bits instead of the currently used 80 bits.
- Update to token version 2 and disallow any usage of token version 1,
token version 2 are generated with 128 bits of HMAC output.
- Add test to verify the deprecation of token version 1 and a general
MAC check test.
- Add a `purpose` column, this allows the `forgejo_auth_token` table to
be used by other parts of Forgejo, while still enjoying the
no-compromise architecture.
- Remove the 'roll your own crypto' time limited code functions and
migrate them to the `forgejo_auth_token` table. This migration ensures
generated codes can only be used for their purpose and ensure they are
invalidated after their usage by deleting it from the database, this
also should help making auditing of the security code easier, as we're
no longer trying to stuff a lot of data into a HMAC construction.
-Helper functions are rewritten to ensure a safe-by-design approach to
these tokens.
- Add the `forgejo_auth_token` to dbconsistency doctor and add it to the
`deleteUser` function.
- TODO: Add cron job to delete expired authorization tokens.
- Unit and integration tests added.
- Optimize generting random files.
- Reduce big file of 128MiB to 32MiB (git was never made for large files
anyways, but simply tests that it works).
- Reduce looped git operations from 100 iterations to 10.
- Add extra print statements to know what a slow test is doing, this
also helps to see if a particular piece of code in a slow test is the
culprit or if the test is just very extensive.
- Set `[ui.notification].EVENT_SOURCE_UPDATE_TIME` to 1s to speed up
`TestEventSourceManagerRun`.
- Sneaked in some general test improvements.
- Only prepare repositories once.
- Move the repositories to temporary directories (these should usually be stored in
memory) which are recreated for each test to avoid persistentance
between tests. Doing some dirty profiling suggests that the preparing
test functions from 140-100ms to 70-40ms
Goals:
- speedup
- less flakiness
- best practices and more use
- documentation
config:
- sync ports in Makefile and playwright config
(otherwise, some tests fail locally because they assert the full URL including the (wrong) port)
- even more generous timeouts
- limit workers to one again (because I finally understand how
Playwright works)
- allow nested functions to group them together with the related test
all:
- deprecate waitForLoadState('networkidle')
- it is discouraged as per https://playwright.dev/docs/api/class-page#page-wait-for-load-state
- I could not find a usage that seems to require it actually (see
added documentation in README)
- adding an exception should be made explicitly
- it does not do what you might expect anyway in most cases
- only log in when necessary
webauthn:
- verify that login is possible after disabling key
- otherwise, the cleanup was not necessary after the previous refactor to create a fresh user each
issue-sidebar / WIP toggle:
- split into smaller chunks
- restore original state first
- add missed assertion to fix race condition (not waiting
before state was reached)
- explicitly toggle the state to detect mismatch earlier
issue-sidebar / labels:
- restore original state first
- better waiting for background request
When a new commit is pushed to an existing pull request, the update of
the commit status will happen asynchronously, via the git hook.
--- FAIL: TestPullRequestCommitStatus/synchronize (2.14s)
actions_trigger_test.go:331:
Error Trace: /workspace/forgejo/forgejo/tests/integration/actions_trigger_test.go:331
Error: Should be true
Test: TestPullRequestCommitStatus/synchronize
- Merge tests together.
- Remove unecessary usage of `onGiteaRun`.
- Make proper use of `unittest`.
- Make proper use of `test.MockVariable`.
- I have not checked all of the testing files yet.
- Do not require minio for mariadb docker.
- Do not require elasticsearch for mysql.
- Fix postgress password parameter.
- Add the multistatement query for mysql (this is not optimal but adding
Makefile code doesn't seem to work really well either)
Some preparations are only used by a few tests, so to make the tests fast, they should only be prepared when they are used.
By the way, this PR splits PrepareTestEnv into small functions to make it simple.
---
Conflict resolution: Mostly magical and just re-pasting the code into
the right places.
Done differently: use `require.NoError` instead of `assert.NoError`.
(cherry picked from commit ec2d1593c269e06655525deb96f74b8094221b6f)
When ReplaceIssueLabels calls issue.LoadLabels it was a noop because
issue.isLabelsLoaded is still set to true because of the call to
issue.LoadLabels that was done at the beginning of the function.
- This uses the `TrimSpace` preprocessing of the binding library to
remove any accidental spaces from the input.
- Integration test added.
- Resolves#4309
- Adjust `PrepareArtifactsStorage` to use `require.NoError` instead of
`assert.NoError`
- Adjust `TestActionsArtifactDownload` to have the proper order of
`assert.Equal` arguments.
Signed-off-by: Gergely Nagy <forgejo@gergo.csillger.hu>
Closes https://github.com/go-gitea/gitea/issues/30296
- Adds a DB fixture for actions artifacts
- Adds artifacts test files
- Clears artifacts test files between each run
- Note: I initially initialized the artifacts only for artifacts tests,
but because the files are small it only takes ~8ms, so I changed it to
always run in test setup for simplicity
- Fix some otherwise flaky tests by making them not depend on previous
tests
(cherry picked from commit 66971e591e5dddd5b6dc1572ac48f4e4ab29b8e0)
Conflicts:
- tests/integration/api_actions_artifact_test.go
Conflict resolved by manually changing the tested artifact
name from "artifact" to "artifact-download"
- tests/integration/api_actions_artifact_v4_test.go
Conflict resolved by manually updating the tested artifact
names, and adjusting the test case only present in our tree.
- tests/test_utils.go
Resolved by manually copying the added function.
When a workflow has
on:
pull_request:
types:
- labeled
- unlabeled
The outcome of the workflow (success or failure) must be associated
with the head sha commit status. Otherwise it cannot be used as a
requirement for merging the pull request (branch protections).
- Make use of `test.MockVariableValue` to override variables for the
duration of the test.
- Don't needlessly call `onGiteaRun`, its only needed when a HTTP server
needs to be called by the code.
- When `onGiteaRun` is used, make use of the passed parameters, such as
the passed `*testing.T` variable and `*url.URL` (this also avoids
needing to serve the routers in the test code again).
- Use `(*url.URL).JoinPath` to craft new URLs.
- Don't override `setting.AppURL` & `setting.Database.LogSQL` when its
does not affect the test.
- Add empty fixture files for `FederatedUser` & `FederationHost` so they
are truncated and do not persist between tests.
- Add a permission check that the doer has write permissions to the head
repository if the the 'delete branch after merge' is enabled when
merging a pull request.
- Unify the checks in the web and API router to `DeleteBranchAfterMerge`.
- Added integration tests.
- If you select a portion of the comment, `Quote reply` will not only
quote that portion and not copy paste the whole text as it previously
did. This is achieved by using the `@github/quote-selection` package.
- There's preprocessing to ensure Forgejo-flavored markdown syntax is
preserved.
- e2e test added.
- Resolves#1342
- Combine review requests comments similairy how labels comments are
combined. If review requests comments were made within 60 seconds of
each other they will be grouped.
- Integration and unit test added.
- Resolves#2774
- If `GetAffectedFiles` is called for a push with an empty oldCommitID,
then set the oldCommitID to the empty tree. This will effictively diff
all the changes included in the push, which is the expected behavior for
branches.
- Integration test added.
- Resolves#5683
- Port of gitea#31778 but implemented differently.
Instead of letting playwright do the full test suite and then print the
output and error, direct the output to std{our,err} for a faster
developing loop. This also makes the output colored.
Make the big move to Eslint flat config format. The outcome of Eslint
still should be the same, but some things has changed:
- `eslint-plugin-github` is dropped, flat configs have been out for a
while and most eslint plugins support it, but for no reason and no
activity in sight this plugin is likely not going to support flat config
for a while and to avoid other plugins not being able to update (as they
are requiring flat configs) drop the github rules.
- Nested configs don't work properly and are unified into the root
eslint config, this unification did cause some conflicts and thats why
the `import-x` is in a seperate 'group' to exclude targeting Vue files.
- The `eslint-plugin-i` is deprecated and `esplint-plugin-import-x` is
its succesor which has better support for flat configs, the same rules
are still applied.
The majority of the flat config was generated by
`@eslint/migrate-config` tool.
- Use the combo markdown editor for the milestone description. The
milestone description is rendered in markdown, so it makes sense to use
a 'markdown-aware' editor. This also includes the option to use
monospace font.
- Resolves#5649
- When a truncated comment is detected in the RSS/Atom feeds, fetch the
comment from the database and use the original content.
- Added integration test.
- Resolves#5650
* added a message for the case where the user's profile is private but the Public activity is not hidden
The activity is still hidden anyway because the profile is private, but previously the message would say:
`Your activity is visible to everyone, except for interactions in private spaces...`
which I would consider as a flaw of the original implementation. Now it will say:
`Your activity is only visible to you and the instance administrators because your profile is private...`
* started showing the message for admins that the activity they see should remain private in the case
where the whole profile is private, not just the activity tab. Previously it would say:
This activity is visible to everyone, but as an administrator you can also see interactions in private spaces.`
which I would also consider as a flaw of the original implementation. Now it will say:
`This activity is visible to you because you're an administrator, but the user wants it to remain private.`
* added test cases
* bumped up the number of our GPL-licensed files
Preview
For both screenshots, Forgejo would previously display misinformation.
Change 1: User viewing their private profile, but activity isn't configured as hidden
https://codeberg.org/attachments/6659c80c-15dd-48be-a379-db737fd1dd5e
Change 2: Admin viewing user's private profile
https://codeberg.org/attachments/220da57f-b658-4474-9ad2-049e8438a0af
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/5638
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
- When an admin wants syncronize tags in the Git data to the database
via the admin dashboard all annoted tags loses their title. This was
caused because the code didn't correctly handle annoted tags. Annoted
tags have their own objectID to store the annoted message, unlike
'normal' tags which point to the commitID. While the function was being
run for annoted tags, the code thought it found a mismatch in the
objectIDs, because the stored version was actually correct which pointed
to the commitID but the code found the objectID of the annoted tag.
- Make `SyncReleasesWithTags` corectly handle annoted tags.
- Added unit and integration tests.
- Resolves#5628
- Add `recentupdated` to the `OrderByMap`.
- Add integration testing for organization and user repository sorting.
- Resolves#5612
- Regression from 12e23ee199 where the
`recentupdated` case was not added to the map, but was handled
seperately as a fallback. The regression came into affect when
5a0bc35799 also relied on this map but
didn't handle the `recentupdated` case.
- Add a helper function that creates and log into a temporary user. So
it doesn't affect other users and tests and the test can more easily be
retried with a 'fresh' state instead of a broken state.
- Adjust the Webauthn test to make use of this.
- Relevant: #5291, #5394
(cherry picked from commit 6029d78ab5006e8fb4f42adb5a8c491f19fa7b0a)
Conflicts:
models/user/user.go
services/user/user_test.go
trivial context conflict
tests/integration/user_test.go
discarded entirely because dot may be allowed in Forgejo under
some conditions
- On editting a team, only update the units if the team isn't the
'Owners' team. Otherwise the 'Owners' team end up having all of their
unit access modes set to 'None'; because the request form doesn't send
over any units, as it's simply not shown in the UI.
- Adds a database inconstency check and fix for the case where the
'Owners' team is affected by this bug.
- Adds unit test.
- Adds integration test.
- Resolves#5528
- Regression of https://github.com/go-gitea/gitea/pull/24012
Port of https://github.com/go-gitea/gitea/pull/32204
(cherry picked from commit d6d3c96e6555fc91b3e2ef21f4d8d7475564bb3e)
Conflicts:
routers/api/v1/api.go
services/context/api.go
trivial context conflicts
The Issue and PullRequest list has 3 states:
- open: This lists all open Issues/PullRequests
- closed: This lists all closed Issues/PullRequests
- all: This lists all open and closed Issues/PullRequests
If you want to get to the all state, you need to click Open while in open state or Closed while in closed state, which is very unintuitive. This PR adss a third button to get to this state.
I'm not sure if the eye icon fits well, but I couldn't find a better one.
Tests will be added once #4124 is merged.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/4125
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: JakobDev <jakobdev@gmx.de>
Co-committed-by: JakobDev <jakobdev@gmx.de>
Fix#30898
we have an option `SearchByEmail`, so enable it, then we can search user
by email.
Also added a test for it.
(cherry picked from commit 5d6d025c9b8d2abca9ec2bfdc795d1f0c1c6592d)
Resolves#20475
(cherry picked from commit 7e68bc88238104d2ee8b5a877fc1ad437f1778a4)
Conflicts:
tests/integration/pull_create_test.go
add missing testPullCreateDirectly from
c63060b130d34e3f03f28f4dccbf04d381a95c17 Fix code owners will not be mentioned when a pull request comes from a forked repository (#30476)
Fix#31423
(cherry picked from commit f4b8f6fc40ce2869135372a5c6ec6418d27ebfba)
Conflicts:
models/fixtures/comment.yml
comment fixtures have to be shifted because there is one more in Forgejo
Since page templates keep changing, some pages that contained forms with
CSRF token no longer have them.
It leads to some calls of `GetCSRF` returning an empty string, which
fails the tests. Like
3269b04d61/tests/integration/attachment_test.go (L62-L63)
The test did try to get the CSRF token and provided it, but it was
empty.
(cherry picked from commit 13283873e9d523d5a5557f55d64f702c1a9f76ec)
Conflicts:
tests/integration/integration_test.go
trivial context conflict
- supports glob patterns in testfiles
- only runs tests on changes
- always runs tests without specified patterns
tests(e2e): refactor global watch patterns
tests(e2e): add watch patterns to test files
- This a port of https://github.com/go-gitea/gitea/pull/31931 in a
behavior-sense. None of the code was actually ported.
- Follow up for #2834, now also don't allow modification.
- Integration test added.
- Unit test modified.
Multiple chunks are uploaded with type "block" without using
"appendBlock" and eventually out of order for bigger uploads.
8MB seems to be the chunk size
This change parses the blockList uploaded after all blocks to get the
final artifact size and order them correctly before calculating the
sha256 checksum over all blocks
Fixes#31354
(cherry picked from commit b594cec2bda6f861effedb2e8e0a7ebba191c0e9)
Conflicts:
routers/api/actions/artifactsv4.go
conflict because of Refactor AppURL usage (#30885) 67c1a07285008cc00036a87cef966c3bd519a50c
that was not cherry-picked in Forgejo
the resolution consist of removing the extra ctx argument
- [x] add architecture-specific removal support
- [x] Fix upload competition
- [x] Fix not checking input when downloading
docs: https://codeberg.org/forgejo/docs/pulls/874
### Release notes
- [ ] I do not want this change to show in the release notes.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/5351
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Exploding Dragon <explodingfkl@gmail.com>
Co-committed-by: Exploding Dragon <explodingfkl@gmail.com>
This PR addresses the missing `bin` field in Composer metadata, which
currently causes vendor-provided binaries to not be symlinked to
`vendor/bin` during installation.
In the current implementation, running `composer install` does not
publish the binaries, leading to issues where expected binaries are not
available.
By properly declaring the `bin` field, this PR ensures that binaries are
correctly symlinked upon installation, as described in the [Composer
documentation](https://getcomposer.org/doc/articles/vendor-binaries.md).
(cherry picked from commit d351a42494e71b5e2da63302c2f9b46c78e6dbde)
Remove unused CSRF options, decouple "new csrf protector" and "prepare"
logic, do not redirect to home page if CSRF validation falis (it
shouldn't happen in daily usage, if it happens, redirecting to home
doesn't help either but just makes the problem more complex for "fetch")
(cherry picked from commit 1fede04b83288d8a91304a83b7601699bb5cba04)
Conflicts:
options/locale/locale_en-US.ini
tests/integration/repo_branch_test.go
trivial context conflicts
Change word order for issue comment actions
- An attempt to address https://codeberg.org/forgejo/forgejo/issues/2650
Org team permissions improvements
- consistency: added missing dot
- clarity: explain what external units mean
- use dedicated keys to explain the permissions.
- split in read/write permissions
- use explicit labels for accessibility
- ext_wiki.desc and ext_issues.desc are no longer in use.
A 500 status code was thrown when passing a non-existent target to the
create release API. This snapshot handles this error and instead throws
a 404 status code.
Discovered while working on #31840.
(cherry picked from commit f05d9c98c4cb95e3a8a71bf3e2f8f4529e09f96f)
- allow running with multiple workers (tested with up to four workers locally which
didn't show signs of flakiness)
- prevent race condition with webauthn tests (running them in parallel
on the same user could prevent another test from logging in)
- fix flakiness on CI action status (Chromium sometimes needs a long
time to fill the href field, firefox is always faster)
This reverts commit e8585eff5c.
- For WebAuthn Credential level 3, the `backup_eligible` and
`backup_state` flags are checked if they are consistent with the values
given on login. Forgejo never stored this data, so add a database
migration that makes all webauthn credentials 'legacy' and on the next
first use capture the values of `backup_eligible` and `backup_state`.
As suggested in https://github.com/go-webauthn/webauthn/discussions/219#discussioncomment-10429662
- Adds unit tests.
- Add E2E test.
- when the PR title has the maximum length, the WIP toggle switch does nothing
- work around this by slightly reducing the max input size (- 10 characters for eventually long prefixes)
- test WIP toggling edge case in playwright
fix(e2e): increase timeouts
A look at recent runs suggests they should be increased globally. The timeouts in the config file have no timeout by default.
- The Conan and Container packages use a different type of
authentication. It first authenticates via the regular way (api tokens
or user:password, handled via `auth.Basic`) and then generates a JWT
token that is used by the package software (such as Docker) to do the
action they wanted to do. This JWT token didn't properly propagate the
API scopes that the token was generated for, and thus could lead to a
'scope escalation' within the Conan and Container packages, read
access to write access.
- Store the API scope in the JWT token, so it can be propagated on
subsequent calls that uses that JWT token.
- Integration test added.
- Resolves#5128
- This is a fork of https://github.com/dchest/captcha, as
https://gitea.com/go-chi/captcha is a fork of
github.com/go-macaron/captcha which is a fork (although not properly
credited) of a older version of https://github.com/dchest/captcha. Hence
why I've just forked the original.
- The fork includes some QoL improvements (uses standard library for
determistic RNG instead of rolling your own crypto), and removal of
audio support (500KiB unused data that bloated the binary otherwise).
Flips the image over the x-asis.
47270f2b55..main
- This move is needed for the next commit, because
gitea.com/go-chi/captcha included the gitea.com/go-chi/cache dependency.
Gusted