Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tags sort by creation time (descending) on branch/tag dropdowns #23491

Merged
merged 11 commits into from
Mar 16, 2023

Conversation

HesterG
Copy link
Contributor

@HesterG HesterG commented Mar 15, 2023

This PR fixes the tags sort issue mentioned in #23432
The tags on dropdown shoud be sorted in descending order of time but are not. Because when getting tags, it execeutes git tag sort --sort=-taggerdate. Git supports two types of tags: lightweight and annotated, and git tag sort --sort=-taggerdate dosen't work with lightweight tags, which will not give correct result. This PR add GetTagNamesByRepoID to get tags from the database so the tags are sorted.

Also adapt this change to the droplist when comparing branches.

Dropdown places:

截屏2023-03-15 14 25 39

截屏2023-03-15 14 25 27

@codecov-commenter
Copy link

codecov-commenter commented Mar 15, 2023

Codecov Report

Merging #23491 (0d61e49) into main (f521e88) will decrease coverage by 0.01%.
The diff coverage is 43.28%.

❗ Current head 0d61e49 differs from pull request most recent head d42a6a4. Consider uploading reports for the commit d42a6a4 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main   #23491      +/-   ##
==========================================
- Coverage   47.14%   47.13%   -0.01%     
==========================================
  Files        1149     1154       +5     
  Lines      151446   152186     +740     
==========================================
+ Hits        71397    71736     +339     
- Misses      71611    71983     +372     
- Partials     8438     8467      +29     
Impacted Files Coverage Δ
cmd/dump.go 0.67% <0.00%> (ø)
models/actions/run.go 1.64% <0.00%> (-0.08%) ⬇️
models/packages/package.go 45.45% <0.00%> (-1.13%) ⬇️
modules/actions/github.go 0.00% <0.00%> (ø)
modules/actions/workflows.go 0.00% <0.00%> (ø)
modules/doctor/storage.go 31.93% <0.00%> (ø)
modules/setting/git.go 45.45% <ø> (ø)
modules/storage/minio.go 1.51% <0.00%> (-0.06%) ⬇️
modules/structs/user.go 100.00% <ø> (ø)
routers/api/v1/admin/email.go 0.00% <0.00%> (ø)
... and 35 more

... and 12 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 15, 2023
@HesterG HesterG changed the title Fix tags sort by creation time (descending) on branch/tag droplists Fix tags sort by creation time (descending) on branch/tag dropdowns Mar 15, 2023
@wolfogre
Copy link
Member

I think it should be mentioned in the description that "Git supports two types of tags: lightweight and annotated, and git tag sort --sort=-taggerdat dosen't work with lightweight tags."

Copy link
Member

@wolfogre wolfogre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 16, 2023
@lunny lunny added type/enhancement An improvement of existing functionality topic/ui Change the appearance of the Gitea UI labels Mar 16, 2023
@lunny lunny added this to the 1.20.0 milestone Mar 16, 2023
@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 Mar 16, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 16, 2023
@@ -660,20 +660,9 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) {
return
}

tags, err := ctx.Repo.GitRepo.GetTags(0, 0)
tags, err := repo_model.GetTagNamesByRepoID(ctx, ctx.Repo.Repository.ID)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

☹️ yet another call to models inside modules.

HasSha1: util.OptionalBoolTrue,
}

tags := make([]string, 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
tags := make([]string, 0)
tags := make([]string)

@jolheiser
Copy link
Member

🎺 🤖

@jolheiser jolheiser merged commit 8d9f8e1 into go-gitea:main Mar 16, 2023
@jolheiser jolheiser removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 16, 2023
@jolheiser
Copy link
Member

Agh sorry @delvh I guess my page never updated with your comments, missed it 🙁

zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 17, 2023
* giteaofficial/main:
  Use `<nav>` instead of `<div>` in the global navbar (go-gitea#23125)
  Fix aria.js bugs: incorrect role element problem, mobile focus problem, tippy problem (go-gitea#23450)
  [skip ci] Updated translations via Crowdin
  Make time tooltips interactive (go-gitea#23526)
  Update mini-css-extract-plugin, remove postcss (go-gitea#23520)
  Fix review comment context menu clipped bug (go-gitea#23523)
  Add absent repounits to create/edit repo API (go-gitea#23500)
  Fix tags sort by creation time (descending) on branch/tag dropdowns  (go-gitea#23491)
  Allow both fullname and username search when `DEFAULT_SHOW_FULL_NAME` is true (go-gitea#23463)
  Handle files starting with colons in WalkGitLog (go-gitea#22935)
  Change `Close` to either `Close issue` or `Close pull request` (go-gitea#23506)
  Update act (go-gitea#23512)
  Move pidfile creation from setting to web cmd package (go-gitea#23285)
@HesterG HesterG deleted the fix-tag-sort-time branch March 17, 2023 09:42
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
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. topic/ui Change the appearance of the Gitea UI type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants