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

Add config options to hide issue events #17414

Merged
merged 52 commits into from
Jan 21, 2022
Merged

Add config options to hide issue events #17414

merged 52 commits into from
Jan 21, 2022

Conversation

qwerty287
Copy link
Contributor

@qwerty287 qwerty287 commented Oct 23, 2021

Adds config options to the user settings to hide most issue events (changed labels, milestones, projects...) on the issue detail page.

Closes #11418

Adds a config option `HIDE_ISSUE_EVENTS` to hide most issue events (changed labels, milestones, projects...) on the issue detail page.
If this is true, only the following events (comment types) are shown:
* plain comments
* closed/reopned/merged
* reviews
@codecov-commenter

This comment has been minimized.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 23, 2021
custom/conf/app.example.ini Outdated Show resolved Hide resolved
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Oct 24, 2021

I think we should group these events into one line, instead of hiding them.

GitHub does a good example.

==

Update: I just found that the grouping feature had been implemented.

In my opinion, we should not use a global setting option to hide some events, it brings divergences in future, some one wants to hide this, some one else wants to hide others.

@qwerty287
Copy link
Contributor Author

@wxiaoguang if you take a look at the linked issue, you'll find this comment:

I mean when you create serval labels for one issue, we could just display one label comment.

done in #13304 🎉 - but I'll leave it open since this propose "hide all event comments"

@lafriks
Copy link
Member

lafriks commented Oct 24, 2021

Why not instead of adding additional app setting just make it as user preference? (There was already PR to move user settings to new table so it should not be too hard to add new setting)

@qwerty287
Copy link
Contributor Author

qwerty287 commented Oct 24, 2021

@lafriks this PR is AFAIK not merged yet, so this would be blocked until then and this would be probably incompatible to the list suggested by @lunny above (which I implemented already)

@lafriks
Copy link
Member

lafriks commented Oct 24, 2021

I don't see why it could not be saved as json un user settings and implemented as checkboxes.

My point is we already have way too many settings in app.ini and we should avoid adding more if that's possible imho

@qwerty287
Copy link
Contributor Author

Yes in general I agreed (also because not every user on an instance must like it if it's disabled) but I wasn't sure about the implementation, but I didn't thought about the way using a JSON, which is fine. So this is probably blocked by #16834

@qwerty287
Copy link
Contributor Author

The only thing I'd like to know is to which page in the settings I'd add it, since it doesn't really fit one of the current pages

@noerw
Copy link
Member

noerw commented Oct 24, 2021

imho the user settings page is missing an "appearance" tab. then we can move theme, language from the other (unrelated) tabs as well.

@qwerty287
Copy link
Contributor Author

I added a appearance tab in a separate branch, https://github.com/qwerty287/gitea/tree/appearance-prefs, and could open a PR for this if you like it :)

@lafriks
Copy link
Member

lafriks commented Oct 25, 2021

I added a appearance tab in a separate branch, https://github.com/qwerty287/gitea/tree/appearance-prefs, and could open a PR for this if you like it :)

Yes, please, do so

@qwerty287
Copy link
Contributor Author

A maintainer might add the status/blocked label?

@wxiaoguang wxiaoguang added pr/wip This PR is not ready for review status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR labels Oct 26, 2021
@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 Dec 27, 2021
@qwerty287 qwerty287 requested a review from Gusted January 7, 2022 16:15
Copy link
Contributor

@Gusted Gusted left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few things, but overall it looks good to me.

models/user/setting_keys.go Show resolved Hide resolved
routers/web/repo/issue.go Show resolved Hide resolved
services/forms/user_form_hidden_comments.go Outdated Show resolved Hide resolved
@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 Jan 8, 2022
@wxiaoguang wxiaoguang added this to the 1.17.0 milestone Jan 16, 2022
@qwerty287
Copy link
Contributor Author

Would anybody mind merging this?

@qwerty287
Copy link
Contributor Author

Of course only after I resolved conflicts...

@qwerty287
Copy link
Contributor Author

Merged and resolved conflicts, ready for merge.

@6543 6543 merged commit 1f40933 into go-gitea:main Jan 21, 2022
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jan 22, 2022
* giteaoffical/main:
  [skip ci] Updated translations via Crowdin
  Add config options to hide issue events (go-gitea#17414)
  Add js vendor directory to .gitattributes (go-gitea#18350)
@qwerty287 qwerty287 deleted the hide-issue-events branch January 24, 2022 14:50
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
* Add config option to hide issue events
Adds a config option `HIDE_ISSUE_EVENTS` to hide most issue events (changed labels, milestones, projects...) on the issue detail page.
If this is true, only the following events (comment types) are shown:
* plain comments
* closed/reopned/merged
* reviews

* Make configurable using a list

* Add docs

* Add missing newline

* Fix merge issues

* Allow changes per user settings

* Fix lint

* Rm old docs

* Apply suggestions from code review

* Use bitsets

* Rm comment

* fmt

* Fix lint

* Use variable/constant to provide key

* fmt

* fix lint

* refactor

* Add a prefix for user setting key

* Add license comment

* Add license comment

* Update services/forms/user_form_hidden_comments.go

Co-authored-by: Gusted <williamzijl7@hotmail.com>

* check len == 0

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: Gusted <williamzijl7@hotmail.com>
Co-authored-by: 6543 <6543@obermui.de>
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
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. topic/issues type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Hide issuecomment events