Feature #31589
closed
Show warning and the reason when the issue cannot be closed because of open subtasks or blocking open issue(s)
0%
Description
You cannot close an issue if it has open subtasks (#10989).
This rule is not obvious and difficult to know for users. In my company, "why I cannot close this issue?" is an FAQ. When they come across the behavior, they spend a long time to check workflows. Of course, there is no problem with workflows. It is really hard for ordinary users to find the cause of the behavior.
I think it will be a great help to prevent confusing users if a warning icon with a tooltip is displayed on the edit issue form like the following screenshot.
Files
Related issues
Updated by Go MAEDA almost 5 years ago
- Related to Feature #10989: Prevent parent issue from being closed if a child issue is open added
Updated by Marius BĂLTEANU almost 5 years ago
Did you start work on this? or someone from your team? If not, I can do it considering that I was the one who implemented the restriction.
Updated by Go MAEDA almost 5 years ago
- Assignee set to Marius BĂLTEANU
Marius BALTEANU wrote:
Did you start work on this? or someone from your team? If not, I can do it considering that I was the one who implemented the restriction.
No one in my team has started work yet. I am appreciative that you will work on this!
Updated by David L over 4 years ago
It would also be very helpful to see sub-tasks in the roadmap. They might be shown in rows under the parent that could be expanded/hidden by clicking on the parent task.
At least, there should be some indication that a particular, open issue has sub-tasks.
Updated by Toshi MARUYAMA over 4 years ago
- Related to Feature #28492: Option not to block closing a parent issue when it has open subtask(s) added
Updated by Toshi MARUYAMA over 4 years ago
- Related to Feature #31322: Provide a way to automatically close all open subtasks too when a parent issue is being closed added
Updated by Toshi MARUYAMA over 4 years ago
I think if #31322 is implemented, this issue is not needed.
Updated by Go MAEDA over 4 years ago
- Related to Feature #32757: Show warning when an issue cannot be closed because of blocking issue relations added
Updated by Mitsuyoshi Kawabata over 4 years ago
Updated by Toshi MARUYAMA over 4 years ago
- File issue-31589-poc.diff issue-31589-poc.diff added
Yes, this is very very simple.
This is POC patch, so there is no test code.
Updated by Marius BĂLTEANU over 4 years ago
- File 0001-Show-an-warning-with-the-reason-for-why-an-issue-can.patch added
Toshi MARUYAMA wrote:
Yes, this is very very simple.
This is POC patch, so there is no test code.
Thanks Toshi for the POC, but I tried to use another approach in order to avoid keeping the logic in multiple places. I'm attaching the results, feedback is welcome!
All tests pass: https://gitlab.com/redmine-org/redmine/pipelines/107661848
Updated by Marius BĂLTEANU over 4 years ago
- Assignee deleted (
Marius BĂLTEANU) - Target version set to Candidate for next major release
Updated by Go MAEDA over 4 years ago
Marius, thank you for posting the patch that also implements #32757. I liked it.
I think it can be a part of the core if ":notive_issue_not_closable_by_*" in the patch is replaced with ":notice_issue_not_closable_by_*".
Updated by Marius BĂLTEANU over 4 years ago
- File deleted (
0001-Show-an-warning-with-the-reason-for-why-an-issue-can.patch)
Updated by Marius BĂLTEANU over 4 years ago
- File 0001-Show-an-warning-with-the-reason-for-why-an-issue-can.patch added
- Subject changed from Show warning when the issue cannot be closed because of open subtasks to Show warning and the reason when the issue cannot be closed because of open subtasks or blocking issue
Go MAEDA wrote:
Marius, thank you for posting the patch that also implements #32757. I liked it.
I think it can be a part of the core if ":notive_issue_not_closable_by_*" in the patch is replaced with ":notice_issue_not_closable_by_*".
Oh, sorry for the typo. I've fixed it in the attached patch.
Updated by Marius BĂLTEANU over 4 years ago
- File deleted (
0001-Show-an-warning-with-the-reason-for-why-an-issue-can.patch)
Updated by Marius BĂLTEANU over 4 years ago
- File 0001-Show-an-warning-with-the-reason-for-why-an-issue-can.patch 0001-Show-an-warning-with-the-reason-for-why-an-issue-can.patch added
- Target version changed from Candidate for next major release to 4.2.0
Updated by Marius BĂLTEANU over 4 years ago
- Related to deleted (Feature #32757: Show warning when an issue cannot be closed because of blocking issue relations)
Updated by Marius BĂLTEANU over 4 years ago
- Has duplicate Feature #32757: Show warning when an issue cannot be closed because of blocking issue relations added
Updated by Toshi MARUYAMA over 4 years ago
Marius BALTEANU wrote:
Toshi MARUYAMA wrote:
Yes, this is very very simple.
This is POC patch, so there is no test code.Thanks Toshi for the POC, but I tried to use another approach in order to avoid keeping the logic in multiple places. I'm attaching the results, feedback is welcome!
All tests pass: https://gitlab.com/redmine-org/redmine/pipelines/107661848
I think it is very strange warning icon always shows.
I think it should be in only following cases.
- User can change parent isses status to closed nevertheless parent issue has no open subtasks
- User has only invisible open subtasks
Updated by Mischa The Evil over 4 years ago
- Subject changed from Show warning and the reason when the issue cannot be closed because of open subtasks or blocking issue to Show warning and the reason when the issue cannot be closed because of open subtasks or blocking open issue(s)
- Category changed from Issues to UI
Marius BALTEANU wrote:
[...] I tried to use another approach in order to avoid keeping the logic in multiple places. I'm attaching the results, feedback is welcome!
I like this approach. Very neat. It also got me thinking about what we're trying to solve here though. So I took a step back to look at the bigger picture. I'll elaborate.
The two hereby patched cases are part of a subset of cases, which itself ultimately is another subset of the way we deal with "unusable" statuses. The following list reflects this structure:
0. Unusable statuses
1. status not available before validation
1.1. statuses marked as is_closed
1.1.1. workflow status transition rules
1.1.2. open subtasks (#10989)
1.1.3. open blocking issues (#1740)
1.2. statuses marked not as is_closed
1.2.1. workflow status transition rules
1.2.2. closed parent issue (#10989)
2. status available, but after validation not deemable for use
2.1. statuses marked not as is_closed
2.1.1. reopening issue assigned to a version marked not as is_closed (#1245)
As said, ultimately we're solving edge-cases where certain statuses are not available for use under specific circumstances (by notifying users about it in the UI).
This can firstly be divided into two categories, namely those where the status is not made available before a validation and those where the status is made available before a validation but where it is rejected during the validation.
The latter is not so much part of this current issue (as its requirements and/thus its implementation is different). The former category is primarily what we're dealing with here and can be sub-divided into two cases, namely those where statuses are marked as is_closed and those where statuses are not marked as is_closed.
- 1.1.2. open subtasks (#10989)
- 1.1.3. open blocking issues (#1740)
- 1.2.2. closed parent issue (#10989)
Cases 1.1.2 and 1.1.3 are covered by your patch (after the merge of issue #32757). I think it would be a good idea to also cover case 1.2.2 in the same manner as the other two cases simultaneously.
Here is some (pseudo) code to give an idea what I'm thinking about:
app/models/issue.rb
attr_reader :not_closable_reason, :not_reopenable_reason
# Returns true if this issue can be closed and if not, returns false and populates the reason
def closable?
<...>
end
# Returns true if this issue can be reopened and if not, returns false and populates the reason
def reopenable?
if ancestors.open(false).any?
@not_reopenable_reason = "This issue cannot be reopened because its parent issue is closed."
return false
end
return true
end
def new_statuses_allowed_to(user=User.current, include_default=false)
<...>
unless closable?
# cannot close a blocked issue or a parent with open subtasks
statuses.reject!(&:is_closed?)
end
unless reopenable?
# cannot reopen a subtask of a closed parent
statuses.select!(&:is_closed?)
end
statuses
<...>
end
What do you think?
Some additional notes:@Marius BALTEANU: The T9N of string notice_issue_not_closable_by_blocking_issue in your patch is not specific enough. It should explicitly cover that the issue cannot be closed because it is blocked by (an)other open issue(s).
@Toshi MARUYAMA, @Mitsuyoshi Kawabata: This issue is not superseding/related to issues #28492 and #31322. The former is a request to allow closed parent issues with open subtasks via a configuration option and the latter is a specific request to provide a way to automatically close all open subtasks when a parent issue is being closed.This issue on the other hand is a specific request to indicate in the UI if and why certain statuses are not available (mainly) in cases other than that the status is not available because of workflow status transition rules.
Finally, @Toshi MARUYAMA, @Marius BALTEANU: I think this notification needs to be shown only when it is applicable (thus when an issue is not closable? [and not reopenable?, by my extension of this issue]). If it is always shown I bet something must have gone wrong with the patch.
Updated by Marius BĂLTEANU over 4 years ago
Mischa, thank you for your deep analysis on this, it's really useful.
Mischa The Evil wrote:
Marius BALTEANU wrote:
[...] I tried to use another approach in order to avoid keeping the logic in multiple places. I'm attaching the results, feedback is welcome!
I like this approach. Very neat. It also got me thinking about what we're trying to solve here though. So I took a step back to look at the bigger picture. I'll elaborate.
The two hereby patched cases are part of a subset of cases, which itself ultimately is another subset of the way we deal with "unusable" statuses. The following list reflects this structure:
[...]
As said, ultimately we're solving edge-cases where certain statuses are not available for use under specific circumstances (by notifying users about it in the UI).
So, to summarize, there are three cases, besides the 'regular' workflow status transition rules (which can be inferred by the user and don't need any notification in the UI whatsoever IMHO), where a certain status (either marked as is_closed or not) is not available:
This can firstly be divided into two categories, namely those where the status is not made available before a validation and those where the status is made available before a validation but where it is rejected during the validation.
The latter is not so much part of this current issue (as its requirements and/thus its implementation is different). The former category is primarily what we're dealing with here and can be sub-divided into two cases, namely those where statuses are marked as is_closed and those where statuses are not marked as is_closed.
- 1.1.2. open subtasks (#10989)
- 1.1.3. open blocking issues (#1740)
- 1.2.2. closed parent issue (#10989)
Cases 1.1.2 and 1.1.3 are covered by your patch (after the merge of issue #32757). I think it would be a good idea to also cover case 1.2.2 in the same manner as the other two cases simultaneously.
Here is some (pseudo) code to give an idea what I'm thinking about:
app/models/issue.rb
[...]
What do you think?
You're right, this issue should cover all three cases.
Some additional notes:
@Marius BALTEANU: The T9N of stringnotice_issue_not_closable_by_blocking_issuein your patch is not specific enough. It should explicitly cover that the issue cannot be closed because it is blocked by (an)other open issue(s).
I agree, what about the following translations? notice_issue_not_closable_by_open_tasks: "This issue cannot be closed because it has at least one open subtask." notice_issue_not_closable_by_blocking_issue: "This issue cannot be closed because it is blocked by at least one open issue." notice_issue_not_reopenable_by_closed_parent_issue:: "This issue cannot be reopened because its parent issue is closed."
I'm not an expert in English, it just sounds better to me instead of using the "(an)other" and "(s)".
Finally,
@Toshi MARUYAMA,@Marius BALTEANU: I think this notification needs to be shown only when it is applicable (thus when an issue is notclosable?[and notreopenable?, by my extension of this issue]). If it is always shown I bet something must have gone wrong with the patch.
It's already doing this and the cases are covered by tests as well. toshio harita MARUYAMA, can you provide the steps to reproduce the problem?
Updated by Marius BĂLTEANU over 4 years ago
- File 0001-Show-warning-and-the-reason-when-the-issue-cannot-be.patch 0001-Show-warning-and-the-reason-when-the-issue-cannot-be.patch added
Attached the updated patch based on my previous note, all tests pass.
Updated by Go MAEDA about 4 years ago
- Related to Defect #6158: Needs warning when trying to close a ticket that is blocked by another one added
Updated by Go MAEDA about 4 years ago
- Status changed from New to Closed
- Assignee set to Go MAEDA
- Resolution set to Fixed
Committed the patch. Thank you all for your contribution to this feature.
Updated by Mischa The Evil almost 4 years ago
- Related to Defect #33415: Issue#closable? doesn't handle the case of issues with open subtask(s) ánd being blocked by other open issue(s) added