Merge pull request #15258 from opf/bug/53344-cannot-modify-a-query-th…

…at-was-created-by-a-deleted-user

Bug/53344 cannot modify a query that was created by a deleted user

frontend/src/app/features/hal/resources/query-resource.ts

@@ -33,6 +33,7 @@ import { QueryOrder } from 'core-app/core/apiv3/endpoints/queries/apiv3-query-or
 import { WorkPackageCollectionResource } from 'core-app/features/hal/resources/wp-collection-resource';
 import { QueryFilterInstanceResource } from 'core-app/features/hal/resources/query-filter-instance-resource';
 import { ProjectResource } from 'core-app/features/hal/resources/project-resource';
+import { UserResource } from 'core-app/features/hal/resources/user-resource';
 import { QuerySortByResource } from 'core-app/features/hal/resources/query-sort-by-resource';
 import { QueryGroupByResource } from 'core-app/features/hal/resources/query-group-by-resource';
 
@@ -41,6 +42,7 @@ export interface QueryResourceEmbedded {
   columns:QueryColumn[];
   groupBy:QueryGroupByResource|undefined;
   project:ProjectResource;
+  user:UserResource;
   sortBy:QuerySortByResource[];
   filters:QueryFilterInstanceResource[];
 }
@@ -96,6 +98,8 @@ export class QueryResource extends HalResource {
 
   public hidden:boolean;
 
+  public user:UserResource;
+
   public project:ProjectResource;
 
   public includeSubprojects:boolean;

frontend/src/app/features/work-packages/components/wp-list/wp-list.service.ts

@@ -31,7 +31,8 @@ import { States } from 'core-app/core/states/states.service';
 import { AuthorisationService } from 'core-app/core/model-auth/model-auth.service';
 import { StateService } from '@uirouter/core';
 import { IsolatedQuerySpace } from 'core-app/features/work-packages/directives/query-space/isolated-query-space';
-import { Injectable } from '@angular/core';
+import { Injectable, Injector } from '@angular/core';
+import { InjectField } from 'core-app/shared/helpers/angular/inject-field.decorator';
 import isPersistedResource from 'core-app/features/hal/helpers/is-persisted-resource';
 import { UrlParamsHelperService } from 'core-app/features/work-packages/components/wp-query/url-params-helper';
 import { ToastService } from 'core-app/shared/components/toaster/toast.service';
@@ -43,6 +44,7 @@ import {
   WorkPackageViewPaginationService,
 } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-pagination.service';
 import { ConfigurationService } from 'core-app/core/config/configuration.service';
+import { CurrentUserService } from 'core-app/core/current-user/current-user.service';
 import { ApiV3Service } from 'core-app/core/apiv3/api-v3.service';
 import { ApiV3QueriesPaths } from 'core-app/core/apiv3/endpoints/queries/apiv3-queries-paths';
 import { ApiV3QueryPaths } from 'core-app/core/apiv3/endpoints/queries/apiv3-query-paths';
@@ -61,6 +63,8 @@ export interface QueryDefinition {
 
 @Injectable()
 export class WorkPackagesListService {
+  @InjectField() protected readonly currentUser:CurrentUserService;
+
   // We remember the query requests coming in so we can ensure only the latest request is being tended to
   private queryRequests = input<QueryDefinition>();
 
@@ -73,7 +77,7 @@ export class WorkPackagesListService {
       // Map the observable from the stream to a new one that completes when states are loaded
       mergeMap((query:QueryResource) => {
         // load the form if needed
-        this.conditionallyLoadForm(query);
+        void this.conditionallyLoadForm(query);
 
         // Project the loaded query into the table states and confirm the query is fully loaded
         this.wpStatesInitialization.initialize(query, query.results);
@@ -85,6 +89,7 @@ export class WorkPackagesListService {
     );
 
   constructor(
+    readonly injector:Injector,
     protected toastService:ToastService,
     readonly I18n:I18nService,
     protected UrlParamsHelper:UrlParamsHelperService,
@@ -282,21 +287,7 @@ export class WorkPackagesListService {
       .then(() => {
         this.toastService.addSuccess(this.I18n.t('js.notice_successful_delete'));
 
-        if (this.$state.$current.data.hardReloadOnBaseRoute) {
-          const url = new URL(window.location.href);
-          url.search = '';
-          window.location.href = url.href;
-        } else {
-          let projectId;
-          if (query.project) {
-            projectId = query.project.href!.split('/').pop();
-          }
-
-          void this.loadDefaultQuery(projectId);
-
-          this.states.changes.queries.next(query.id!);
-          this.reloadSidemenu(null);
-        }
+        void this.navigateToDefaultQuery(query);
       });
 
     return promise;
@@ -317,10 +308,14 @@ export class WorkPackagesListService {
     void promise
       .then(() => {
         this.toastService.addSuccess(this.I18n.t('js.notice_successful_update'));
-
-        this.$state.go('.', { query_id: query!.id, query_props: null }, { reload: true });
-        this.states.changes.queries.next(query!.id!);
-        this.reloadSidemenu(query.id);
+        const queryAccessibleByUser = query.public || query.user.id === this.currentUser.userId;
+        if (queryAccessibleByUser) {
+          void this.$state.go('.', { query_id: query.id, query_props: null }, { reload: true });
+          this.states.changes.queries.next(query.id);
+          this.reloadSidemenu(query.id);
+        } else {
+          this.navigateToDefaultQuery(query);
+        }
       })
       .catch((error:ErrorResource) => {
         this.toastService.addError(error.message);
@@ -435,6 +430,26 @@ export class WorkPackagesListService {
       );
   }
 
+  private navigateToDefaultQuery(query:QueryResource):void {
+    const { hardReloadOnBaseRoute } = this.$state.$current.data as { hardReloadOnBaseRoute?:boolean };
+
+    if (hardReloadOnBaseRoute) {
+      const url = new URL(window.location.href);
+      url.search = '';
+      window.location.href = url.href;
+    } else {
+      let projectId;
+      if (query.project.href) {
+        projectId = query.project.href.split('/').pop();
+      }
+
+      void this.loadDefaultQuery(projectId);
+
+      this.states.changes.queries.next(query.id);
+      this.reloadSidemenu(null);
+    }
+  }
+
   private reloadSidemenu(selectedQueryId:string|null):void {
     const menuIdentifier:string|undefined = this.$state.current.data.sidemenuId;
 

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

@@ -39,10 +39,10 @@
   before do
     login_as(user)
     work_package
-    wp_table.visit!
   end
 
   it "allows refreshing the current query (Bug #26921)" do
+    wp_table.visit!
     wp_table.expect_work_package_listed work_package
     # Instantiate lazy let here
     wp_table.ensure_work_package_not_listed! other_work_package
@@ -65,4 +65,22 @@
 
     wp_table.expect_work_package_listed work_package, other_work_package
   end
+
+  describe "making a public query from another user private" do
+    let!(:other_public_view) do
+      create(:view,
+             query: create(:query, project:, public: true, name: "Other user query"))
+    end
+
+    it "redirects to the default query page" do
+      wp_table.visit_query(other_public_view.query)
+
+      wp_table.click_setting_item I18n.t("js.toolbar.settings.visibility_settings")
+      find_by_id("show-public").set false
+      find(".button", text: "Save").click
+
+      wp_table.expect_and_dismiss_toaster message: "Successful update."
+      expect(page).to have_current_path(project_work_packages_path(project))
+    end
+  end
 end