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

--------------------------------------------------------------------------------

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

--------------------------------------------------------------------------------
+1, this issue is simple and clear.
If we have #31589 we can skip #28492 and #31322.
--------------------------------------------------------------------------------
Yes, this is very very simple.
This is POC patch, so there is no test code.
--------------------------------------------------------------------------------
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

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
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_*".
--------------------------------------------------------------------------------

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

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

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

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

<pre>
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)
</pre>

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_

<pre><code class="ruby">
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
</code></pre>

What do you think?

Some additional notes:
<code>@</code>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).

<code>@</code>Toshi MARUYAMA, <code>@</code>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, <code>@</code>Toshi MARUYAMA, <code>@</code>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.

--------------------------------------------------------------------------------
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:
> <code>@</code>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, <code>@</code>Toshi MARUYAMA, <code>@</code>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?

--------------------------------------------------------------------------------
Attached the updated patch based on my previous note, all "tests":https://gitlab.com/redmine-org/redmine/pipelines/108734385 pass.
--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
Committed the patch. Thank you all for your contribution to this feature.
--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

relates,Closed,10989,Prevent parent issue from being closed if a child issue is open
relates,New,28492,Option not to block closing a parent issue when it has open subtask(s)
relates,New,31322,Provide a way to automatically close all open subtasks too when a parent issue is being closed
relates,Closed,6158,Needs warning when trying to close a ticket that is blocked by another one
relates,New,33415,Issue#closable? doesn't handle the case of issues with open subtask(s) ánd being blocked by other open issue(s)
duplicates,Closed,32757,Show warning when an issue cannot be closed because of blocking issue relations