-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Fix incorrect issue filter in user dashboard #23630
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
Conversation
f087fa3
to
313520e
Compare
313520e
to
5343ec1
Compare
@@ -444,21 +444,6 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { | |||
repoOpts.TeamID = team.ID | |||
} | |||
|
|||
switch filterMode { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know whether we need to remove repoOpts
above in this PR. it is not used.
models/issues/issue.go
Outdated
@@ -1402,17 +1404,64 @@ func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *organizati | |||
} else { | |||
cond = cond.And( | |||
builder.Or( | |||
repo_model.UserOwnedRepoCond(userID), // owned repos | |||
repo_model.UserAccessRepoCond(repoIDstr, userID), // user can access repo in a unit independent way | |||
repo_model.UserAssignedRepoCond(repoIDstr, userID), // user has been assigned accessible public repos |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
repo_model.UserAssignedRepoCond
is only used here.
Do I need to remove it or keep it.
models/issues/issue.go
Outdated
repo_model.UserOwnedRepoCond(userID), // owned repos | ||
repo_model.UserAccessRepoCond(repoIDstr, userID), // user can access repo in a unit independent way | ||
repo_model.UserAssignedRepoCond(repoIDstr, userID), // user has been assigned accessible public repos | ||
repo_model.UserMentionedRepoCond(repoIDstr, userID), // user has been mentioned accessible public repos |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto.
models/issues/issue.go
Outdated
repo_model.UserAccessRepoCond(repoIDstr, userID), // user can access repo in a unit independent way | ||
repo_model.UserAssignedRepoCond(repoIDstr, userID), // user has been assigned accessible public repos | ||
repo_model.UserMentionedRepoCond(repoIDstr, userID), // user has been mentioned accessible public repos | ||
repo_model.UserCreateIssueRepoCond(repoIDstr, userID, isPull), // user has created issue/pr accessible public repos |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto.
ed31247
to
da6e7f8
Compare
All `access_mode` value of Owner Teams are 0(AccessModeNone) in `team_unit` table, which should be 4(AccessModeOwner) In `team` table:  In `team_unit` table:  ps: In #23630, `access_mode` in `team_unit` is used to check the team unit permission, but I found that user can not see issues in owned org repos.
Got the sql in mssql. Have no idea with the error. sqlSELECT
issue.repo_id AS repo_id,
COUNT(*) AS count
FROM
[issue]
INNER JOIN
[repository]
ON [issue].repo_id = [repository].id
WHERE
(
issue.is_closed = ?
)
AND (
issue.is_pull = ?
)
AND repository.is_archived = ?
AND (
repository.owner_id = ?
OR issue.repo_id IN(
SELECT
[access].repo_id
FROM
[access]
INNER JOIN
repository
ON [repository].id = [access].repo_id
INNER JOIN
user
ON [user].id = [repository].owner_id
WHERE
[access].user_id = ?
AND [access].mode > ?
AND (
(
[user].type = ?
AND (
[access].repo_id IN(
SELECT
[team_repo].repo_id
FROM
team_repo
INNER JOIN
team_user
ON [team_user].team_id = [team_repo].team_id
INNER JOIN
team_unit
ON [team_unit].team_id = [team_repo].team_id
WHERE
[team_user].uid = ?
AND [team_unit].[type] = ?
AND [team_unit].[access_mode] > ?
)
OR [access].repo_id IN(
SELECT
repo_id
FROM
[collaboration]
WHERE
[collaboration].user_id = ?
)
)
)
OR [user].type = ?
)
)
OR (
repository.is_private = ?
AND issue.id IN(
SELECT
issue_assignees.issue_id
FROM
issue_assignees
WHERE
issue_assignees.assignee_id = ?
)
)
OR (
repository.is_private = ?
AND issue.id IN(
SELECT
issue_user.issue_id
FROM
issue_user
WHERE
issue_user.is_mentioned = ?
AND issue_user.uid = ?
)
)
OR (
repository.is_private = ?
AND issue.id IN(
SELECT
issue.id
FROM
issue
WHERE
issue.is_pull = ?
AND issue.poster_id = ?
)
)
)
GROUP BY
issue.repo_id |
Try to quote all names, like |
ff4432a
to
862c853
Compare
Hmm, CI passes. That's why I recommend to use two words for variable names / table names / column names @lunny It saves a lot of debugging time. ps: the SQL seems quite complex ...... |
Yes, too complex. |
It works now. ps: maybe we should also add some new tests. |
Should re-test, I recall @lunny made some recent changes surrounding filters that may impact this. |
Simplified the SQL in |
}), | ||
), | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the difference between these functions and the ones in repo_model
? Maybe outline in a comment the differences?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The functions in repo_model
are only used here (but will cause the issue). So we can remove them or move these new functions into repo_model
. I didn't do that because I'm not sure whether the functions in repo_model
are only designed for issuePullAccessibleRepoCond
.
The difference is that the functions in repo_model
use issue.repo_id in repo_ids
to filter the issue, so all issues in these repos will be caught.
So we need use issue.id in issue_ids
to get the correct issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's try it I guess.
I think this has been replaced by #26657 ? |
@yp05327 What do you think, can this PR be closed? |
As we have rewrite the search engine for issues, I will close this PR. |
Fixes #22799
Changes:
(selected a repo in repo filter which have no issues created by user, then access
created by you
)After fix: