Merge pull request #18919 from colemanw/searchKitValidate

Search ext: Fix validation and saving on search admin screen
civicrm · Nov 4, 2020 · fbb3855 · fbb3855
2 parents cd7078a + 2badf24
commit fbb3855
Show file tree

Hide file tree

Showing 8 changed files with 134 additions and 63 deletions.
diff --git a/ext/search/ang/crmSearchAdmin.module.js b/ext/search/ang/crmSearchAdmin.module.js
@@ -48,7 +48,7 @@
             return crmApi4('SavedSearch', 'get', {
               where: [['id', '=', params.id]],
               chain: {
-                groups: ['Group', 'get', {where: [['saved_search_id', '=', '$id']]}],
+                groups: ['Group', 'get', {select: ['id', 'title', 'description', 'visibility', 'group_type'], where: [['saved_search_id', '=', '$id']]}],
                 displays: ['SearchDisplay', 'get', {where: [['saved_search_id', '=', '$id']]}]
               }
             }, 0);

diff --git a/ext/search/ang/crmSearchAdmin/crmSearchAdmin.component.js b/ext/search/ang/crmSearchAdmin/crmSearchAdmin.component.js
@@ -69,8 +69,10 @@
 
         $scope.$watch('$ctrl.savedSearch', onChangeAnything, true);
 
-        // Is this savedSearch record saved, unsaved or saving
-        $scope.status = this.savedSearch && this.savedSearch.id ? 'saved' : 'unsaved';
+        // After watcher runs for the first time and messes up the status, set it correctly
+        $timeout(function() {
+          $scope.status = ctrl.savedSearch && ctrl.savedSearch.id ? 'saved' : 'unsaved';
+        });
 
         loadFieldOptions();
       };
@@ -80,6 +82,9 @@
       }
 
       this.save = function() {
+        if (!validate()) {
+          return;
+        }
         $scope.status = 'saving';
         var params = _.cloneDeep(ctrl.savedSearch),
           apiCalls = {},
@@ -98,12 +103,17 @@
         delete params.displays;
         apiCalls.saved = ['SavedSearch', 'save', {records: [params], chain: chain}, 0];
         crmApi4(apiCalls).then(function(results) {
+          // Set new status to saved unless the user changed something in the interim
+          var newStatus = $scope.status === 'unsaved' ? 'unsaved' : 'saved';
           ctrl.savedSearch.id = results.saved.id;
-          ctrl.savedSearch.groups = results.saved.groups || [];
-          ctrl.savedSearch.displays = results.saved.displays || [];
-          if ($scope.status === 'saving') {
-            $scope.status = 'saved';
+          if (results.saved.groups && results.saved.groups.length) {
+            ctrl.savedSearch.groups[0].id = results.saved.groups[0].id;
           }
+          ctrl.savedSearch.displays = results.saved.displays || [];
+          // Wait until after onChangeAnything to update status
+          $timeout(function() {
+            $scope.status = newStatus;
+          });
         });
       };
 
@@ -123,6 +133,11 @@
         var display = ctrl.savedSearch.displays[index];
         if (display.id) {
           display.trashed = !display.trashed;
+          if ($scope.controls.tab === ('display_' + index) && display.trashed) {
+            $scope.selectTab('compose');
+          } else if (!display.trashed) {
+            $scope.selectTab('display_' + index);
+          }
         } else {
           $scope.selectTab('compose');
           ctrl.savedSearch.displays.splice(index, 1);
@@ -157,7 +172,9 @@
         if (!ctrl.groupExists && (!ctrl.savedSearch.groups.length || !ctrl.savedSearch.groups[0].id)) {
           ctrl.savedSearch.groups.length = 0;
         }
-        $scope.selectTab('compose');
+        if ($scope.controls.tab === 'group') {
+          $scope.selectTab('compose');
+        }
       };
 
       $scope.getJoinEntities = function() {
@@ -213,6 +230,39 @@
         }
       };
 
+      function validate() {
+        var errors = [],
+          errorEl,
+          label,
+          tab;
+        if (!ctrl.savedSearch.label) {
+          errorEl = '#crm-saved-search-label';
+          label = ts('Search Label');
+          errors.push(ts('%1 is a required field.', {1: label}));
+        }
+        if (ctrl.groupExists && !ctrl.savedSearch.groups[0].title) {
+          errorEl = '#crm-search-admin-group-title';
+          label = ts('Group Title');
+          errors.push(ts('%1 is a required field.', {1: label}));
+          tab = 'group';
+        }
+        _.each(ctrl.savedSearch.displays, function(display, index) {
+          if (!display.trashed && !display.label) {
+            errorEl = '#crm-search-admin-display-label';
+            label = ts('Display Label');
+            errors.push(ts('%1 is a required field.', {1: label}));
+            tab = 'display_' + index;
+          }
+        });
+        if (errors.length) {
+          if (tab) {
+            $scope.selectTab(tab);
+          }
+          $(errorEl).crmError(errors.join('<br>'), ts('Error Saving'), {expires: 5000});
+        }
+        return !errors.length;
+      }
+
       /**
        * Called when clicking on a column header
        * @param col

diff --git a/ext/search/ang/crmSearchAdmin/crmSearchAdmin.html b/ext/search/ang/crmSearchAdmin/crmSearchAdmin.html
@@ -12,18 +12,21 @@ <h1 crm-page-title>{{ $ctrl.entityTitle + ': ' + $ctrl.savedSearch.label }}</h1>
   </div>
 
   <form>
-    <div class="navbar-form clearfix">
-      <label for="crm-saved-search-label">{{:: ts('Label:') }}</label>
-      <input id="crm-saved-search-label" class="form-control" ng-model="$ctrl.savedSearch.label" type="text" />
-      <label for="crm-search-main-entity">{{:: ts('Search for:') }}</label>
-      <input id="crm-search-main-entity" class="form-control" ng-model="$ctrl.savedSearch.api_entity" crm-ui-select="::{allowClear: false, data: entities}" ng-disabled="$ctrl.savedSearch.id" />
-      <div class="btn-group btn-group-md pull-right">
-        <button type="submit" class="btn" ng-class="{'btn-primary': status === 'unsaved', 'btn-warning': status === 'saving', 'btn-success': status === 'saved'}" ng-disabled="status !== 'unsaved'" ng-click="$ctrl.save()">
-          <i class="crm-i" ng-class="{'fa-check': status !== 'saving', 'fa-spin fa-spinner': status === 'saving'}"></i>
-          <span ng-if="status === 'saved'">{{ ts('Saved') }}</span>
-          <span ng-if="status === 'unsaved'">{{ ts('Save') }}</span>
-          <span ng-if="status === 'saving'">{{ ts('Saving...') }}</span>
-        </button>
+    <div class="crm-flex-box">
+      <div class="nav-stacked">
+        <input id="crm-saved-search-label" class="form-control" ng-model="$ctrl.savedSearch.label" type="text" required placeholder="{{ ts('Untitled Search') }}" />
+      </div>
+      <div class="crm-flex-4 form-inline">
+        <label for="crm-search-main-entity">{{:: ts('Search for:') }}</label>
+        <input id="crm-search-main-entity" class="form-control" ng-model="$ctrl.savedSearch.api_entity" crm-ui-select="::{allowClear: false, data: entities}" ng-disabled="$ctrl.savedSearch.id" />
+        <div class="btn-group btn-group-md pull-right">
+          <button type="submit" class="btn" ng-class="{'btn-primary': status === 'unsaved', 'btn-warning': status === 'saving', 'btn-success': status === 'saved'}" ng-disabled="status !== 'unsaved'" ng-click="$ctrl.save()">
+            <i class="crm-i" ng-class="{'fa-check': status !== 'saving', 'fa-spin fa-spinner': status === 'saving'}"></i>
+            <span ng-if="status === 'saved'">{{ ts('Saved') }}</span>
+            <span ng-if="status === 'unsaved'">{{ ts('Save') }}</span>
+            <span ng-if="status === 'saving'">{{ ts('Saving...') }}</span>
+          </button>
+        </div>
       </div>
     </div>
     <div class="crm-flex-box">

diff --git a/ext/search/ang/crmSearchAdmin/crmSearchAdminDisplay.html b/ext/search/ang/crmSearchAdmin/crmSearchAdminDisplay.html
@@ -1,7 +1,7 @@
 <fieldset>
   <div class="form-inline">
-    <label for="search_display_label">{{:: ts('Name:') }} <span class="crm-marker">*</span></label>
-    <input id="search_display_label" type="text" class="form-control" ng-model="$ctrl.display.label" required />
+    <label for="crm-search-admin-display-label">{{:: ts('Name:') }} <span class="crm-marker">*</span></label>
+    <input id="crm-search-admin-display-label" type="text" class="form-control" ng-model="$ctrl.display.label" required placeholder="{{ ts('Untitled') }}"/>
     <label class="pull-right">{{:: $ctrl.displayTypes[$ctrl.display.type].label }}</label>
   </div>
 </fieldset>
diff --git a/ext/search/ang/crmSearchAdmin/group.html b/ext/search/ang/crmSearchAdmin/group.html
@@ -1,33 +1,27 @@
-<div ng-if="!$ctrl.groupExists">
-  <div class="alert alert-warning">
-    {{:: ts('Smart group "%1" will be deleted.', {1: $ctrl.savedSearch.groups[0].title}) }}
-  </div>
+<div class="alert alert-warning" ng-show="!smartGroupColumns.length">
+  {{:: ts('Unable to create smart group because this search does not include any contacts.') }}
+</div>
+
+<div class="form-inline">
+  <label for="crm-search-admin-group-title">{{ ts('Group Title:') }} <span class="crm-marker">*</span></label>
+  <input id="crm-search-admin-group-title" class="form-control" placeholder="{{:: ts('Untitled') }}" ng-model="$ctrl.savedSearch.groups[0].title" ng-disabled="!smartGroupColumns.length" ng-required="smartGroupColumns.length">
+  <label for="api-save-search-select-column">{{:: ts('Contact Column:') }}</label>
+  <input id="api-save-search-select-column" ng-model="$ctrl.savedSearch.api_params.select[0]" class="form-control" crm-ui-select="{data: smartGroupColumns}"/>
 </div>
-<div ng-if="$ctrl.groupExists">
-  <div class="alert alert-warning" ng-show="!smartGroupColumns.length">
-    {{:: ts('Unable to create smart group because this search does not include any contacts.') }}
+<fieldset ng-show="smartGroupColumns.length">
+  <label>{{:: ts('Description:') }}</label>
+  <textarea class="form-control" ng-model="$ctrl.savedSearch.groups[0].description"></textarea>
+  <div class="form-inline">
+    <label>{{:: ts('Group Type:') }} </label>
+    <div class="checkbox" ng-repeat="option in groupOptions.group_type track by option.id">&nbsp;
+      <label>
+        <input type="checkbox" checklist-model="$ctrl.savedSearch.groups[0].group_type" checklist-value="option.id">
+        {{ option.label }}
+      </label>&nbsp;
+    </div>
   </div>
-  <input class="form-control" placeholder="{{:: ts('Group Title') }}" ng-model="$ctrl.savedSearch.groups[0].title" ng-disabled="!smartGroupColumns.length">
-  <hr />
   <div class="form-inline">
-   <label for="api-save-search-select-column">{{:: ts('Contact Column:') }} <span class="crm-marker">*</span></label>
-    <input id="api-save-search-select-column" ng-model="$ctrl.savedSearch.api_params.select[0]" class="form-control" crm-ui-select="{data: smartGroupColumns}"/>
+    <label>{{:: ts('Visibility:') }}</label>
+    <select class="form-control" ng-model="$ctrl.savedSearch.groups[0].visibility" ng-options="item.id as item.label for item in groupOptions.visibility track by item.id" crm-ui-select></select>
   </div>
-  <fieldset ng-show="smartGroupColumns.length">
-    <label>{{:: ts('Description:') }}</label>
-    <textarea class="form-control" ng-model="$ctrl.savedSearch.groups[0].description"></textarea>
-    <div class="form-inline">
-      <label>{{:: ts('Group Type:') }} </label>
-      <div class="checkbox" ng-repeat="option in groupOptions.group_type track by option.id">&nbsp;
-        <label>
-          <input type="checkbox" checklist-model="$ctrl.savedSearch.groups[0].group_type" checklist-value="option.id">
-          {{ option.label }}
-        </label>&nbsp;
-      </div>
-    </div>
-    <div class="form-inline">
-      <label>{{:: ts('Visibility:') }}</label>
-      <select class="form-control" ng-model="$ctrl.savedSearch.groups[0].visibility" ng-options="item.id as item.label for item in groupOptions.visibility track by item.id" crm-ui-select></select>
-    </div>
-  </fieldset>
-</div>
+</fieldset>
diff --git a/ext/search/ang/crmSearchAdmin/tabs.html b/ext/search/ang/crmSearchAdmin/tabs.html
@@ -4,22 +4,22 @@
     {{ ts('Compose Search') }}
   </a>
 </li>
-<li role="presentation" ng-class="{active: controls.tab === 'group'}" ng-if="$ctrl.savedSearch.groups.length">
-  <a href ng-click="selectTab('group')" ng-class="{strikethrough: !$ctrl.groupExists}">
+<li role="presentation" ng-class="{active: controls.tab === 'group'}" ng-if="$ctrl.savedSearch.groups.length" title="{{ !$ctrl.groupExists ? ts('Group will be deleted.') : '' }}">
+  <a href ng-click="selectTab('group')" ng-disabled="!$ctrl.groupExists">
     <i class="crm-i fa-users"></i>
     {{:: ts('Smart Group:') }} {{ $ctrl.savedSearch.groups[0].title }}
   </a>
-  <button class="btn-xs btn-danger-outline crm-search-delete-display" ng-click="$ctrl.removeGroup()">
-    <i class="crm-i fa-trash"></i>
+  <button class="btn-xs btn-danger-outline crm-search-delete-display" ng-click="$ctrl.removeGroup()" title="{{ $ctrl.groupExists ? ts('Delete') : ts('Undelete') }}">
+    <i class="crm-i fa-{{ $ctrl.groupExists ? 'trash' : 'undo' }}"></i>
   </button>
 </li>
-<li role="presentation" ng-repeat="display in $ctrl.savedSearch.displays" ng-class="{active: controls.tab === ('display_' + $index)}">
-  <a href ng-click="selectTab('display_' + $index)" ng-class="{strikethrough: display.trashed}">
+<li role="presentation" ng-repeat="display in $ctrl.savedSearch.displays" ng-class="{active: controls.tab === ('display_' + $index)}" title="{{ display.trashed ? ts('Display will be deleted.') : '' }}">
+  <a href ng-click="selectTab('display_' + $index)" ng-disabled="display.trashed">
     <i class="crm-i {{ $ctrl.displayTypes[display.type].icon }}"></i>
     {{ display.label || ts('Untitled') }}
   </a>
-  <button class="btn-xs btn-danger-outline crm-search-delete-display" ng-click="$ctrl.removeDisplay($index)">
-    <i class="crm-i fa-trash"></i>
+  <button class="btn-xs btn-danger-outline crm-search-delete-display" ng-click="$ctrl.removeDisplay($index)" title="{{ display.trashed ? ts('Undelete') : ts('Delete') }}">
+    <i class="crm-i fa-{{ display.trashed ? 'undo' : 'trash' }}"></i>
   </button>
 </li>
 <li role="presentation">

diff --git a/ext/search/css/search.css b/ext/search/css/search.css
@@ -24,11 +24,33 @@
   min-height: 200px;
 }
 
-#bootstrap-theme.crm-search ul.nav.nav-stacked {
+#bootstrap-theme.crm-search .nav-stacked {
   margin-left: 0;
   margin-right: 20px;
 }
 
+#bootstrap-theme.crm-search ul.nav-stacked {
+  margin-top: 20px;
+}
+
+#bootstrap-theme.crm-search input.ng-invalid {
+  border-color: #8A1F11;
+}
+#bootstrap-theme.crm-search input.ng-invalid::placeholder {
+  color: #8A1F11;
+}
+
+#bootstrap-theme.crm-search ul.nav-stacked li {
+  cursor: default;
+}
+
+#bootstrap-theme.crm-search ul.nav-stacked li a[disabled] {
+  text-decoration: line-through !important;
+  color: grey;
+  cursor: default;
+  pointer-events: none;
+}
+
 #bootstrap-theme.crm-search fieldset {
   padding: 6px;
   border-top: 1px solid lightgrey;

diff --git a/js/Common.js b/js/Common.js
@@ -1305,10 +1305,10 @@ if (!CRM.vars) CRM.vars = {};
 
     var extra = {
       expires: 0
-    };
+    }, label;
     if ($(this).length) {
       if (title === '') {
-        var label = $('label[for="' + $(this).attr('name') + '"], label[for="' + $(this).attr('id') + '"]').not('[generated=true]');
+        label = $('label[for="' + $(this).attr('name') + '"], label[for="' + $(this).attr('id') + '"]').not('[generated=true]');
         if (label.length) {
           label.addClass('crm-error');
           var $label = label.clone();
@@ -1328,7 +1328,9 @@ if (!CRM.vars) CRM.vars = {};
         ele.one('change', function () {
           if (msg && msg.close) msg.close();
           ele.removeClass('crm-error');
-          label.removeClass('crm-error');
+          if (label) {
+            label.removeClass('crm-error');
+          }
         });
       }, 1000);
     }