Defect #5593

Grey out workflow checkboxes for transitions to the same status

Added by Yuki Kita over 8 years ago. Updated 11 days ago.

Status:ClosedStart date:2010-05-25
Priority:NormalDue date:
Assignee:Jean-Philippe Lang% Done:

0%

Category:UI
Target version:4.0.0
Resolution:Fixed Affected version:

Description

In workflow settings, it is configurable whether the current status can transit to the same status or not, though it is self-evident that the current status can transit to the same status.

edit.rhtml.diff Magnifier (781 Bytes) Yuki Kita, 2010-05-25 17:06

Screenshot.png (31.8 KB) Yuki Kita, 2010-05-25 17:06

5593-r13306.diff Magnifier - compatible with Redmine 2.5.2.devel.13306 (662 Bytes) Go MAEDA, 2014-07-13 07:38

5593-r17212.diff Magnifier - Patch for Redmine 3.4.4.devel.17212 (766 Bytes) Go MAEDA, 2018-02-25 08:04

5593-before@2x.png - screenshot (before) (35.3 KB) Go MAEDA, 2018-02-25 08:10

5593-after@2x.png - screenshot (after) (34.4 KB) Go MAEDA, 2018-02-25 08:10

tests_for_5593.patch Magnifier (1.21 KB) Marius BALTEANU, 2018-02-25 16:37

5593-note14@2x.png (35.4 KB) Go MAEDA, 2018-02-26 01:25

5593-r17235.patch Magnifier (2.01 KB) Mizuki ISHIKAWA, 2018-03-14 06:11

5593-r17235-2.patch Magnifier (3.86 KB) Mizuki ISHIKAWA, 2018-03-14 06:58

wrong-color@2x.png - Checked statuses is displayed in white under the certain conditions (53.7 KB) Go MAEDA, 2018-03-19 14:14

5593-r17236.patch Magnifier (4.46 KB) Mizuki ISHIKAWA, 2018-03-22 01:17


Related issues

Related to Redmine - Feature #5816: New issue initial status should be settable in workflow Closed 2010-07-05
Related to Redmine - Patch #16364: Issue workflow doesn't force status transition though wor... New

Associated revisions

Revision 17487
Added by Jean-Philippe Lang 11 days ago

Disable workflow checkboxes with no status change (#5593).

Patch by Mizuki ISHIKAWA.

History

#1 Updated by Norbert Bérci about 8 years ago

+1

Is it possible to apply Yuki Kita's patch? It seems extremely simple, but improves greately the UI (removes confusion about why are there same issue transitions).

#2 Updated by Thomas Pihl about 8 years ago

It does look nice. But...

Sometimes we only allow users to create new issues, not change any state. Then we would check the new/new checkbox. How would we solve that?

#3 Updated by Yuki Kita about 8 years ago

Thomas Pihl wrote:

It does look nice. But...

Sometimes we only allow users to create new issues, not change any state. Then we would check the new/new checkbox. How would we solve that?

You do not have to check the new/new checkbox to prohibit users from changing state.
Instead of doing it, uncheck all checkbox and set the default value of issue statuses to "new".
I think this solves your concern.

#4 Updated by yusuke kokubo about 8 years ago

+ 1

#5 Updated by Akiko Takano about 8 years ago

+ 1

#6 Updated by Norbert Bérci about 8 years ago

To those interested: let's discuss #5816 (about the initial statuses).

#7 Updated by Terence Mill over 6 years ago

+1
Small but easy embeddable enhancement!

#8 Updated by Go MAEDA about 4 years ago

+1

I have made Yuki Kita's patch compatible with current trunk(Redmine 2.5.2.devel.13306).

#9 Updated by Mischa The Evil about 3 years ago

  • Target version set to Candidate for next major release

#10 Updated by Go MAEDA over 1 year ago

  • Related to Patch #16364: Issue workflow doesn't force status transition though workflow is defined so added

#11 Updated by Go MAEDA 7 months ago

Updated the patch for the current trunk.

Before:
screenshot (before)

After:
screenshot (after)

#12 Updated by Bernhard Rohloff 7 months ago

+1

This patch also provides great reference points within the checkbox matrix.

#13 Updated by Marius BALTEANU 7 months ago

+1

Attached is a patch that fixes an existing failing test and adds a test for Go Maeda's patch.

#14 Updated by Marius BALTEANU 7 months ago

Reading the tickets and comments from the related issue (#16364) where some users request the "force status transition" feature, I'm wondering if is better to show a disabled checked checkbox instead of "-" in order to make more obvious for the users that the "transition" between the same status is allowed and it cannot be disabled.

#15 Updated by Go MAEDA 7 months ago

Marius BALTEANU wrote:

I'm wondering if is better to show a disabled checked checkbox instead of "-" in order to make more obvious for the users that the "transition" between the same status is allowed and it cannot be disabled.

That makes sense. Do you mean like this?

#16 Updated by Bernhard Rohloff 7 months ago

Marius' suggestion is a neat additional improvement. It coherently presents the current implementation but lets room for future changes without changing parts of the UI.
So if #16364 gets a thing someday, all we have to do is activating the checkboxes again.

#17 Updated by Marius BALTEANU 7 months ago

Go MAEDA wrote:

That makes sense. Do you mean like this?

Exactly.

#18 Updated by Mischa The Evil 7 months ago

  • Subject changed from Meanless settings for workflow to Meaningless settings for workflow
  • Target version changed from Candidate for next major release to 4.0.0

#19 Updated by Mizuki ISHIKAWA 7 months ago

I think Marius' suggestion is good as well.
I updated the patch to add disabled checkbox instead of "-".

#20 Updated by Mizuki ISHIKAWA 7 months ago

Fixed disabled checkbox not to be affected by toggleCheckboxesBySelector.

#21 Updated by Go MAEDA 6 months ago

Mizuki, thank you for posting the patch.

But I found that transitions whose source status and destination status are the same are displayed in a different color under certain conditions. Could you look into this and update the patch?

Steps to reproduce:

  1. Before applying the patch, edit the workflow for "Manager" - "Bug", check all transitions and save. Every transition is displayed in green.
  2. Uncheck the checkbox for "New" -> "New" transition and save. "New" -> "New" transition should be displayed in white.
  3. Apply the patch 5593-r17235-2.patch
  4. Check "Manager" - "Bug" workflow. Transitions like "New" -> "New" and "Assigned -> Assigned" are checked and disabled. Although checked statuses should displayed in green but "New" -> "New" status (unchecked in step 2) is displayed in white.

Checked statuses is displayed in white under the certain conditions

#22 Updated by Mizuki ISHIKAWA 6 months ago

I attached a patch that corrected the problem pointed out to Go MAEDA.
I mainly added the following changes.

diff --git a/app/views/workflows/_form.html.erb b/app/views/workflows/_form.html.erb
index 3fc69c4f5..542db4e35 100644
--- a/app/views/workflows/_form.html.erb
+++ b/app/views/workflows/_form.html.erb
@@ -38,7 +38,7 @@
       <% end %>
     </td>
     <% for new_status in @statuses -%>
-    <% checked = workflows.detect {|w| w.old_status == old_status && w.new_status == new_status} %>
+    <% checked = (old_status == new_status) || workflows.detect {|w| w.old_status == old_status && w.new_status == new_status} %>
     <td class="<%= checked ? 'enabled' : '' %>" title="<%= old_status_name %> &#187; <%= new_status.name %>">
       <%= transition_tag workflows, old_status, new_status, name %>
     </td>

But there is one problem with this patch.

The state of workflow is saved as WorkflowTransition. (Even when the source status and the destination status are the same)
When a patch is applied, the checkbox displayed will be checked, even if the data indicates that it is not checked the checkbox.
That is, the data and the displayed results are contradictory.

I think it is difficult to display a disabled checkbox while maintaining consistency with the data.

#23 Updated by Marius BALTEANU 6 months ago

Mizuki ISHIKAWA wrote:

When a patch is applied, the checkbox displayed will be checked, even if the data indicates that it is not checked the checkbox.
That is, the data and the displayed results are contradictory.

You're worried that the displayed results are not according with the data from the database? For ex: In the UI, the checkbox for New -> New is checked and disabled, but in the db there is no entry in the WorkflowTransition for the transition (I'm asking just to confirm that I understand correctly).

If yes, I don't see a real problem from the following reasons:
- is not a real inconsistency because you don't really need a transition to update a ticket without changing the status.
- only a few users can observe this inconsistency and they need to be quite technical to deep in Redmine code and database.

#24 Updated by Mizuki ISHIKAWA 6 months ago

Marius BALTEANU wrote:

Mizuki ISHIKAWA wrote:

When a patch is applied, the checkbox displayed will be checked, even if the data indicates that it is not checked the checkbox.
That is, the data and the displayed results are contradictory.

You're worried that the displayed results are not according with the data from the database? For ex: In the UI, the checkbox for New -> New is checked and disabled, but in the db there is no entry in the WorkflowTransition for the transition (I'm asking just to confirm that I understand correctly).

Yes, I was worried about it.

If yes, I don't see a real problem from the following reasons:
- is not a real inconsistency because you don't really need a transition to update a ticket without changing the status.
- only a few users can observe this inconsistency and they need to be quite technical to deep in Redmine code and database.

Certainly, I think that few people feel strange about this problem.
I seemed to be over-thinking about it problem.
Thank you for checking, Marius BALTEANU.

#25 Updated by Jean-Philippe Lang 11 days ago

  • Subject changed from Meaningless settings for workflow to Grey out workflow checkboxes for transitions to the same status
  • Status changed from New to Closed
  • Assignee set to Jean-Philippe Lang
  • Resolution set to Fixed

Committed, thanks.

Also available in: Atom PDF