10000 Calculate `PublicOnly` for org membership only once by 6543 · Pull Request #32234 · go-gitea/gitea · GitHub
[go: up one dir, main page]
Skip to content

Calculate PublicOnly for org membership only once #32234

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 30 commits into from
Nov 11, 2024
Merged

Calculate PublicOnly for org membership only once #32234

merged 30 commits into from
Nov 11, 2024

Conversation

6543
Copy link
Member
@6543 6543 commented Oct 10, 2024 •
edited
Loading

Refactoring of #32211

this move the PublicOnly() filter calcuation next to the DB querys and let it be decided by the Doer

Sponsored by Kithara Software GmbH

@6543 6543 added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Oct 10, 2024
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 10, 2024
@github-actions github-actions bot added modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code labels Oct 10, 2024
@delvh delvh changed the title Make calculation of PublicOnly filter for OrgMembership in single place Calculate PublicOnly for org membership only once Oct 10, 2024
delvh
delvh reviewed Oct 10, 2024
View reviewed changes
@pull-request-size pull-request-size bot added size/L and removed size/M labels Oct 11, 2024
@6543 6543 marked this pull request as ready for review October 11, 2024 17:31
@6543 6543 requested a review from delvh October 11, 2024 17:31
@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 Oct 12, 2024
lunny
lunny reviewed Oct 13, 2024
View reviewed changes
@lunny
Copy link
Member
lunny commented Oct 13, 2024

As a habit, we almost check permission in service layer but not in models layer. What's the problem with the previous code?

@6543
Copy link
Member Author
6543 commented Oct 13, 2024

I can create a new func in the service witch checks permissions ... but we would have to alter the existing code a lot and i want to only touch it here for a explizite context ... : the refactoring that makes the #32211 diff smal

@6543
Copy link
Member Author
6543 commented Oct 13, 2024
8000

What's the problem with the previous code?

the determination if a user can see public or private org membership is determined in different places ... I want to have that check in a single place ...

@6543
Copy link
Member Author
6543 commented Oct 20, 2024

#32234 (comment)

this needs an answer to move forward from @lunny ...

@6543
Copy link
Member Author
6543 commented Oct 22, 2024

@lunny made 7bb645e here and 8780d12 ...

lunny
lunny reviewed Oct 26, 2024
View reviewed changes
@6543 6543 requested a review from lunny October 28, 2024 17:05
@6543
Copy link
Member Author
6543 commented Oct 31, 2024

@lunny anything else?

@6543 6543 requested a review from techknowlogick November 10, 2024 21:56
lunny
lunny reviewed Nov 10, 2024
View reviewed changes
@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 Nov 10, 2024
@6543 6543 merged commit 43c252d into go-gitea:main Nov 11, 2024
26 checks passed
@GiteaBot GiteaBot added this to the 1.24.0 milestone Nov 11, 2024
@6543
Copy link
Member Author
6543 commented Nov 11, 2024

thanks ❤️ !

@6543 6543 deleted the refactor-of-32211 branch November 11, 2024 00:38
zjjhot added a commit to zjjhot/gitea that referenced this pull request Nov 11, 2024
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
40a30db 
* giteaofficial/main:
  Calculate `PublicOnly` for org membership only once (go-gitea#32234)
  Add `DEFAULT_MIRROR_REPO_UNITS` and `DEFAULT_TEMPLATE_REPO_UNITS` options (go-gitea#32416)
  Add a doctor check to disable the "Actions" unit for mirrors (go-gitea#32424)
  Refactor sidebar assignee&milestone&project selectors (go-gitea#32465)
@lunny lunny modified the milestones: 1.24.0, 1.23.0 Nov 13, 2024
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Feb 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lunny lunny lunny approved these changes

@delvh delvh delvh approved these changes

@techknowlogick techknowlogick Awaiting requested review from techknowlogick

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Milestone
1.23.0
Development

Successfully merging this pull request may close these issues.
4 participants
@6543 @lunny @delvh @GiteaBot
0