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 org admin can not access org project unit #25691

Closed
wants to merge 21 commits into from

Conversation

yp05327
Copy link
Contributor

@yp05327 yp05327 commented Jul 5, 2023

Fix #25670

We should backport the logic fix to 1.19 and 1.20
DB migration is only for 1.21

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 5, 2023
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 5, 2023
Comment on lines 32 to 48
AccessModeRead = 1
// AccessModeAdmin admin access
AccessModeAdmin = 3

// Unit Type
TypeInvalid UnitType = iota // 0 invalid
TypeCode // 1 code
TypeIssues // 2 issues
TypePullRequests // 3 PRs
TypeReleases // 4 Releases
TypeWiki // 5 Wiki
TypeExternalWiki // 6 ExternalWiki
TypeExternalTracker // 7 ExternalTracker
TypeProjects // 8 Kanban board
TypePackages // 9 Packages
TypeActions // 10 Actions
)
Copy link
Member

Choose a reason for hiding this comment

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

💯

Copy link
Contributor Author

Choose a reason for hiding this comment

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

???

@yp05327
Copy link
Contributor Author

yp05327 commented Nov 15, 2023

Need someone to review.

@yp05327
Copy link
Contributor Author

yp05327 commented Jan 18, 2024

Need reviewers.


// find all admin teams
teams := make([]*Team, 0)
err := sess.Where("team.authorize = ?", AccessModeAdmin).Find(&teams)
Copy link
Member

Choose a reason for hiding this comment

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

It may consume much memory for a big instance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

}

if existingUnitTypes.Contains(u) {
sess.Cols("access_mode").Update(newTeamUnit)
Copy link
Member

Choose a reason for hiding this comment

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

returned error haven't been handlered.

if existingUnitTypes.Contains(u) {
sess.Cols("access_mode").Update(newTeamUnit)
} else {
sess.Insert(newTeamUnit)
Copy link
Member

Choose a reason for hiding this comment

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

The same as above.

}
}

return sess.Commit()
Copy link
Member

Choose a reason for hiding this comment

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

You need to detect whether there are still records haven't been committed. Some database will fail if submit a zero change commit.

@yp05327
Copy link
Contributor Author

yp05327 commented Nov 5, 2024

@lunny
You added a doctor to fix DB, so I think the migration is not needed.
Just merge the logic fix (to your PR) is OK I think.
Or should I remove the migration? Then merge into main first.

@yp05327 yp05327 closed this Dec 13, 2024
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Mar 13, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/migrations size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Org admin users can not access org project
4 participants