[26717] Fix grouping by numeric value

Values in groups returned by the API are always string, so we need to
ensure objects are compared non-strict.

https://community.openproject.com/wp/26717

frontend/app/components/wp-edit/wp-notification.service.ts

@@ -59,12 +59,12 @@ export class WorkPackageNotificationService {
       return this.handleErrorResponse(resource, workPackage);
     }
 
-    this.showGeneralError();
+    this.showGeneralError(response);
   }
 
   public handleErrorResponse(errorResource:any, workPackage?:WorkPackageResourceInterface) {
     if (!(errorResource instanceof ErrorResource)) {
-      return this.showGeneralError();
+      return this.showGeneralError(errorResource);
     }
 
     if (workPackage) {
@@ -78,8 +78,14 @@ export class WorkPackageNotificationService {
     this.showCustomError(errorResource, workPackage) || this.showApiErrorMessages(errorResource);
   }
 
-  public showGeneralError() {
-    this.NotificationsService.addError(this.I18n.t('js.error.internal'));
+  public showGeneralError(message?:string) {
+    let error = this.I18n.t('js.error.internal');
+
+    if (message) {
+      error += ' ' + message;
+    }
+
+    this.NotificationsService.addError(error);
   }
 
   public showEditingBlockedError(attribute:string) {

frontend/app/components/wp-fast-table/builders/modes/grouped/grouped-render-pass.ts

@@ -64,7 +64,11 @@ export class GroupedRenderPass extends PlainRenderPass {
 
       // Otherwise, fall back to simple value comparison.
       let value = group.value === '' ? null : group.value;
-      return value === property;
+
+      // Values provided by the API are always string
+      // so avoid triple equal here
+      // tslint:disable-next-line
+      return value == property;
     }) as GroupObject;
   }
 

frontend/app/components/wp-fast-table/state/wp-table-additional-elements.service.ts

@@ -45,12 +45,14 @@ import {
   WorkPackageRelationsService
 } from '../../wp-relations/wp-relations.service';
 import {WorkPackageTableHierarchiesService} from './wp-table-hierarchy.service';
+import { WorkPackageNotificationService } from 'core-components/wp-edit/wp-notification.service';
 
 export class WorkPackageTableAdditionalElementsService {
 
   constructor(public states:States,
               public wpTableHierarchies:WorkPackageTableHierarchiesService,
               public wpTableColumns:WorkPackageTableColumnsService,
+              public wpNotificationsService:WorkPackageNotificationService,
               public $q:IQService,
               public halRequest:HalRequestService,
               public wpCacheService:WorkPackageCacheService,
@@ -71,6 +73,10 @@ export class WorkPackageTableAdditionalElementsService {
     this.wpCacheService.requireAll(wpIds)
       .then(() => {
         this.states.table.additionalRequiredWorkPackages.putValue(null, 'All required work packages are loaded');
+      })
+      .catch((e) => {
+        this.states.table.additionalRequiredWorkPackages.putValue(null, 'Failure loading required work packages');
+        this.wpNotificationsService.handleErrorResponse(e);
       });
   }
 

spec/features/work_packages/table/group_by/group_by_progress_spec.rb

@@ -0,0 +1,53 @@
+require 'spec_helper'
+
+describe 'Work Package group by progress', js: true do
+  let(:user) { FactoryGirl.create :admin }
+
+  let(:project) { FactoryGirl.create(:project) }
+
+  let!(:wp_1) { FactoryGirl.create(:work_package, project: project) }
+  let!(:wp_2) { FactoryGirl.create(:work_package, project: project, done_ratio: 10) }
+  let!(:wp_3) { FactoryGirl.create(:work_package, project: project, done_ratio: 10) }
+  let!(:wp_4) { FactoryGirl.create(:work_package, project: project, done_ratio: 50) }
+
+  let(:wp_table) { Pages::WorkPackagesTable.new(project) }
+
+  let!(:query) do
+    query              = FactoryGirl.build(:query, user: user, project: project)
+    query.column_names = ['subject', 'done_ratio']
+
+    query.save!
+    query
+  end
+
+  before do
+    login_as(user)
+
+    wp_table.visit_query(query)
+    wp_table.expect_work_package_listed wp_1, wp_2, wp_3, wp_4
+  end
+
+  it 'shows group headers for group by progress (regression test #26717)' do
+    # Group by category
+    wp_table.click_setting_item 'Group by ...'
+    select 'Progress (%)', from: 'selected_columns_new'
+    click_button 'Apply'
+
+    # Expect table to be grouped as WP created above
+    expect(page).to have_selector('.group--value .count', count: 3)
+    expect(page).to have_selector('.group--value', text: '0 (1)')
+    expect(page).to have_selector('.group--value', text: '10 (2)')
+    expect(page).to have_selector('.group--value', text: '50 (1)')
+
+    # Update category of wp_none
+    cat = wp_table.edit_field(wp_1, :percentageDone)
+    cat.update '50'
+
+    loading_indicator_saveguard
+
+    # Expect changed groups
+    expect(page).to have_selector('.group--value .count', count: 2)
+    expect(page).to have_selector('.group--value', text: '10 (2)')
+    expect(page).to have_selector('.group--value', text: '50 (2)')
+  end
+end