-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Fix PR, milestone and label functionality if issue unit is disabled #2710
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2710 +/- ##
=======================================
Coverage 26.95% 26.95%
=======================================
Files 87 87
Lines 17301 17301
=======================================
Hits 4664 4664
Misses 11957 11957
Partials 680 680 Continue to review full report at Codecov.
|
9a03cc8
to
d2237b9
Compare
LGTM |
routers/repo/issue.go
Outdated
prUnitEnabled := ctx.Repo.Repository.UnitEnabled(models.UnitTypePullRequests) | ||
for _, issue := range issues { | ||
if issue.IsPull && !prUnitEnabled || !issue.IsPull && !issueUnitEnabled { | ||
ctx.Handle(404, "UnitEnabled", nil) |
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.
Same here.
routers/repo/issue.go
Outdated
} | ||
if issue.IsPull && !ctx.Repo.Repository.UnitEnabled(models.UnitTypePullRequests) || | ||
!issue.IsPull && !ctx.Repo.Repository.UnitEnabled(models.UnitTypeIssues) { | ||
ctx.Handle(404, "UnitEnabled", nil) |
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.
Error message should be UnitDisabled
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.
@daviian no, there should be function name that returned error so that from log entries it would be possible to understand what happened
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.
From that point of view yes, but it is confusing at the first moment.
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.
That is how it is used in rest of the code :)
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.
@lafriks However in this case the UnitEnabled doesn't even return an error and 404 happens if the required unit is not enabled.
@daviian updated ;) |
@lafriks Update and delete on comments in PR without issue tracker don't work either. |
@daviian fixed |
LGTM |
Make LG-TM work again |
…o-gitea#2710) * Fix PR, milestone and label functionality if issue unit is disabled or not assigned to user * Fix multi-actions in PR page * Change error message * Fix comment update and delete functionality in PR
Fix PR, milestone and label functionality if issue unit is disabled or not available for user
Fixes #2447