From 022eac4ac8e59f861237cc1e02f7ef117eaf8e30 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 30 Apr 2024 17:36:28 +0200 Subject: [PATCH 1/2] Get repo assignees and reviewers should ignore deactivated users (#30770) (#30782) Backport #30770 If an user is deactivated, it should not be in the list of users who are suggested to be assigned or review-requested. old assignees or reviewers are not affected. --- *Sponsored by Kithara Software GmbH* --- models/repo/user_repo.go | 8 ++++++-- models/repo/user_repo_test.go | 22 +++++++++++++++++----- tests/integration/api_repo_test.go | 4 +++- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/models/repo/user_repo.go b/models/repo/user_repo.go index 1c5412fe7d..c305603e02 100644 --- a/models/repo/user_repo.go +++ b/models/repo/user_repo.go @@ -130,7 +130,10 @@ func GetRepoAssignees(ctx context.Context, repo *Repository) (_ []*user_model.Us // and just waste 1 unit is cheaper than re-allocate memory once. users := make([]*user_model.User, 0, len(uniqueUserIDs)+1) if len(userIDs) > 0 { - if err = e.In("id", uniqueUserIDs.Values()).OrderBy(user_model.GetOrderByName()).Find(&users); err != nil { + if err = e.In("id", uniqueUserIDs.Values()). + Where(builder.Eq{"`user`.is_active": true}). + OrderBy(user_model.GetOrderByName()). + Find(&users); err != nil { return nil, err } } @@ -152,7 +155,8 @@ func GetReviewers(ctx context.Context, repo *Repository, doerID, posterID int64) return nil, err } - cond := builder.And(builder.Neq{"`user`.id": posterID}) + cond := builder.And(builder.Neq{"`user`.id": posterID}). + And(builder.Eq{"`user`.is_active": true}) if repo.IsPrivate || repo.Owner.Visibility == api.VisibleTypePrivate { // This a private repository: diff --git a/models/repo/user_repo_test.go b/models/repo/user_repo_test.go index 591dcea5b5..d2bf6dc912 100644 --- a/models/repo/user_repo_test.go +++ b/models/repo/user_repo_test.go @@ -9,6 +9,7 @@ import ( "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" "github.com/stretchr/testify/assert" ) @@ -25,8 +26,17 @@ func TestRepoAssignees(t *testing.T) { repo21 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 21}) users, err = repo_model.GetRepoAssignees(db.DefaultContext, repo21) assert.NoError(t, err) - assert.Len(t, users, 4) - assert.ElementsMatch(t, []int64{10, 15, 16, 18}, []int64{users[0].ID, users[1].ID, users[2].ID, users[3].ID}) + if assert.Len(t, users, 4) { + assert.ElementsMatch(t, []int64{10, 15, 16, 18}, []int64{users[0].ID, users[1].ID, users[2].ID, users[3].ID}) + } + + // do not return deactivated users + assert.NoError(t, user_model.UpdateUserCols(db.DefaultContext, &user_model.User{ID: 15, IsActive: false}, "is_active")) + users, err = repo_model.GetRepoAssignees(db.DefaultContext, repo21) + assert.NoError(t, err) + if assert.Len(t, users, 3) { + assert.NotContains(t, []int64{users[0].ID, users[1].ID, users[2].ID}, 15) + } } func TestRepoGetReviewers(t *testing.T) { @@ -38,17 +48,19 @@ func TestRepoGetReviewers(t *testing.T) { ctx := db.DefaultContext reviewers, err := repo_model.GetReviewers(ctx, repo1, 2, 2) assert.NoError(t, err) - assert.Len(t, reviewers, 4) + if assert.Len(t, reviewers, 3) { + assert.ElementsMatch(t, []int64{1, 4, 11}, []int64{reviewers[0].ID, reviewers[1].ID, reviewers[2].ID}) + } // should include doer if doer is not PR poster. reviewers, err = repo_model.GetReviewers(ctx, repo1, 11, 2) assert.NoError(t, err) - assert.Len(t, reviewers, 4) + assert.Len(t, reviewers, 3) // should not include PR poster, if PR poster would be otherwise eligible reviewers, err = repo_model.GetReviewers(ctx, repo1, 11, 4) assert.NoError(t, err) - assert.Len(t, reviewers, 3) + assert.Len(t, reviewers, 2) // test private user repo repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) diff --git a/tests/integration/api_repo_test.go b/tests/integration/api_repo_test.go index bc2720d51e..f33827e58b 100644 --- a/tests/integration/api_repo_test.go +++ b/tests/integration/api_repo_test.go @@ -684,7 +684,9 @@ func TestAPIRepoGetReviewers(t *testing.T) { resp := MakeRequest(t, req, http.StatusOK) var reviewers []*api.User DecodeJSON(t, resp, &reviewers) - assert.Len(t, reviewers, 4) + if assert.Len(t, reviewers, 3) { + assert.ElementsMatch(t, []int64{1, 4, 11}, []int64{reviewers[0].ID, reviewers[1].ID, reviewers[2].ID}) + } } func TestAPIRepoGetAssignees(t *testing.T) { From 2bedd16c14178461132ad5de347e385721ee3841 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Wed, 1 May 2024 05:19:13 +0800 Subject: [PATCH 2/2] Improve logout from worker (#30775) (#30789) Backport #30775 by wxiaoguang A quick fix for #30756 Co-authored-by: wxiaoguang --- web_src/js/features/notification.js | 3 ++- web_src/js/features/stopwatch.js | 3 ++- web_src/js/modules/worker.js | 9 +++++++++ 3 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 web_src/js/modules/worker.js diff --git a/web_src/js/features/notification.js b/web_src/js/features/notification.js index 2de640e674..8e5a1f83db 100644 --- a/web_src/js/features/notification.js +++ b/web_src/js/features/notification.js @@ -1,6 +1,7 @@ import $ from 'jquery'; import {GET} from '../modules/fetch.js'; import {toggleElem} from '../utils/dom.js'; +import {logoutFromWorker} from '../modules/worker.js'; const {appSubUrl, notificationSettings, assetVersionEncoded} = window.config; let notificationSequenceNumber = 0; @@ -95,7 +96,7 @@ export function initNotificationCount() { type: 'close', }); worker.port.close(); - window.location.href = `${appSubUrl}/`; + logoutFromWorker(); } else if (event.data.type === 'close') { worker.port.postMessage({ type: 'close', diff --git a/web_src/js/features/stopwatch.js b/web_src/js/features/stopwatch.js index 2ec74344fc..c58a446075 100644 --- a/web_src/js/features/stopwatch.js +++ b/web_src/js/features/stopwatch.js @@ -2,6 +2,7 @@ import prettyMilliseconds from 'pretty-ms'; import {createTippy} from '../modules/tippy.js'; import {GET} from '../modules/fetch.js'; import {hideElem, showElem} from '../utils/dom.js'; +import {logoutFromWorker} from '../modules/worker.js'; const {appSubUrl, notificationSettings, enableTimeTracking, assetVersionEncoded} = window.config; @@ -75,7 +76,7 @@ export function initStopwatch() { type: 'close', }); worker.port.close(); - window.location.href = `${appSubUrl}/`; + logoutFromWorker(); } else if (event.data.type === 'close') { worker.port.postMessage({ type: 'close', diff --git a/web_src/js/modules/worker.js b/web_src/js/modules/worker.js new file mode 100644 index 0000000000..ef3f1dea48 --- /dev/null +++ b/web_src/js/modules/worker.js @@ -0,0 +1,9 @@ +import {sleep} from '../utils.js'; + +const {appSubUrl} = window.config; + +export async function logoutFromWorker() { + // wait for a while because other requests (eg: logout) may be in the flight + await sleep(5000); + window.location.href = `${appSubUrl}/`; +}