Skip to content

Conversation

realaravinth
Copy link
Contributor

On behalf of Loïc Dachary

They were previously not covered at all, either by integration tests
or unit tests.
https://app.codecov.io/gh/go-gitea/gitea/blob/main/models/migrate.go
It fixes a bug where the num_comments field was incorrectly set to include all types of comments.
It also sets num_closed_issues: 0 as default in milestone fixtures. If they are not set, Incr("num_closed_issues") will be a noop because the field is null.
The would be the case for instance in models/migrate.go
[xorm] [info] 2021/12/30 21:38:05.370805 [SQL] UPDATE milestone SET num_issues = num_issues + ?, num_closed_issues


source: https://lab.forgefriends.org/forgefriends/forgefriends/-/merge_requests/31

@realaravinth realaravinth marked this pull request as draft January 3, 2022 08:28
@realaravinth realaravinth marked this pull request as ready for review January 3, 2022 10:14
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Jan 3, 2022
@lunny lunny added the type/bug label Jan 3, 2022
Loïc Dachary added 3 commits January 3, 2022 20:03
If they are not set, Incr("num_closed_issues") will be a noop because
the field is null.

The would be the case for instance in models/migrate.go

[xorm] [info]  2021/12/30 21:38:05.370805 [SQL] UPDATE `milestone` SET `num_issues` = `num_issues` + ?, `num_closed_issues` = `num_closed_issues` + ? WHERE `id`=? [1 1 1]

Signed-off-by: Loïc Dachary <[email protected]>
The num_comment field is only about the comments of type
CommentTypeComment, i.e. 0.

Signed-off-by: Loïc Dachary <[email protected]>
They were previously not covered at all, either by integration tests
or unit tests.

https://app.codecov.io/gh/go-gitea/gitea/blob/main/models/migrate.go

Signed-off-by: Loïc Dachary <[email protected]>
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jan 3, 2022
@wxiaoguang wxiaoguang merged commit ade41f3 into go-gitea:main Jan 3, 2022
@realaravinth
Copy link
Contributor Author

@wxiaoguang thanks for the merge and for getting rid of the unecessary LogSQL = true 👍


source

zjjhot added a commit to zjjhot/gitea that referenced this pull request Jan 4, 2022
* giteaoffical/main: (22 commits)
  Add MP4 as default allowed attachment type (go-gitea#18170)
  [skip ci] Updated translations via Crowdin
  Include folders into size cost (go-gitea#18158)
  Don't delete branch if other PRs with this branch are open (go-gitea#18164)
  Remove unused route "/tasks/trigger" (go-gitea#18160)
  Fix EasyMDE validation (go-gitea#18161)
  Fix bug (go-gitea#18168)
  tests: add coverage for models migration helpers  (go-gitea#18162)
  [skip ci] Updated translations via Crowdin
  Require codereview to have content (go-gitea#18156)
  chore(lint): use golangci-lint to call revive and misspell checker. (go-gitea#18145)
  Update owners for 2022 (go-gitea#18155)
  Refactor auth package (go-gitea#17962)
  Unify and simplify TrN for i18n (go-gitea#18141)
  Use correct user when determining max repo limits for error messages (go-gitea#18153)
  Add singuliere to MAINTAINERS (go-gitea#18148)
  [skip ci] Updated licenses and gitignores
  Add API to get issue/pull comments and events (timeline) (go-gitea#17403)
  Upgrade certmagic from v0.14.1 to v0.15.2 (go-gitea#18138)
  Upgrade certmagic from v0.14.1 to v0.15.2 (go-gitea#18138)
  ...
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
They were previously not covered at all, either by integration tests or unit tests.

This PR also fixes a bug where the `num_comments` field was incorrectly set to include all types of comments.

It sets num_closed_issues: 0 as default in milestone unit test fixtures. If they are not set, Incr("num_closed_issues") will be a noop because the field is null.
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants