Add activity visibility options #29306
Conversation
|
|
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
models/organization/org_user.go
Outdated
| @@ -77,6 +77,27 @@ func IsOrganizationMember(ctx context.Context, orgID, uid int64) (bool, error) { | |||
| Exist() | |||
| } | |||
|
|
|||
| // IsOrganizationsMember returns a map with key of orgID and value is true if given user is member of organization. | |||
| func IsOrganizationsMember(ctx context.Context, orgIDs []int64, uid int64) (map[int64]bool, error) { | |||
There was a problem hiding this comment.
The permission of organization is complex. Not all users can see all repos of the org.
e.g. a member in a team which has no repo access, then members can also see the actions of some private repos they can not access.
Not sure whether we need to consider this case, but actually it exists.
And maybe we can add some integration tests for checking all cases of different permissions.
@silverwind
What's your idea about this?
|
DB migration is necessary, plz add it. |
Yes, but I wait for any updates on https://github.com/go-gitea/gitea/pull/29306/files/583357a10b4760a2e2c57e69c0bdd9ad55a3aaf5#r1512068444 |
I think it needs enough tests to show and ensure the expected behaviors, and make sure the logic won't break by future changes. The tests could also answer the question in that comment. |
|
if this is ok for @wxiaoguang and @yp05327 then I will add the migration |
| } | ||
|
|
||
| // String provides the mode string of the visibility type (public, all, none) | ||
| func (vt ActivityVisibility) String() string { |
There was a problem hiding this comment.
Is this "String" method really used? I think it only causes problems. For example: it's impossible to render the value directly into template.
So IMO either:
- Remove such "String" method, and then just output the int value into template directly
- Or if it is really needed, name it to "ToString" or something else, do not make it a "Stringer"
Would there be some tests to test the new behavior? |
I have tested it manually at that time. And I found this issue. IMO, it is a bug. |
Add options to which actions are shown (relates to #21585)
before:
after:
Missing