-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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 feature flags to the config for Wiki and Issues #7032
Conversation
The next step for me would be (in another pull request) to try and make pull requests work with issues disabled. |
I think just change |
I thought about that, but I was unsure if the config file is parsed early enough. I do not really know enough about the initialization sequence. When some code path uses I can change the patch set to do exactly what you said, if that's the better solution. |
Do we have any ETA for this feature to be pulled into the master branch? Working in a large organization where Confluence and JIRA are used, such flag would be very helpful. Thanks! |
Codecov Report
@@ Coverage Diff @@
## master #7032 +/- ##
==========================================
- Coverage 41.32% 41.31% -0.01%
==========================================
Files 474 474
Lines 63837 63850 +13
==========================================
- Hits 26379 26378 -1
- Misses 34019 34031 +12
- Partials 3439 3441 +2
Continue to review full report at Codecov.
|
Now the docker test fails because of unrelated issues. :( Can that be re-run or do I need to push an empty commit to the branch?
|
Currently this will totally break options page to enable/disable pull requests if both issue and wiki are disabled, no? IMHO this PR can not be merged if it leaves broken state that no PR can be used in Gitea |
Actually Pull-Requests are working just fine. I don't know about a broken options page? Can you point me to it? |
Never mind ... I found it. |
Signed-off-by: Julian Picht <julian.picht@gmail.com>
Signed-off-by: Julian Picht <julian.picht@gmail.com>
Signed-off-by: Julian Picht <julian.picht@gmail.com>
Signed-off-by: Julian Picht <julian.picht@gmail.com>
Signed-off-by: Julian Picht <julian.picht@gmail.com>
This reverts commit e2eb47569ed4d8cd05f89abac600b4e062f7f8a7. Signed-off-by: Julian Picht <julian.picht@gmail.com>
…Issues Signed-off-by: Julian Picht <julian.picht@gmail.com>
Signed-off-by: Julian Picht <julian.picht@gmail.com>
Signed-off-by: Julian Picht <julian.picht@gmail.com>
Signed-off-by: Julian Picht <julian.picht@gmail.com>
@zeripath I changed the config section to [repository] |
@lunny Somehow the page still shows one of your change request to be open. It somehow got confused, when I rebased the branch on the current master. The requested change has been implemented. |
@@ -1278,6 +1278,9 @@ func createRepository(e *xorm.Session, doer, u *User, repo *Repository) (err err | |||
var units = make([]RepoUnit, 0, len(DefaultRepoUnits)) | |||
for _, tp := range DefaultRepoUnits { | |||
if tp == UnitTypeIssues { | |||
if !setting.Repository.EnableIssues { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wouldn't it be possible to do these changes without changing how unit rights are saved to DB? so that if you later enable enable issues back you would not have to go through all repo teams to enable issues back
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really think this is will happen.
Example: We have a company wiki, and an issue tracker. All we do need gitea for is for managing git repositories.
Enabling wiki and issue functions will - very probably - not happen in this use case.
I also don't see a different use case, where you would want to globally disable the Wiki / Issue functions, do you?
DefaultRepoUnits also does not contain UnitTypeIssues (as by @lunny's request #7032 (comment)). So that even removing that line will not change the behavior.
I can provide a small bash+curl script to enable the wiki and/or issues via the API for inclusion in the documentation, so a user could easily work around this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I mean is what if I already have existing repository and now I disable issues globally, units for existing repositories will be added so there is not really much point in skiping this for new repos as you will have to check anyway later not only if unit is enabled but also new setting. And if for whaterver reason I later want to enable it back it would be better that user would not have to run scripts to fix db to have working instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me it seems this PR is doing two things at the same time
- Disabling the possibility to enable issues/wiki (including default value)
- Disabling any issue/wiki view, even if it was enabled.
Of course in the second case, it would be confusing to still have the settings for enabling it, but there is really no point of affecting the defaults (as @lafriks point out). If only the first case is wanted, there should not be necessary with any changes in routers, as the wiki/issues is anyhow disabled. For a fresh installation I don't think it matter, but the question is how it shall behave for an existing installation if issues/wikis are globally disabled. Either issues/wikis are disabled for existing repos even if they was enabled before the ini setting was changed. Other alternative is to only allow disable issues/wiki for a repo but not enable it. This would correspond to how FORCE_PRIVATE option is working.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I find it more useful with a setting for default repo units. Even if it is a common use-case to not use internal wiki/tracker, you might want to link to an external one.
This PR still leaves it open to enable issues/wiki from API, is that intentional?
I can certainly add "DEFAULT_WIKI_ENABLED" and "DEFAULT_ISSUES_ENABLED" config options, but including this in this pull request would be be scope creep IMO
No, that would not be intentional. I thought I disabled the API routes, but I will check again. |
I think https://try.gitea.io/api/swagger#/repository/repoEdit is still available which allows to enable or disable wiki/issue tracker? And I don't think the route shall be disabled because it would disable any repository setting through API. Rather an http error response shall be returned if trying to enable issue/wiki (alternatively, just ignore the setting). |
@jpicht sorry for late to review. I will review this ASAP. |
@@ -454,7 +454,7 @@ func mustAllowPulls(ctx *context.APIContext) { | |||
} | |||
|
|||
func mustEnableIssuesOrPulls(ctx *context.APIContext) { | |||
if !ctx.Repo.CanRead(models.UnitTypeIssues) && | |||
if (!setting.Repository.EnableIssues || !ctx.Repo.CanRead(models.UnitTypeIssues)) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So even the repository created some issues, this will still hide them when setting.Repository.EnableIssues
changed to true?
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions. |
@jpicht are you still working on this? |
We are running this in production, so I would be happy to get it merged. I've ported it to the latest rc one or two weeks back. The problem with merging though, seems to mostly be to define what is supposed to happen in the corner cases, that (IMO) would probably never happen: re-enabling issues/wiki after disabling them. I do only see the use case that my company has: we already have a wiki and an issue tracker, we do not want to confuse people with a second one, that they are supposed to ignore. If there's another use case I'd be happy to accommodate that in my patch. I'll push an updated version in the next few days. |
@jpicht In the meantime, you could cheat the feature by using a custom template that hides Wikis and Issues from the menu. |
I don't think the corner cases is a big problem. My main comment on this PR is that I think it solves a too specific use case. Shouldn't we have a system where any repo unit can be globally disabled? (Maybe a bit connected to the plugin discussions). So the app.ini could be something like I don't mean disabling of all units need to be implemented in the same PR, just that it would be good to think about the structure. Sorry if I am being discouraging, I really see the use of being able to globally disable certain units. In my organization we don't use the internal issue and wiki, so it would be good to be able to disable those (we have a script to turn them off at creation). The problem is that this PR also switches off other units with the same flag options (external tracker and wiki), which makes it much less useful. |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions. |
This pull request aims to close #3336, implementing ENABLE_WIKI and ENABLE_ISSUES config options (defaulting to true).
It would probably be good if somebody, who really knows how the Issue system works, had a close look over what I did and if there are some places that I missed (maybe pull-request related).
Currently there are two unrelated commits in this branch, that I needed to make the Makefile work for me. I can remove them, if they pose a problem.