[32785] Ensure we load first page on filter change

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

frontend/src/app/components/wp-list/wp-list.service.ts

@@ -44,7 +44,7 @@ import {NotificationsService} from 'core-app/modules/common/notifications/notifi
 import {I18nService} from "core-app/modules/common/i18n/i18n.service";
 import {from, Observable, of} from 'rxjs';
 import {input} from "reactivestates";
-import {catchError, mergeMap, share, switchMap, take, delay, combineLatest} from "rxjs/operators";
+import {catchError, mergeMap, share, switchMap, take} from "rxjs/operators";
 import {WorkPackageViewPaginationService} from "core-app/modules/work_packages/routing/wp-view-base/view-services/wp-view-pagination.service";
 import {ConfigurationService} from "core-app/modules/common/config/configuration.service";
 import {PaginationService} from "core-components/table-pagination/pagination-service";
@@ -118,7 +118,7 @@ export class WorkPackagesListService {
         // Load a default query
         const queryProps = this.UrlParamsHelper.buildV3GetQueryFromJsonParams(decodedProps);
         return from(this.handleQueryLoadingError(error, queryProps, queryParams.query_id, projectIdentifier));
-      }),
+      })
     );
   }
 
@@ -158,23 +158,21 @@ export class WorkPackagesListService {
   /**
    * Reloads the current query and set the pagination to the first page.
    */
-  public reloadQuery(query:QueryResource):Promise<QueryResource> {
-    let pagination = this.getPaginationInfo();
-    pagination.offset = 1;
-
-    let wpListPromise = this.QueryDm.reload(query, pagination);
+  public reloadQuery(query:QueryResource, projectIdentifier?:string):Observable<QueryResource> {
+    const pagination = { ...this.wpTablePagination.current, page: 1 };
+    const queryParams = this.UrlParamsHelper.encodeQueryJsonParams(query, pagination);
 
-    return this.updateStatesFromQueryOnPromise(wpListPromise)
-      .then((query:QueryResource) => {
-        this.conditionallyLoadForm(query);
-        return query;
-      })
-      .catch((error) => {
-        let projectIdentifier = query.project && query.project.id;
-
-        return this.handleQueryLoadingError(error, {}, query.id!, projectIdentifier);
+    this.queryRequests.clear();
+    this.queryRequests.putValue({
+      queryParams: { query_id: query.id || undefined, query_props: queryParams },
+      projectIdentifier: projectIdentifier
     });
 
+    return this
+      .queryLoading
+      .pipe(
+        take(1)
+      );
   }
 
   /**
@@ -235,7 +233,7 @@ export class WorkPackagesListService {
         this.NotificationsService.addSuccess(this.I18n.t('js.notice_successful_create'));
 
         // Reload the query, and then reload the menu
-        this.reloadQuery(query).then(() => {
+        this.reloadQuery(query).subscribe(() => {
           this.states.changes.queries.next(query.id!);
         });
 

frontend/src/app/modules/work_packages/routing/partitioned-query-space-page/partitioned-query-space-page.component.ts

@@ -73,7 +73,7 @@ export class PartitionedQuerySpacePageComponent extends WorkPackagesViewBase imp
 
   /** Current query title to render */
   selectedTitle?:string;
-  currentQuery:QueryResource;
+  currentQuery:QueryResource|undefined;
 
   /** Whether we're saving the query */
   querySaving:boolean;
@@ -122,6 +122,8 @@ export class PartitionedQuerySpacePageComponent extends WorkPackagesViewBase imp
       .pipe(
         this.untilDestroyed()
       ).subscribe(() => {
+      /** Ensure we reload the query from the changed props */
+      this.currentQuery = undefined;
       this.refresh(true, true);
     });
 
@@ -179,7 +181,7 @@ export class PartitionedQuerySpacePageComponent extends WorkPackagesViewBase imp
       this.updateQueryName(val);
     } else {
       this.wpListService
-        .create(this.currentQuery, val)
+        .create(this.currentQuery!, val)
         .then(() => this.querySaving = false)
         .catch(() => this.querySaving = false);
     }
@@ -187,7 +189,7 @@ export class PartitionedQuerySpacePageComponent extends WorkPackagesViewBase imp
 
   updateQueryName(val:string) {
     this.querySaving = true;
-    this.currentQuery.name = val;
+    this.currentQuery!.name = val;
     this.wpListService.save(this.currentQuery)
       .then(() => this.querySaving = false)
       .catch(() => this.querySaving = false);
@@ -220,7 +222,7 @@ export class PartitionedQuerySpacePageComponent extends WorkPackagesViewBase imp
     let promise:Promise<unknown>;
 
     if (firstPage) {
-      promise = this.wpListService.loadCurrentQueryFromParams(this.projectIdentifier);
+      promise = this.loadFirstPage();
     } else {
       promise = this.wpListService.reloadCurrentResultsList();
     }
@@ -234,6 +236,14 @@ export class PartitionedQuerySpacePageComponent extends WorkPackagesViewBase imp
     return promise;
   }
 
+  protected loadFirstPage():Promise<QueryResource> {
+    if (this.currentQuery) {
+      return this.wpListService.reloadQuery(this.currentQuery, this.projectIdentifier).toPromise();
+    } else {
+      return this.wpListService.loadCurrentQueryFromParams(this.projectIdentifier);
+    }
+  }
+
   protected additionalLoadingTime():Promise<unknown> {
     return Promise.resolve();
   }

spec/features/work_packages/table/queries/filter_pagination_spec.rb

@@ -0,0 +1,87 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2020 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-2017 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 docs/COPYRIGHT.rdoc for more details.
+#++
+
+require 'spec_helper'
+require 'features/work_packages/work_packages_page'
+
+describe 'Filter updates pagination', type: :feature, js: true do
+  let(:user) do
+    FactoryBot.create(:user,
+                      member_in_project: project,
+                      member_with_permissions: %i[view_work_packages])
+  end
+
+  let(:work_packages_page) { WorkPackagesPage.new(project) }
+  let(:wp_table) { Pages::WorkPackagesTable.new(project) }
+  let(:filters) { Components::WorkPackages::Filters.new }
+
+  let(:project) { FactoryBot.create(:project) }
+  let(:work_package_1) { FactoryBot.create(:work_package, project: project, assigned_to: user) }
+  let(:work_package_2) { FactoryBot.create(:work_package, project: project) }
+
+  before do
+    allow(Setting).to receive(:per_page_options).and_return '1'
+
+    work_package_1
+    work_package_2
+
+    login_as user
+    wp_table.visit!
+  end
+
+  it 'will reset page to 1 if changing filter' do
+    wp_table.expect_work_package_listed work_package_1
+    wp_table.ensure_work_package_not_listed! work_package_2
+
+    # Expect pagination to be correct
+    expect(page).to have_selector('.pagination--item.-current', text: '1')
+
+    # Go to second page
+    within('.pagination--pages') do
+      find('.pagination--item a', text: '2').click
+    end
+
+    wp_table.expect_work_package_listed work_package_2
+    wp_table.ensure_work_package_not_listed! work_package_1
+
+    # Expect pagination to be correct
+    expect(page).to have_selector('.pagination--item.-current', text: '2')
+
+    # Change filter to assigned to
+    filters.expect_filter_count 1
+    filters.open
+    filters.add_filter_by 'Assignee', 'is', user.name
+    filters.expect_filter_count 2
+
+    wp_table.expect_work_package_listed work_package_1
+    wp_table.ensure_work_package_not_listed! work_package_2
+
+    # Expect pagination to be back to 1
+    expect(page).to have_selector('.pagination--range', text: '1 - 1/1')
+  end
+end