Feature #31589

Show warning and the reason when the issue cannot be closed because of open subtasks or blocking open issue(s)

Added by Go MAEDA 12 months ago. Updated 3 months ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Go MAEDA% Done:

0%

Category:UI
Target version:4.2.0
Resolution:Fixed

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.

warning-unable-to-close-parent@2x.png (19.5 KB) Go MAEDA, 2019-06-19 05:22

issue-31589-poc.diff Magnifier (2.07 KB) Toshi MARUYAMA, 2020-01-06 16:22

0001-Show-an-warning-with-the-reason-for-why-an-issue-can.patch Magnifier (6.49 KB) Marius BALTEANU, 2020-01-08 08:35

0001-Show-warning-and-the-reason-when-the-issue-cannot-be.patch Magnifier (7.42 KB) Marius BALTEANU, 2020-01-12 20:24


Related issues

Related to Redmine - Feature #10989: Prevent parent issue from being closed if a child issue i... Closed
Related to Redmine - Feature #28492: Option not to block closing a parent issue when it has op... New
Related to Redmine - Feature #31322: Provide a way to automatically close all open subtasks to... New
Related to Redmine - Defect #6158: Needs warning when trying to close a ticket that is block... Closed 2010-08-17
Related to Redmine - Defect #33415: Issue#closable? doesn't handle the case of issues with op... New
Duplicated by Redmine - Feature #32757: Show warning when an issue cannot be closed because of bl... Closed

Associated revisions

Revision 19570
Added by Go MAEDA 3 months ago

Show warning and the reason when the issue cannot be closed or reopen because of open subtask(s), blocking issue(s) or closed parent issue (#31589).

Patch by Marius BALTEANU.

Revision 19571
Added by Go MAEDA 3 months ago

Update locales (#31589).

History

#1 Updated by Go MAEDA 12 months ago

  • Related to Feature #10989: Prevent parent issue from being closed if a child issue is open added

#2 Updated by Marius BALTEANU 12 months 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.

#3 Updated by Go MAEDA 12 months ago

  • Assignee set to Marius BALTEANU

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!

#4 Updated by David L 7 months 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.

#5 Updated by Toshi MARUYAMA 5 months ago

  • Related to Feature #28492: Option not to block closing a parent issue when it has open subtask(s) added

#6 Updated by Toshi MARUYAMA 5 months ago

  • Related to Feature #31322: Provide a way to automatically close all open subtasks too when a parent issue is being closed added

#7 Updated by Toshi MARUYAMA 5 months ago

I think if #31322 is implemented, this issue is not needed.

#8 Updated by Go MAEDA 5 months ago

  • Related to Feature #32757: Show warning when an issue cannot be closed because of blocking issue relations added

#9 Updated by Mitsuyoshi Kawabata 5 months ago

+1, this issue is simple and clear.
If we have #31589 we can skip #28492 and #31322.

#10 Updated by Toshi MARUYAMA 5 months ago

Yes, this is very very simple.
This is POC patch, so there is no test code.

#11 Updated by Marius BALTEANU 5 months 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

#12 Updated by Marius BALTEANU 5 months ago

  • Assignee deleted (Marius BALTEANU)
  • Target version set to Candidate for next major release

#13 Updated by Go MAEDA 5 months 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_*".

#14 Updated by Marius BALTEANU 5 months ago

  • File deleted (0001-Show-an-warning-with-the-reason-for-why-an-issue-can.patch)

#15 Updated by Marius BALTEANU 5 months 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.

#16 Updated by Marius BALTEANU 5 months ago

  • File deleted (0001-Show-an-warning-with-the-reason-for-why-an-issue-can.patch)

#17 Updated by Marius BALTEANU 5 months ago

#18 Updated by Marius BALTEANU 5 months ago

  • Related to deleted (Feature #32757: Show warning when an issue cannot be closed because of blocking issue relations)

#19 Updated by Marius BALTEANU 5 months ago

  • Duplicated by Feature #32757: Show warning when an issue cannot be closed because of blocking issue relations added

#20 Updated by Toshi MARUYAMA 5 months 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

#21 Updated by Mischa The Evil 5 months 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.

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:
  • 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.

#22 Updated by Marius BALTEANU 5 months 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).
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.

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:
  • 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 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).

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 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.

It's already doing this and the cases are covered by tests as well. @Toshi MARUYAMA, can you provide the steps to reproduce the problem?

#23 Updated by Marius BALTEANU 5 months ago

Attached the updated patch based on my previous note, all tests pass.

#24 Updated by Go MAEDA 3 months ago

  • Related to Defect #6158: Needs warning when trying to close a ticket that is blocked by another one added

#25 Updated by Go MAEDA 3 months 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.

#26 Updated by Mischa The Evil 26 days 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

Also available in: Atom PDF