Fix public activity showing private repos #892
Conversation
Signed-off-by: Morgan Bazalgette <the@howl.moe>
models/repo.go
Outdated
|
|
||
| // If repo has become private, we need to set its actions to private. | ||
| if repo.IsPrivate { | ||
| e.Where("repo_id = ?", repo.ID).Cols("is_private").Update(&Action{ |
There was a problem hiding this comment.
We should check the returned error
| e.Where("repo_id = ?", repo.ID).Cols("is_private").Update(&Action{ | ||
| IsPrivate: true, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Shouldn't we also mark the actions as public if the repo becomes public?
There was a problem hiding this comment.
I've thought about that, and I am not sure whether having a feed that adds new events "in the past" would make much sense.
|
LGTM |
tboerger
added
the
lgtm/need 1
|
It would be good to add tests for bugfixes, had you looked at what it would take ? |
| // If repo has become private, we need to set its actions to private. | ||
| if repo.IsPrivate { | ||
| _, err = e.Where("repo_id = ?", repo.ID).Cols("is_private").Update(&Action{ | ||
| IsPrivate: true, |
There was a problem hiding this comment.
IsPrivate: repo.IsPrivate, ? And remove the if repo.IsPrivate ?
There was a problem hiding this comment.
We already discussed this in the linked comment. As I said, "I am not sure whether having a feed that adds new events "in the past" would make much sense.".
|
@strk, will look into tests later today |
|
LGTM |
tboerger
added
lgtm/done
|
Added a test, as requested by @strk. |
|
LGTM, thanks for the test ! |
Fixes #811