The column namespaces.public might not be necessary
Commit 6051c28f added the ability for project owners to determine if a group should show up in search results or not, regardless of whether they had any public/internal projects.
After some discussions with @dzaporozhets and others it appears that this column might not be needed at all as per @dzaporozhets pages such as the "Explore" page can just show all groups (meaning we can just use Group.all). This would also remove the need for an index on namespaces.public (which I added manually for gitlab.com for the time being due to the lack thereof burning down the DB).
In merge request !1790 (merged) the code I changed honors this public column, removing the column means we can also simplify some logic in said MR.
So the question is: can we simply remove this column and the corresponding code, or should we keep things as-is?
Slack conversation about this:
Yorick Peterse [16:42]
So just to confirm, groups are always visible, regardless of their projects, whether somebody is a member or whether they're logged in?
Dmitriy Zaporozhets [16:43]
name and path is public
[16:43]
membership - not
[16:43]
basically everything except avatar, name and path is private
Yorick Peterse [16:44]
ok so say Alice is a member of a private project, can Bob see that project's group on Alice's profile page?
[16:44]
(if Bob is not a member of the private project)
Dmitriy Zaporozhets [16:45]
I think no
[16:45]
otherwise its expose of membership
Yorick Peterse [16:45]
Ah ok, so in that case the logic stays mostly the same
[16:45]
That just means the "public" column is not needed I think?
[16:45]
Right now it works like this:
Douwe Maan [16:46]
@yorickpeterse: The public column was added so that the group shows up in the “Share Project With Group” dropdown in EE when the group has no public projects(edited)
Yorick Peterse [16:46]
hmm
[16:46]
ok in that case I think the logic has to stay as-is
[16:46]
Then we just need to add an index on that column
[16:47]
I'll write down how the logic works now, just a sec
Dmitriy Zaporozhets [16:47]
why not show all groups in “Share Project With Group” dropdown in EE ?
[16:47]
why we need separate checkbox
[16:48]
I dont get this complication
Douwe Maan [16:48]
@dzaporozhets: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/47
GitLab Enterprise Edition
Dmitriy Zaporozhets [16:48]
if all groups are public - it should appear everywhere
Douwe Maan [16:49]
Showing it everywhere makes sense to me though
Yorick Peterse [16:49]
When logged in:
1. Bob visits Alice's profile page
2. Bob sees the following groups Alice is a member of:
A: All groups that have public or internal projects
B: All groups that both Alice and Bob are a member of (either directly or
via any private projects)
When logged out:
1. Bob visits Alice's profile page
2. Bob sees the following groups Alice is a member of:
A: All groups that have public projects, internal projects are ignored
Dmitriy Zaporozhets [16:50]
@yorickpeterse: For example Explore -> Groups can use Group.all instead of detecting what groups are public
[16:50]
@yorickpeterse: right.
[16:51]
So this public field makes no sense to me
Yorick Peterse [16:51]
But so can or can't this be changed to just "Bob sees all groups Alice is a member of", or would it still have to filter out membership via a private project?
Dmitriy Zaporozhets [16:51]
just bring extra confusing
[16:51]
no membership is private
[16:52]
Except profile -> we should render all groups
On profile -> only case described by Yorick above
Yorick Peterse [16:52]
Ah ok
[16:53]
Also ffs, my build is stuck _again_ in cloning
[16:53]
That's like the 4th time now
Dmitriy Zaporozhets [16:53]
All groups:
* search
* explore page
* whatever selectbox with groups where membership does not matter
Yorick Peterse [16:54]
Was the `public` field added via an MR in EE or CE?
Dmitriy Zaporozhets [16:55]
I think CE
Yorick Peterse [16:55]
If it was added in EE then this code isn't even going to work for CE users since the column won't exist
[16:56]
Ah yes, it was added in CE
[16:56]
Hm, this also comes with some UI changes
[16:57]
I'll create an issue for this so we can discuss stuff there