This is, in large part, a refactoring: we rename `CommitVerification` to
`ObjectVerification`, and adjust `ParseObjectWithSignature` (previously
`ParseCommitWithSignature`) to work on an object, rather than a commit.
This in turn, lets us implement `ParseTagWithSignature` on top of it, so
commit & tag signature verification will share most of the code.
Work sponsored by @glts.
Signed-off-by: Gergely Nagy <forgejo@gergo.csillger.hu>
Fix https://github.com/go-gitea/gitea/pull/29874#discussion_r1542227686
- The migration of v292 will miss many projects. These projects will
have no default board. This PR introduced a new migration number and
removed v292 migration.
- This PR also added the missed transactions on project-related
operations.
- Only `SetDefaultBoard` will remove duplicated defaults but not in
`GetDefaultBoard`
(cherry picked from commit 40cdc84b368cce8328b4b49ea5ecf1c5fa040300)
Resolves#29965.
---
Manually tested this by:
- Following the
[installation](https://docs.gitea.com/next/installation/install-with-docker#basics)
guide (but built a local Docker image instead)
- Creating 2 users, one who is the `Owner` of a newly-created repository
and the other a `Collaborator`
- Had the `Collaborator` create a PR that the `Owner` reviews
- `Collaborator` resolves conversation and `Owner` merges PR
And with this change we see that we can no longer see re-request review
button for the `Owner`:
<img width="1351" alt="Screenshot 2024-03-25 at 12 39 18 AM"
src="https://github.com/go-gitea/gitea/assets/60799661/bcd9c579-3cf7-474f-a51e-b436fe1a39a4">
(cherry picked from commit 242b331260925e604150346e61329097d5731e77)
This PR replaces the use of `max( id )`, and instead using ``max(
`index` )`` for determining the latest commit status. Building business
logic over an `auto_increment` primary key like `id` is risky and
there’re already plenty of discussions on the Internet.
There‘s no guarantee for `auto_increment` values to be monotonic,
especially upon failures or with a cluster. In the specific case, we met
the problem of commit statuses being outdated when using TiDB as the
database. As [being
documented](https://docs.pingcap.com/tidb/stable/auto-increment),
`auto_increment` values assigned to an `insert` statement will only be
monotonic on a per server (node) basis.
Closes#30074.
(cherry picked from commit 7443a10fc3d722d3326a0cb7b15b208f907c72d7)
On creation of an empty project (no template) a default board will be
created instead of falling back to the uneditable pseudo-board.
Every project now has to have exactly one default boards. As a
consequence, you cannot unset a board as default, instead you have to
set another board as default. Existing projects will be modified using a
cron job, additionally this check will run every midnight by default.
Deleting the default board is not allowed, you have to set another board
as default to do it.
Fixes#29873Fixes#14679 along the way
Fixes#29853
Co-authored-by: delvh <dev.lh@web.de>
(cherry picked from commit e5160185ed65fd1c2bcb2fc7dc7e0b5514ddb299)
Conflicts:
options/locale/locale_en-US.ini
trivial conflict because Forgejo strings do not have
surrounding double quotes
Fix#30103
ps: comments has `LoadAttributes`, but maybe considering performance
problem, we don't call it.
(cherry picked from commit ce3c3512265df3b4940672be40065c4fb415ef95)
Help #29999, or its tests cannot pass.
Also, add some comments to clarify the usage of `TxContext`.
I don't check all usages of `TxContext` because there are too many
(almost 140+). It's a better idea to replace them with `WithTx` instead
of checking them one by one. However, that may be another refactoring
PR.
(cherry picked from commit c6c4d66004c70b24abc8048b39b660b8361a0395)
This PR uses `db.ListOptions` instead of `Paginor` to make the code
simpler.
And it also fixed the performance problem when viewing /pulls or
/issues. Before the counting in fact will also do the search.
---------
Co-authored-by: Jason Song <i@wolfogre.com>
Co-authored-by: silverwind <me@silverwind.io>
(cherry picked from commit 3f26fe2fa2c7141c9e622297e50a70f3e0003e4d)
- `user_model.DeleteInactiveEmailAddresses` related code was added in
Gogs as part to delete inactive users, however since then the related
code to delete users has changed and this code now already delete email
addresses of the user, it's therefore not needed anymore to
`DeleteInactiveEmailAddresses`.
- The call to `DeleteInactiveEmailAddresses` can actually cause issues.
As the associated user might not have been deleted, because it
was not older than the specified `olderThan` argument. Therefore causing
a database inconsistency and lead to internal server errors if the user
tries to activate their account.
- Adds unit test to verify correct behavior (fails without this patch).
- Currently protected branch rules do not apply to admins, however in
some cases (like in the case of Forgejo project) you might also want to
apply these rules to admins to avoid accidental merges.
- Add new option to configure this on a per-rule basis.
- Adds integration tests.
- Resolves#65
Fixes https://github.com/go-gitea/gitea/issues/28297
This PR also fixed a problem that it needs a database transaction when
removing the WIP title.
---
Resolves#2771
Also partially ports gitea#29783
(cherry picked from commit 17d7ab5ad4ce3d0fbc1251572c22687c237a30b1)
There is no code change. Just move notification list related
structs/functions from one file to another.
---
Resolves#2772
Simply move the moderation code to the new function (which wasn't
changed).
(cherry picked from commit b25eec41eb4d7058be808daefd6fd47eed61c7d3)
The fix against the race incorrectly assumes the sha of the commit being
pushed belongs to the base repository. It finds the highest possible
pull request ID from the head repository instead of looking it up in
the base repository.
Figuring out if a PR was created in the future based on the highest
index of the base repository would require collecting all of them
because there is no way to know in advance which repository may be
involved in the race.
Fixing this race can be done either by:
* Introducing a new field in the pull_request table https://codeberg.org/forgejo/forgejo/pulls/2842
which feels more like a hack than a real solution
* Refactoring the logic
which would be a significant undertaking
The race has been in the codebase for a very long time and manifests
itself in the CI, when events happen in quick succession. The only
concrete manifestation was however fixed by https://codeberg.org/forgejo/forgejo/issues/2009
Since this race now only exists in theory and not in practice, let's
revert this bugous commit until a proper solution is implemented.
Fixes: https://codeberg.org/forgejo/forgejo/issues/2817
This reverts commit 036f1eddc5.
Conflicts:
services/pull/pull.go
This PR adds `setting.Service.DefaultOrgMemberVisible` value to dataset
of user when the initial org creator is being added to the created org.
Fixes#30012.
(cherry picked from commit e3e08dcc5184cdbdac5023fabaafba123a995c3e)
This PR will avoid load pullrequest.Issue twice in pull request list
page. It will reduce x times database queries for those WIP pull
requests.
Partially fix#29585
---------
Co-authored-by: Giteabot <teabot@gitea.io>
(cherry picked from commit 62f8174aa2fae1481c7e17a6afcb731a5b178cd0)
Conflicts:
models/activities/notification_list.go
moved to models/activities/notification.go
`CommitGPGSignature` was originally made to store information about a
commit's GPG signature. Nowadays, it is used to store information about
SSH signatures too, and not just commit signatures, but tag signatures
too.
As such, rename it to `ObjectSignature`, because that describes what it
does a whole lot better.
Signed-off-by: Gergely Nagy <forgejo@gergo.csillger.hu>
Because the `git` module did not recognize SSH signed tags, those
signatures ended up in the `notes` column of the `release` table. While
future signatures will not end up there, Forgejo should clean up the old
ones.
This migration does just that: finds all releases that have an SSH
signature, and removes those signatures, preserving the rest of the
note (if any).
While this may seem like an expensive operation, it's only done once,
and even on the largest known Forgejo instance as of this
writing (Codeberg), the number of affected rows are just over a hundred,
a tiny amount all things considered.
Signed-off-by: Gergely Nagy <forgejo@gergo.csillger.hu>
Fix#28157
This PR fix the possible bugs about actions schedule.
- Move `UpdateRepositoryUnit` and `SetRepoDefaultBranch` from models to
service layer
- Remove schedules plan from database and cancel waiting & running
schedules tasks in this repository when actions unit has been disabled
or global disabled.
- Remove schedules plan from database and cancel waiting & running
schedules tasks in this repository when default branch changed.
(cherry picked from commit 97292da960)
Conflicts:
modules/actions/github.go
routers/web/repo/setting/default_branch.go
routers/web/repo/setting/setting.go
services/repository/branch.go
services/repository/setting.go
tests/integration/actions_trigger_test.go
Repositories displaying an "Add more..." tab on the header is a neat way
to let people discover they can enable more units. However, displaying
it all the time for repository owners, even when they deliberately do
not want to enable more units gets noisy very fast.
As such, this patch introduces a new setting which lets people disable
this hint under the appearance settings.
Fixes#2378.
Signed-off-by: Gergely Nagy <forgejo@gergo.csillger.hu>
This PR do some loading speed optimization for feeds user interface
pages.
- Load action users batchly but not one by one.
- Load action repositories batchly but not one by one.
- Load action's Repo Owners batchly but not one by one.
- Load action's possible issues batchly but not one by one.
- Load action's possible comments batchly but not one by one.
(cherry picked from commit aed3b53abdd02a3ffbf9e8eb90272ff567333073)
Unlike other async processing in the queue, we should sync branches to
the DB immediately when handling git hook calling. If it fails, users
can see the error message in the output of the git command.
It can avoid potential inconsistency issues, and help #29494.
---------
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Regression of #2507, which switched the HEAD from `pr.GetGitRefName()`
to `pr.HeadCommitID` but it had to be `prInfo.HeadCommitID`. Resolves#2656
I was able to reproduce this locally with _some_ pull requests, haven't
been able to get a reproducer trough integration testing.
(cherry picked from commit 25b842df261452a29570ba89ffc3a4842d73f68c)
Conflicts:
routers/web/repo/wiki.go
services/repository/branch.go
services/repository/migrate.go
services/wiki/wiki.go
also apply to Forgejo specific usage of the refactored functions
Refactor the webhook logic, to have the type-dependent processing happen
only in one place.
---
1. An event happens
2. It is pre-processed (depending on the webhook type) and its body is
added to a task queue
3. When the task is processed, some more logic (depending on the webhook
type as well) is applied to make an HTTP request
This means that webhook-type dependant logic is needed in step 2 and 3.
This is cumbersome and brittle to maintain.
Updated webhook flow with this PR:
1. An event happens
2. It is stored as-is and added to a task queue
3. When the task is processed, the event is processed (depending on the
webhook type) to make an HTTP request
So the only webhook-type dependent logic happens in one place (step 3)
which should be much more robust.
- the raw event must be stored in the hooktask (until now, the
pre-processed body was stored)
- to ensure that previous hooktasks are correctly sent, a
`payload_version` is added (version 1: the body has already been
pre-process / version 2: the body is the raw event)
So future webhook additions will only have to deal with creating an
http.Request based on the raw event (no need to adjust the code in
multiple places, like currently).
Moreover since this processing happens when fetching from the task
queue, it ensures that the queuing of new events (upon a `git push` for
instance) does not get slowed down by a slow webhook.
As a concrete example, the PR #19307 for custom webhooks, should be
substantially smaller:
- no need to change `services/webhook/deliver.go`
- minimal change in `services/webhook/webhook.go` (add the new webhook
to the map)
- no need to change all the individual webhook files (since with this
refactor the `*webhook_model.Webhook` is provided as argument)
(cherry picked from commit 26653b196bd1d15c532af41f60351596dd4330bd)
Conflicts:
services/webhook/deliver_test.go
trivial context conflict
Fix#29000Fix#28685Fix#18568
Related: #27497
And by the way fix#24036, add a Cancel button there (one line)
(cherry picked from commit 5cddab4f74bbb307ddf13e458c7ac22f93b9283a)
The tests on migration tests failed but CI reports successfully
https://github.com/go-gitea/gitea/actions/runs/7364373807/job/20044685969#step:8:141
This PR will fix the bug on migration v283 and also the CI hidden
behaviour.
The reason is on the Makefile
`GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mysql.ini $(GO) test
$(GOTESTFLAGS) -tags='$(TEST_TAGS)' $(MIGRATE_TEST_PACKAGES)` will
return the error exit code.
But
`for pkg in $(shell $(GO) list
code.gitea.io/gitea/models/migrations/...); do \
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mysql.ini $(GO) test
$(GOTESTFLAGS) -tags '$(TEST_TAGS)' $$pkg; \
done`
will not work.
This also fix#29602
(cherry picked from commit 45277486c2c6213b7766b1da708a991cdb1f3565)
Conflicts:
.github/workflows/pull-db-tests.yml
Makefile
models/migrations/v1_22/v283.go
models/migrations/v1_22/v286_test.go
models/migrations/v1_22/v287_test.go
already in Forgejo for the Makefile & CI logic but Gitea changes
otherwise rule
Follow #29522
Administrators should be able to set a user's email address even if the
email address is not in `EMAIL_DOMAIN_ALLOWLIST`
(cherry picked from commit 136dd99e86eea9c8bfe61b972a12b395655171e8)
Fix#27457
Administrators should be able to manually create any user even if the
user's email address is not in `EMAIL_DOMAIN_ALLOWLIST`.
(cherry picked from commit 4fd9c56ed09b31e2f6164a5f534a31c6624d0478)
Add two "HTMLURL" methods for PackageDescriptor.
And rename "FullWebLink" to "VersionWebLink"
(cherry picked from commit bf6502a8f7a2e9a2b64b43b7733316d863c9a768)
To avoid conflicting with User.GetDisplayName, because there is no data
type in template.
And it matches other methods like GetActFullName / GetActUserName
(cherry picked from commit 3f081d4b54261c1b4ee4f1df40c610fdd9581ef2)
Fixes#28853
Needs both https://gitea.com/gitea/act_runner/pulls/473 and
https://gitea.com/gitea/act_runner/pulls/471 on the runner side and
patched `actions/upload-artifact@v4` / `actions/download-artifact@v4`,
like `christopherhx/gitea-upload-artifact@v4` and
`christopherhx/gitea-download-artifact@v4`, to not return errors due to
GHES not beeing supported yet.
(cherry picked from commit a53d268aca87a281aadc2246541f8749eddcebed)
This PR touches the most interesting part of the "template refactoring".
1. Unclear variable type. Especially for "web/feed/convert.go":
sometimes it uses text, sometimes it uses HTML.
2. Assign text content to "RenderedContent" field, for example: `
project.RenderedContent = project.Description` in web/org/projects.go
3. Assign rendered content to text field, for example: `r.Note =
rendered content` in web/repo/release.go
4. (possible) Incorrectly calling `{{Str2html
.PackageDescriptor.Metadata.ReleaseNotes}}` in
package/content/nuget.tmpl, I guess the name Str2html misleads
developers to use it to "render string to html", but it only sanitizes.
if ReleaseNotes really contains HTML, then this is not a problem.
(cherry picked from commit e71eb8930a5d0f60874b038c223498b41ad65592)
Conflicts:
modules/templates/util_string.go
trivial context conflict
just some refactoring bits towards replacing **util.OptionalBool** with
**optional.Option[bool]**
---------
Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
(cherry picked from commit f6656181e4a07d6c415927220efa2077d509f7c6)
Conflicts:
models/repo/repo_list_test.go
trivial shared fixture count conflicts
just some refactoring bits towards replacing **util.OptionalBool** with
**optional.Option[bool]**
(cherry picked from commit 274c0aea2e88db9bc41690c90e13e8aedf6193d4)
Thanks to inferenceus : some sort orders on the "explore/users" page
could list users by their lastlogintime/updatetime.
It leaks user's activity unintentionally. This PR makes that page only
use "supported" sort orders.
Removing the "sort orders" could also be a good solution, while IMO at
the moment keeping the "create time" and "name" orders is also fine, in
case some users would like to find a target user in the search result,
the "sort order" might help.
(cherry picked from commit eedb8f41297c343d6073a7bab46e4df6ee297a90)
- Disable the CODEOWNERS feature for forked repositories, as it would
otherwise inadvertently request reviewers when for example a pull
request is opened against a forked repository to propose changes to an
existant pull request in the original repository.
- Adds integration test.
- Resolves#2525
- The content history table contains the content history of issues and
comments. For issues they are saved with an comment id of zero.
- If you want to check if the issue has an content history, it should
take into account that SQL has `comment_id = 0`, as it otherwise could
return incorrect results when for example the issue already has an
comment that has an content history.
- Fix the code of `HasIssueContentHistory` to take this into account, it
relied on XORM to generate the SQL from the non-default values of the
struct, this wouldn't generate the `comment_id = 0` SQL as `0` is the
default value of an integer.
- Remove an unncessary log (it's not the responsibility of `models`
code to do logging).
- Adds unit test.
- Resolves#2513
- The CODEOWNER feature relies on the changed files to determine which
reviewers should be added according to the `CODEOWNER` file.
- The current approach was to 'diff' between the base and head branch,
which seems logical but fail in practice when the pull request is out of
date with the base branch. Therefore it should instead diff between the
head branch and the merge base of the head and base branch, so only the
actual affected files by the pull requests are used, the same approach
is used by the diff of an unmerged pull request.
- Add integration testing (for the feature as well).
- Resolves#2458
This PR will also keep the consistent between list assigned users and
check assigned users.
(cherry picked from commit 98ab9445d1020c515c3c789f0b27d952903a2978)
Fix#14459
The following users can add/remove review requests of a PR
- the poster of the PR
- the owner or collaborators of the repository
- members with read permission on the pull requests unit
(cherry picked from commit c42083a33950be6ee9f822c6d0de3c3a79d1f51b)
Conflicts:
models/repo/repo_list_test.go
tests/integration/api_nodeinfo_test.go
tests/integration/api_repo_test.go
shared fixture counts
Adds a new API `/repos/{owner}/{repo}/commits/{sha}/pull` that allows
you to get the merged PR associated to a commit.
---------
Co-authored-by: 6543 <6543@obermui.de>
(cherry picked from commit 0a426cc575734e5eff410d6a790f40473117f753)
- In order to determine if the "Add more..." tab should be shown, the
template has to know if the repository has all units enabled, this is
done in the repository header which can be shown for quite a lot of
pages (code, issues, projects, actions etc.)
- This was previously set in the `RepoRefByType` function, which would
be called by pages such as code, issues and releases, but it was not
being called for all pages such as actions, packages and wiki. Which
would in turn incorrectly show the "Add more..." button when it
shouldn't.
- Now call it from the template itself, so the value is 'always' loaded
when necessary.
Clarify when "string" should be used (and be escaped), and when
"template.HTML" should be used (no need to escape)
And help PRs like #29059 , to render the error messages correctly.
(cherry picked from commit f3eb835886031df7a562abc123c3f6011c81eca8)
Conflicts:
modules/web/middleware/binding.go
routers/web/feed/convert.go
tests/integration/branches_test.go
tests/integration/repo_branch_test.go
trivial context conflicts
`KeyID` is never set.
(cherry picked from commit 155269fa586c41a268530c3bb56349e68e6761d7)
Conflicts:
models/user/email_address.go
trivial context conflict
Follow-up of #2282 and #2296 (which tried to address #2278)
One of the issue with the previous PR is that when a conversation on the Files tab was marked as "resolved", it would fetch all the comments for that line (even the outdated ones, which should not be shown on this page - except when explicitly activated).
To properly fix this, I have changed `FetchCodeCommentsByLine` to `FetchCodeConversation`. Its role is to fetch all comments related to a given (review, path, line) and reverted my changes in the template (which were based on a misunderstanding).
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/2306
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: oliverpool <git@olivier.pfad.fr>
Co-committed-by: oliverpool <git@olivier.pfad.fr>
RequestReview get deleted on review.
So we don't have to try to load them on comments.
broken out #28544
(cherry picked from commit 6fad2c874438275d3f69bb1cc223708bd2d27ff6)
With this option, it is possible to require a linear commit history with
the following benefits over the next best option `Rebase+fast-forward`:
The original commits continue existing, with the original signatures
continuing to stay valid instead of being rewritten, there is no merge
commit, and reverting commits becomes easier.
Closes#24906
If a repository administrator is viewing a repository, and there are
units that can be enabled, display an "Add more..." link that leads to
the repository unit settings page.
The goal here is to allow instances to configure a small set of repo
units to be enabled by default, but also highlight for repo admins that
they can add more.
Signed-off-by: Gergely Nagy <forgejo@gergo.csillger.hu>
- I found this while doing some unrelated testing in Forgejo. It wasn't
my intention to log failed SQL queries if they were cancelled (which can
happen quite frequently for larger instances) as in those cases it's not
interesting to know which SQL query was run. My intentation was only to
log an SQL query if there was an error reported by the database.
- Ref #2140
Commit 360b3fd17c (Include username in email headers (#28981),
2024-02-03) adds usernames to the From field of notification emails in
the form of `Display Name (@username)`, to prevent spoofing. However,
some email filtering software flags "@" in the display name part of the
From field as potential spoofing, as you could set the display name part
to another email address than the one you are sending from (e.g.
`From: "apparent@email-address" <actual@email-address>`). To avoid
being flagged, instead send emails from `Display Name (username)`.
Closes: #29107
---------
Co-authored-by: Giteabot <teabot@gitea.io>
(cherry picked from commit 5b2fd0fc19a2a77414c8e2989b4794b6617221f5)
