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

Allow to set organization visibility (public, internal, private) #1763

Merged
merged 167 commits into from
Feb 18, 2019

Conversation

DblK
Copy link
Member

@DblK DblK commented May 19, 2017

This will implement a way to have a visibility on organization (Public, Limited & Private).
It will implement #714 .

The UI modification is in settings:
image

The rules for displaying organization and repositories within organization are described in #714

@lunny lunny added type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first. labels May 19, 2017
@lunny lunny added this to the 1.3.0 milestone May 19, 2017
@bkcsoft bkcsoft added the pr/wip This PR is not ready for review label May 20, 2017
@bkcsoft bkcsoft changed the title [WIP] Feature allow private organization Feature allow private organization May 20, 2017
@DblK
Copy link
Member Author

DblK commented May 22, 2017

UI modifications in Admin:
image

Finally both Explore/Organizations and Explore/Repositories are filtered to display the right organizations.

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 22, 2017
@camlafit
Copy link
Contributor

Hi

Looks great but :)

  • we have a problem with administrator account, it can manage organization policy from admin/orgs but lost public access, it can see all private organization from explore/organizations* . It looks logical but seems strange behavior. I don't have any idea to improve this behavior.
  • from explore/organizations page , I get this error going to any organization page :
template: explore/repo_list:3:21: executing "explore/repo_list" at <.Owner.Visibility>: can't evaluate field Visibility in type *models.User
  • on admin/orgs miss an icon status as done in explore/organizations
  • all user are displayed on explore/users without user connected
  • Default organization status is OK, a new organization take service.DEFAULT_VISIBILITY value

@camlafit
Copy link
Contributor

Hi

Looks also a problem about displaying repo and orgnizaiton. An user set as repo owner can't see neither orgnaization page, neither repo page (404).
On another organization set also as private it's ok. On dashboard user all commits are displayed.

log show this :

2017/05/22 17:28:49 [...ules/context/repo.go:527 func1()] [E] CheckUnit: You have not allowed to visit this repository unit: 1
2017/05/22 17:28:52 [...routers/repo/view.go:270 Home()] [E] Home: Cannot find any unit allowed on this repository
2017/05/22 17:28:54 [...ules/context/repo.go:527 func1()] [E] CheckUnit: You have not allowed to visit this repository unit: 1

@DblK
Copy link
Member Author

DblK commented May 22, 2017

Thanks @camlafit for your tests.
So here are my answers:

  1. That's true that admin can see everything on explore/organizations. If I understand what you said, you wish admin can be both admin and public? In others words, you wish admin can check if he wants standard user view or not? If yes, I think admin should stay admin. I don't see how to change that. Maybe a new user settings for admin only to display as "non admin user"? If yes, another PR will be done for that because I think it's an enhancement of what is done here.

  2. I saw the issue. It is not happening when going an organization has no repository. I saw the bug and I will fix it.
    EDIT! Fixed in next commit

  3. I did not change this page so it is normal. I will add the icon for private organizations.
    EDIT: Here are the change done to admin/orgs and admin/repos
    image
    image

  4. I think this should be made in another PR. Displaying all users are not part of this feature. Please made another issue and I will try to fix it too. Please explain what you want to have. I bet you wish to have something like a private or limited user.

  5. Glad to hear it is working. It was not on your issue but I thought it can be useful.

  6. So, the user is not the creator of the organization but has been part of the Owner team after that? Could you explain more about the two different use case so I can reproduce and fix it before merging.

  7. I will review the dashboard to ensure nothing appears. Or I think it is working as expected but when user left the organization they still see in the dashboard. If yes, it is not related to this PR but should be done as another issue and I will do the PR.

If you have other remarks, feel free to add more. I will dig into the first ones you gave me ;)

@camlafit
Copy link
Contributor

Hi

Thanks for improvement

1/ As admin we can see all organization from explore/organizations andb loooks ok. But all link target a 404 this part doesn't look normal. We should find a 403 forbidden page or a redirection to admin organization page.
Now about organization page look ok, empty or not page is displayed. But link to repo target a 404.

2/ looks solved :)

3/ great looks better as this 👍

4/ yes looks better to split #714 I've opened #1780 about user displaying

5/ yes it's useful. On my case all organization are private with unique exception. Easier as this.

6/ dashboard user looks updated, organization with pb is no more displayed.

7/ I've an organization set as Private, with an unique team "owner" and unique member and unique repo. On his dashboard commits are displayed, but it's not possible to go to their repository, 404 page.
Configuration looks same as others organizations but I don't understand why I get this 404.
Maybe in this case user was creator and not set after as owner. I don't remember.

I've two accounts , an administrator and a personal. In my process I prepare organization and repo from admin account. I add standard account as owner
Looks possible I've create only organization in this case and create repo from normal user. I don't remember. It's possible also to create repo in personnal space and move after to organization. Sorry I don't remember. I don't know if I see history change about this orga/repo.

@bkcsoft bkcsoft removed the pr/wip This PR is not ready for review label May 23, 2017
@DblK
Copy link
Member Author

DblK commented May 23, 2017

I could not see the problem with 1) nor 7).
From my point of view, unless you can provide me a use case to reproduce it each time, this PR is done and waiting for the review of code.

@DblK
Copy link
Member Author

DblK commented May 24, 2017

According to our discussion (@camlafit) and my investigation, the error is not part of this PR (see #1794).
This PR is good to go ^^

@DblK
Copy link
Member Author

DblK commented May 24, 2017

For the change in the UI in explore/repo to add the lock close to private organization:
image

@camlafit
Copy link
Contributor

camlafit commented May 24, 2017

Hi

Looks again a pb. If user is only organization member, repositories are not lister.
Repositories are displayed only if user is set in owner team.

@DblK
Copy link
Member Author

DblK commented May 24, 2017

Your problem is not specific to this PR but to the master. Look like you should add a comment for #1794.
This is related to the implementation of Units. No repositories are show but public.

Please test it also on master to be sure the bug is from this PR. If it is not working on master, please fill a new issue or complete an existing one.

So for me, so far no bug from this PR (some but inherited from master).

@lunny
Copy link
Member

lunny commented Aug 28, 2017

need rebase and more tests

@lunny lunny removed this from the 1.3.0 milestone Oct 17, 2017
@techknowlogick
Copy link
Member

@lunny resolved conflict with recent settings module change. please review.

@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 Feb 17, 2019
@DblK
Copy link
Member Author

DblK commented Feb 17, 2019

Thx @techknowlogick for finishing/rebasing my PR. I will love to use this feature.
Finally I will be able to open my own Gitea instance to friends :p

@lafriks lafriks merged commit 64ce159 into go-gitea:master Feb 18, 2019
@lafriks lafriks changed the title Feature allow private organization Allow to set organization visibility (public, internal, private) Feb 18, 2019
@nicorac
Copy link

nicorac commented Feb 18, 2019

Just upgraded to 64ce159 and noticed an issue in Organizations list admin page (/admin/orgs).

Opened an issue here: #6111

@techknowlogick
Copy link
Member

@DblK 😄 Thanks for starting the PR.

@lafriks lafriks removed the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Feb 19, 2019
@lunny lunny mentioned this pull request Feb 19, 2019
6 tasks
@DblK
Copy link
Member Author

DblK commented Apr 23, 2019

Good work @techknowlogick. Finally landed in a release ;)

@camlafit
Copy link
Contributor

Works as a charm \o/

@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
@delvh delvh removed the type/changelog Adds the changelog for a new Gitea version label Oct 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.