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 owner team access mode value in team_unit table #23675

Merged
merged 10 commits into from
Apr 3, 2023

Conversation

yp05327
Copy link
Contributor

@yp05327 yp05327 commented Mar 24, 2023

All access_mode value of Owner Teams are 0(AccessModeNone) in team_unit table, which should be 4(AccessModeOwner)
In team table:
image
In team_unit table:
image

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.

@lunny lunny added type/bug outdated/backport/v1.19 This PR should be backported to Gitea 1.19 labels Mar 24, 2023
@lunny lunny added this to the 1.20.0 milestone Mar 24, 2023
@codecov-commenter
Copy link

codecov-commenter commented Mar 24, 2023

Codecov Report

Merging #23675 (fe0e75c) into main (f521e88) will increase coverage by 0.00%.
The diff coverage is 41.80%.

❗ Current head fe0e75c differs from pull request most recent head 54b3d7c. Consider uploading reports for the commit 54b3d7c to get more accurate results

@@           Coverage Diff            @@
##             main   #23675    +/-   ##
========================================
  Coverage   47.14%   47.14%            
========================================
  Files        1149     1155     +6     
  Lines      151446   152397   +951     
========================================
+ Hits        71397    71848   +451     
- Misses      71611    72076   +465     
- Partials     8438     8473    +35     
Impacted Files Coverage Δ
cmd/dump.go 0.66% <0.00%> (-0.01%) ⬇️
cmd/web.go 0.00% <0.00%> (ø)
models/actions/run.go 1.64% <0.00%> (-0.08%) ⬇️
models/actions/runner.go 1.44% <ø> (ø)
models/packages/package.go 45.45% <0.00%> (-1.13%) ⬇️
models/user/search.go 77.50% <0.00%> (-6.29%) ⬇️
modules/actions/workflows.go 0.00% <0.00%> (ø)
modules/context/context.go 64.54% <0.00%> (-3.53%) ⬇️
modules/doctor/storage.go 30.65% <0.00%> (-1.29%) ⬇️
modules/setting/git.go 45.45% <ø> (ø)
... and 38 more

... and 44 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 24, 2023
@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 24, 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 Apr 3, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 3, 2023
@jolheiser jolheiser enabled auto-merge (squash) April 3, 2023 13:52
@jolheiser jolheiser merged commit 6eb856c into go-gitea:main Apr 3, 2023
@GiteaBot
Copy link
Contributor

GiteaBot commented Apr 3, 2023

I was unable to create a backport for 1.19. @yp05327, please send one manually. 🍵

@GiteaBot GiteaBot added the backport/manual No power to the bots! Create your backport yourself! label Apr 3, 2023
@go-gitea go-gitea deleted a comment from GiteaBot Apr 3, 2023
@jolheiser jolheiser removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 3, 2023
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 4, 2023
* upstream/main:
  Use User.ID instead of User.Name in ActivityPub API for Person IRI (go-gitea#23823)
  Remove fomantic ".link" selector and styles (go-gitea#23888)
  [skip ci] Updated translations via Crowdin
  Fix `cases.Title` crash for concurrency (go-gitea#23885)
  Disable editing tags (go-gitea#23883)
  Fix user profile description rendering (go-gitea#23882)
  Introduce GiteaLocaleNumber custom element to handle number localization on pages. (go-gitea#23861)
  Convert .Source.SkipVerify to $cfg.SkipVerify (go-gitea#23839)
  Fix review box viewport overflow issue (go-gitea#23800)
  Fix owner team access mode value in team_unit table (go-gitea#23675)
  Fix submit button won't refresh in New Repository Fork page (go-gitea#22994)
  Introduce GitHub markdown editor, keep EasyMDE as fallback (go-gitea#23876)
  Improve LoadUnitConfig to handle invalid or duplicate units (go-gitea#23736)
  Append `(comment)` when a link points at a comment rather than the whole issue (go-gitea#23734)
  Rename actions unit to `repo.actions` and add docs for it (go-gitea#23733)
  Try to catch more broken translations (go-gitea#23867)
@silverwind
Copy link
Member

silverwind commented Apr 9, 2023

Fixes #23474? I guess a backport would be nice if it does so I no longer need that workaround that I currently execute via cron every day 😆.

@yp05327
Copy link
Contributor Author

yp05327 commented Apr 10, 2023

Fixes #23474? I guess a backport would be nice if it does so I no longer need that workaround that I currently execute via cron every day 😆.

It seems that we found this bug in a different way.
Your issue will be fixed if it is only caused by the incorrect AccessMode in TeamUnit table.

@lunny
Is it safe to only backport the code here?
https://github.com/go-gitea/gitea/pull/23675/files#diff-223cf46382a22079a281b568e5cfcb9648ed6a200d8768ea3283354205f1b508R341-R344

@lunny
Copy link
Member

lunny commented Apr 10, 2023

Fixes #23474? I guess a backport would be nice if it does so I no longer need that workaround that I currently execute via cron every day 😆.

It seems that we found this bug in a different way.

Your issue will be fixed if it is only caused by the incorrect AccessMode in TeamUnit table.

@lunny

Is it safe to only backport the code here?

https://github.com/go-gitea/gitea/pull/23675/files#diff-223cf46382a22079a281b568e5cfcb9648ed6a200d8768ea3283354205f1b508R341-R344

We can partially back port. All changes except migration could be back port.

@yp05327
Copy link
Contributor Author

yp05327 commented Apr 13, 2023

@lunny
I found a problem when working on #24012

In #18675:

Don't allow TypeExternal{Tracker,Wiki} to influence this as they can only be set to READ perms.

So for owner team or admin team, Tracker and Wiki unit accessmode should be 1(read) in db?

6543 pushed a commit that referenced this pull request Apr 13, 2023
6543 pushed a commit that referenced this pull request Apr 15, 2023
…24117)

Fix the incorrect migration in #23675 and #24012

External Unit (Tracker and Wiki) access mode should be `read` in
owner/admin team.
silverwind added a commit that referenced this pull request Apr 19, 2023
Add test for #23675

Should be merged after #24117

---------

Co-authored-by: silverwind <me@silverwind.io>
silverwind pushed a commit that referenced this pull request Apr 22, 2023
Partly backport #23675

Co-authored-by: Giteabot <teabot@gitea.io>
@lunny lunny added the backport/done All backports for this PR have been created label Jul 26, 2023
@GiteaBot
Copy link
Contributor

GiteaBot commented Aug 1, 2023

We lock pull requests 3 months after they were closed. If there's any need for further discussion, please open a new issue. 🍵

@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created backport/manual No power to the bots! Create your backport yourself! lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. outdated/backport/v1.19 This PR should be backported to Gitea 1.19 type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants