Merge pull request #16957 from opf/bug/58360-statuses-edit-page-shoul…

…d-show-all-checkboxes-and-disable-those-leading-to-impossible-states

[58360] Disable "is read-only" checkbox in admin statuses when "is default" is checked

app/forms/statuses/form.rb

@@ -64,18 +64,26 @@ def initialize(submit_label: nil)
         name: :is_closed
       )
 
-      unless already_default_status?
-        statuses_form.check_box(
-          label: attribute_name(:is_default),
-          name: :is_default
-        )
-      end
+      statuses_form.check_box(
+        label: attribute_name(:is_default),
+        name: :is_default,
+        disabled: already_default_status?,
+        caption: I18n.t("statuses.edit.status_default_text"),
+        data: {
+          "admin--statuses-target": "isDefaultCheckbox",
+          action: "admin--statuses#updateReadonlyCheckboxDisabledState"
+        }
+      )
 
       statuses_form.check_box(
         label: attribute_name(:is_readonly),
         name: :is_readonly,
-        disabled: readonly_work_packages_restricted?,
-        caption: I18n.t("statuses.edit.status_readonly_html").html_safe
+        disabled: readonly_disabled?,
+        caption: I18n.t("statuses.edit.status_readonly_html").html_safe,
+        data: {
+          "admin--statuses-target": "isReadonlyCheckbox",
+          restricted: readonly_work_packages_restricted?
+        }
       )
 
       if readonly_work_packages_restricted?
@@ -112,17 +120,21 @@ def status
       model
     end
 
+    def percent_complete_field_caption
+      I18n.t("statuses.edit.status_percent_complete_text",
+             href: url_helpers.admin_settings_progress_tracking_path).html_safe
+    end
+
     def already_default_status?
       status.is_default_was == true
     end
 
-    def percent_complete_field_caption
-      I18n.t("statuses.edit.status_percent_complete_text",
-             href: url_helpers.admin_settings_progress_tracking_path).html_safe
+    def readonly_disabled?
+      readonly_work_packages_restricted? || already_default_status?
     end
 
     def readonly_work_packages_restricted?
-      !EnterpriseToken.allows_to?(:readonly_work_packages)
+      !status.can_readonly?
     end
   end
 end

app/views/statuses/edit.html.erb

@@ -41,6 +41,14 @@ See COPYRIGHT and LICENSE files for more details.
 
 <%= error_messages_for :status %>
 
-<%= primer_form_with(model: @status) do |f| %>
-  <%= render Statuses::Form.new(f) %>
-<% end %>
+<%=
+primer_form_with(
+  model: @status,
+  data: {
+    application_target: "dynamic",
+    controller: "admin--statuses"
+  }
+) do |f|
+  render Statuses::Form.new(f)
+end
+%>

app/views/statuses/new.html.erb

@@ -41,6 +41,14 @@ See COPYRIGHT and LICENSE files for more details.
 
 <%= error_messages_for :status %>
 
-<%= primer_form_with(model: @status) do |f| %>
-  <%= render Statuses::Form.new(f, submit_label: I18n.t(:button_create)) %>
-<% end %>
+<%=
+primer_form_with(
+  model: @status,
+  data: {
+    application_target: "dynamic",
+    controller: "admin--statuses"
+  }
+) do |f|
+  render Statuses::Form.new(f, submit_label: I18n.t(:button_create))
+end
+%>

config/locales/en.yml

@@ -539,6 +539,8 @@ en:
       status_color_text: |
         Click to assign or change the color of this status.
         It is shown in the status button and can be used for highlighting work packages in the table.
+      status_default_text: |-
+        New work packages are by default set to this type. They cannot be read-only.
       status_excluded_from_totals_text: |-
         Check this option to exclude work packages with this status from totals of Work,
         Remaining work, and % Complete in a hierarchy.

frontend/src/global_styles/content/_forms.sass

@@ -173,6 +173,13 @@ form
     .-columns-2
       column-count: 2
 
+  // follows new board page columns width (which uses -wide class (500px) for its tiles)
+  .boards-enterprise-banner-wide
+    max-width: calc(2*500px + 1rem)
+
+  @media screen and (max-width: $breakpoint-md)
+    .boards-enterprise-banner-wide
+      max-width: 500px
 
 hr
   width: 100%

frontend/src/stimulus/controllers/dynamic/admin/statuses.controller.ts

@@ -0,0 +1,56 @@
+/*
+ * -- copyright
+ * OpenProject is an open source project management software.
+ * Copyright (C) the OpenProject GmbH
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 3.
+ *
+ * OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+ * Copyright (C) 2006-2013 Jean-Philippe Lang
+ * Copyright (C) 2010-2013 the ChiliProject Team
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ * See COPYRIGHT and LICENSE files for more details.
+ * ++
+ */
+
+import { Controller } from '@hotwired/stimulus';
+
+export default class StatusesController extends Controller {
+  static targets = [
+    'isDefaultCheckbox',
+    'isReadonlyCheckbox',
+  ];
+
+  declare readonly isDefaultCheckboxTarget:HTMLInputElement;
+  declare readonly isReadonlyCheckboxTarget:HTMLInputElement;
+
+  updateReadonlyCheckboxDisabledState() {
+    // do nothing if the readonly checkbox is restricted due to enterprise
+    // edition not being available
+    if (this.isReadonlyCheckboxTarget.dataset.restricted === 'true') {
+      return;
+    }
+
+    if (this.isDefaultCheckboxTarget.checked) {
+      this.isReadonlyCheckboxTarget.checked = false;
+      this.isReadonlyCheckboxTarget.disabled = true;
+    } else {
+      this.isReadonlyCheckboxTarget.disabled = false;
+    }
+  }
+}

modules/boards/app/views/boards/boards/_form.html.erb

@@ -63,8 +63,8 @@ See COPYRIGHT and LICENSE files for more details.
     <label class="form--label" for="board_type"><%= t('boards.label_board_type') %></label>
     <div class="form--field-container -enterprise-restricted">
       <% unless EnterpriseToken.allows_to?(:board_view) %>
-        <p>
-          <%= angular_component_tag 'op-enterprise-banner',
+        <p class="boards-enterprise-banner-wide">
+          <%= angular_component_tag 'opce-enterprise-banner',
                                     inputs: {
                                       collapsible: false,
                                       opReferrer: 'boards',

modules/boards/spec/features/board_enterprise_spec.rb

@@ -67,12 +67,12 @@
 
       board_page = board_index.open_board(manual_board)
       board_page.expect_query "My board"
-      expect(page).not_to have_test_selector "op-enterprise-banner"
+      expect(page).not_to have_enterprise_banner
 
       board_index.visit!
       board_page = board_index.open_board(action_board)
       board_page.expect_query "Subproject board"
-      expect(page).to have_test_selector "op-enterprise-banner"
+      expect(page).to have_enterprise_banner
     end
   end
 
@@ -95,12 +95,12 @@
 
       board_page = board_index.open_board(manual_board)
       board_page.expect_query "My board"
-      expect(page).not_to have_test_selector "op-enterprise-banner"
+      expect(page).not_to have_enterprise_banner
 
       board_index.visit!
       board_page = board_index.open_board(action_board)
       board_page.expect_query "Subproject board"
-      expect(page).not_to have_test_selector "op-enterprise-banner"
+      expect(page).not_to have_enterprise_banner
     end
   end
 end

modules/boards/spec/features/boards_global_create_spec.rb

@@ -50,8 +50,8 @@
     end
 
     context "with a Community Edition", with_ee: %i[] do
-      it "renders an enterprise banner and disables all restriced board types", :aggregate_failures do
-        expect(page).to have_css("op-enterprise-banner")
+      it "renders an enterprise banner and disables all restricted board types", :aggregate_failures do
+        expect(page).to have_enterprise_banner
         expect(page).to have_selector(:radio_button, "Basic")
 
         %w[Status Assignee Version Subproject Parent-child].each do |restricted_board_type|

spec/features/admin/statuses_spec.rb

@@ -0,0 +1,137 @@
+# frozen_string_literal: true
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+require "spec_helper"
+
+RSpec.describe "Statuses admin page", :js, :with_cuprite do
+  shared_let(:admin) { create(:admin) }
+  shared_let(:status_new) { create(:status, name: "New", default_done_ratio: 0, is_default: true) }
+  shared_let(:status_in_progress) { create(:status, name: "In Progress", default_done_ratio: 40) }
+  shared_let(:status_done) { create(:status, name: "Done", default_done_ratio: 100, is_closed: true, is_readonly: true) }
+
+  before do
+    login_as(admin)
+  end
+
+  describe "create page" do
+    context "with enterprise edition", with_ee: %i[readonly_work_packages] do
+      it "has 'is read-only' checkbox unchecked and disabled only when 'is default' is checked (mutually exclusive)" do
+        visit new_status_path
+
+        expect(page).not_to have_enterprise_banner
+        expect(page).to have_unchecked_field(Status.human_attribute_name(:is_readonly))
+        expect(page).to have_unchecked_field(Status.human_attribute_name(:is_default))
+
+        # given read-only is checked, when is_default is checked then read-only should become unchecked and disabled
+        page.check(Status.human_attribute_name(:is_readonly))
+        expect(page).to have_checked_field(Status.human_attribute_name(:is_readonly))
+        page.check(Status.human_attribute_name(:is_default))
+        expect(page).to have_unchecked_field(Status.human_attribute_name(:is_readonly), disabled: true)
+
+        # given read-only is disabled, when is_default is unchecked then read-only should become enabled
+        page.uncheck(Status.human_attribute_name(:is_default))
+        expect(page).to have_unchecked_field(Status.human_attribute_name(:is_readonly), disabled: false)
+      end
+    end
+
+    context "with commmunity edition", with_ee: false do
+      it "has 'is read-only' checkbox always disabled" do
+        visit new_status_path
+
+        expect(page).to have_enterprise_banner
+        expect(page).to have_unchecked_field(Status.human_attribute_name(:is_readonly), disabled: true)
+
+        # check that it remains unchecked
+        page.check(Status.human_attribute_name(:is_default))
+        page.uncheck(Status.human_attribute_name(:is_default))
+        expect(page).to have_unchecked_field(Status.human_attribute_name(:is_readonly), disabled: true)
+      end
+    end
+  end
+
+  describe "edit page" do
+    it "has 'is default' checkbox disabled for the default status (cannot be unchecked)" do
+      visit statuses_path
+
+      click_on "New"
+      expect(page).to have_checked_field(Status.human_attribute_name(:is_default), disabled: true)
+
+      page.go_back
+      click_on "In Progress"
+      expect(page).to have_unchecked_field(Status.human_attribute_name(:is_default), disabled: false)
+    end
+
+    context "with enterprise edition", with_ee: %i[readonly_work_packages] do
+      it "has 'is read-only' checkbox unchecked and disabled only when 'is default' is checked (mutually exclusive)" do
+        visit statuses_path
+
+        click_on "New"
+        expect(page).not_to have_enterprise_banner
+        expect(page).to have_unchecked_field(Status.human_attribute_name(:is_readonly), disabled: true)
+
+        page.go_back
+        click_on "In Progress"
+        expect(page).not_to have_enterprise_banner
+        expect(page).to have_unchecked_field(Status.human_attribute_name(:is_default))
+
+        # given read-only is checked, when is_default is checked then read-only should become unchecked and disabled
+        page.check(Status.human_attribute_name(:is_readonly))
+        expect(page).to have_checked_field(Status.human_attribute_name(:is_readonly))
+        page.check(Status.human_attribute_name(:is_default))
+        expect(page).to have_unchecked_field(Status.human_attribute_name(:is_readonly), disabled: true)
+
+        # given read-only is disabled, when is_default is unchecked then read-only should become enabled
+        page.uncheck(Status.human_attribute_name(:is_default))
+        expect(page).to have_unchecked_field(Status.human_attribute_name(:is_readonly), disabled: false)
+      end
+    end
+
+    context "with commmunity edition", with_ee: false do
+      it "has 'is read-only' checkbox always disabled" do
+        visit statuses_path
+
+        click_on "In Progress"
+        expect(page).to have_enterprise_banner
+        expect(page).to have_unchecked_field(Status.human_attribute_name(:is_readonly), disabled: true)
+
+        # check that it remains unchecked
+        page.check(Status.human_attribute_name(:is_default))
+        page.uncheck(Status.human_attribute_name(:is_default))
+        expect(page).to have_unchecked_field(Status.human_attribute_name(:is_readonly), disabled: true)
+
+        # readonly statuses are no longer readonly if enterprise edition is not enabled
+        page.go_back
+        click_on "Done"
+        expect(page).to have_enterprise_banner
+        expect(page).to have_unchecked_field(Status.human_attribute_name(:is_readonly), disabled: true)
+      end
+    end
+  end
+end