[32652] Add viewTopic action to extensions and hide BCF group if not present

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

frontend/src/app/modules/bim/bcf/api/bcf-authorization.service.ts

@@ -0,0 +1,65 @@
+import {multiInput} from "reactivestates";
+import {BcfExtensionResource} from "core-app/modules/bim/bcf/api/extensions/bcf-extension.resource";
+import {BcfApiService} from "core-app/modules/bim/bcf/api/bcf-api.service";
+import {Observable} from "rxjs";
+import {map, take} from "rxjs/operators";
+import {Injectable} from "@angular/core";
+
+export type AllowedExtensionKey = keyof BcfExtensionResource;
+
+@Injectable({ providedIn: 'root' })
+export class BcfAuthorizationService {
+
+  // Poor mans caching to avoid repeatedly fetching from the backend.
+  protected authorizationMap = multiInput<BcfExtensionResource>();
+
+  constructor(readonly bcfApi:BcfApiService) {
+  }
+
+  /**
+   * Returns an observable boolean whether the given action
+   * is authorized in the project by using the project extensions.
+   *
+   * Ensures the extension is loaded only once per project
+   *
+   * @param projectIdentifier Project identifier to check permission in
+   * @param extension The extension key to check for
+   * @param action The desired action
+   */
+  public authorized$(projectIdentifier:string, extension:AllowedExtensionKey, action:string):Observable<boolean> {
+    const state = this.authorizationMap.get(projectIdentifier);
+
+    state.putFromPromiseIfPristine(() =>
+      this.bcfApi
+        .projects.id(projectIdentifier)
+        .extensions
+        .get()
+        .toPromise()
+    );
+
+    return state
+      .values$()
+      .pipe(
+        map(
+          resource => resource[extension] && resource[extension].includes(action)
+        )
+      );
+  }
+
+  /**
+   * One-time check to determine current allowed state.
+   *
+   * @param projectIdentifier Project identifier to check permission in
+   * @param extension The extension key to check for
+   * @param action The desired action
+   */
+  public isAllowedTo(projectIdentifier:string, extension:AllowedExtensionKey, action:string):Promise<boolean> {
+    return this
+      .authorized$(projectIdentifier, extension, action)
+      .pipe(
+        take(1)
+      )
+      .toPromise();
+  }
+}
+

frontend/src/app/modules/bim/bcf/bcf-wp-attribute-group/bcf-new-wp-attribute-group.component.ts

@@ -1,7 +1,7 @@
 import {ChangeDetectionStrategy, Component} from "@angular/core";
-import {WorkPackageResource} from "core-app/modules/hal/resources/work-package-resource";
-import {take} from "rxjs/operators";
 import {BcfWpAttributeGroupComponent} from "core-app/modules/bim/bcf/bcf-wp-attribute-group/bcf-wp-attribute-group.component";
+import {take} from "rxjs/operators";
+import {WorkPackageResource} from "core-app/modules/hal/resources/work-package-resource";
 
 @Component({
   templateUrl: './bcf-wp-attribute-group.component.html',

frontend/src/app/modules/bim/bcf/bcf-wp-attribute-group/bcf-wp-attribute-group.component.html

@@ -1,4 +1,4 @@
-<ng-container *ngIf="viewpoints.length > 0 || viewerVisible">
+<ng-container *ngIf="viewAllowed && (viewpoints.length > 0 || viewerVisible)">
   <div class="attributes-group description-group">
     <div class="attributes-group--header">
       <div class="attributes-group--header-container">

frontend/src/app/modules/bim/bcf/bcf-wp-attribute-group/bcf-wp-attribute-group.component.ts

@@ -22,7 +22,7 @@ import {UntilDestroyedMixin} from "core-app/helpers/angular/until-destroyed.mixi
 import {NotificationsService} from "core-app/modules/common/notifications/notifications.service";
 import {BcfViewpointInterface} from "core-app/modules/bim/bcf/api/viewpoints/bcf-viewpoint.interface";
 import {WorkPackageCreateService} from "core-components/wp-new/wp-create.service";
-import {BcfExtensionResource} from "core-app/modules/bim/bcf/api/extensions/bcf-extension.resource";
+import {BcfAuthorizationService} from "core-app/modules/bim/bcf/api/bcf-authorization.service";
 
 export interface ViewpointItem {
   /** The URL of the viewpoint, if persisted */
@@ -110,8 +110,10 @@ export class BcfWpAttributeGroupComponent extends UntilDestroyedMixin implements
   // Remember the topic UUID, which we might just create
   topicUUID:string|undefined;
 
+  // Store whether viewing is allowed
+  viewAllowed:boolean = false;
   // Store whether viewpoint creation is allowed
-  createAllowed = false;
+  createAllowed:boolean = false;
 
   // Currently, this is static. Need observable if this changes over time
   viewerVisible = this.viewerBridge.viewerVisible();
@@ -120,6 +122,7 @@ export class BcfWpAttributeGroupComponent extends UntilDestroyedMixin implements
               readonly pathHelper:PathHelperService,
               readonly currentProject:CurrentProjectService,
               readonly bcfApi:BcfApiService,
+              readonly bcfAuthorization:BcfAuthorizationService,
               readonly viewerBridge:ViewerBridgeService,
               readonly wpCache:WorkPackageCacheService,
               readonly wpCreate:WorkPackageCreateService,
@@ -200,37 +203,21 @@ export class BcfWpAttributeGroupComponent extends UntilDestroyedMixin implements
       .pipe(
         this.untilDestroyed()
       )
-      .subscribe(wp => {
+      .subscribe(async wp => {
         this.workPackage = wp;
         this.setTopicUUIDFromWorkPackage();
 
-        this.fetchCreateAllowed();
+        const projectId = this.workPackage.project.idFromLink;
+        this.viewAllowed = await this.bcfAuthorization.isAllowedTo(projectId, 'project_actions', 'viewTopic');
+        this.createAllowed = await this.bcfAuthorization.isAllowedTo(projectId, 'topic_actions', 'createViewpoint');
 
         if (wp.bcfViewpoints) {
           this.setViewpoints(wp.bcfViewpoints.map((el:HalLink) => {
             return { href: el.href, snapshotURL: `${el.href}/snapshot` };
           }));
           this.loadViewpointFromRoute();
-          this.cdRef.detectChanges();
         }
-      });
-  }
-
-  // Poor mans caching to avoid repeatedly fetching from the backend.
-  protected createAllowedMemoize:{[key:string]:Promise<BcfExtensionResource>} = {};
-
-  protected fetchCreateAllowed() {
-    if (!this.createAllowedMemoize[this.wpProjectId]) {
-      this.createAllowedMemoize[this.wpProjectId] = this.bcfApi
-        .projects.id(this.wpProjectId)
-        .extensions
-        .get()
-        .toPromise();
-    }
-
-    this.createAllowedMemoize[this.wpProjectId]
-      .then(resource => {
-        this.createAllowed = resource.topic_actions && resource.topic_actions.includes('createViewpoint');
+       
         this.cdRef.detectChanges();
       });
   }

frontend/src/app/modules/common/autocomplete/version-autocompleter.component.ts

@@ -69,6 +69,10 @@ export class VersionAutocompleterComponent extends CreateAutocompleterComponent
    * @returns {Promise<boolean>}
    */
   public canCreateNewActionElements():Promise<boolean> {
+    if (!this.currentProject.id) {
+      return Promise.resolve(false);
+    }
+
     return this.versionDm
       .canCreateVersionInProject(this.currentProject.id!)
       .catch(() => false);
@@ -79,7 +83,7 @@ export class VersionAutocompleterComponent extends CreateAutocompleterComponent
       .then((version) => {
         this.onCreate.emit(version);
       })
-      .catch(error =>  {
+      .catch(error => {
         this.closeSelect();
         this.halNotification.handleRawError(error);
       });

modules/bim/app/representers/bim/bcf/api/v2_1/project_extensions/representer.rb

@@ -74,7 +74,12 @@ module Bim::Bcf::API::V2_1
              getter: ->(decorator:, **) {
                [].tap do |actions|
                  actions << 'update' if decorator.allowed?(:edit_project)
-                 actions << 'createTopic' if decorator.allowed?(:manage_bcf)
+
+                 if decorator.allowed?(:manage_bcf)
+                   actions << 'viewTopic' << 'createTopic'
+                 elsif decorator.allowed?(:view_linked_issues)
+                   actions << 'viewTopic'
+                 end
                end
              }
 

modules/bim/spec/features/viewer/show_viewpoint_spec.rb

@@ -100,5 +100,21 @@ describe 'Show viewpoint in model viewer', type: :feature, js: true do
       path = Regexp.escape("bcf/split/details/#{work_package.id}/overview")
       expect(page).to have_current_path /#{path}/
     end
+
+    context 'when user only has view_linked_issues permission' do
+      let(:permissions) { %i[view_ifc_models view_linked_issues manage_bcf view_work_packages] }
+
+      let(:user) do
+        FactoryBot.create :user,
+                          member_in_project: project,
+                          member_with_permissions: permissions
+      end
+
+      it 'does not show the viewpoint' do
+        wp_details.visit!
+        bcf_details.expect_viewpoint_count 0
+        expect(page).to have_no_selector('h3.attributes-group--header-text', text: 'BCF')
+      end
+    end
   end
 end

modules/bim/spec/representers/bcf/api/v2_1/project_extensions/representer_spec.rb

@@ -135,7 +135,15 @@ describe Bim::Bcf::API::V2_1::ProjectExtensions::Representer, 'rendering' do
       let(:path) { 'project_actions' }
 
       it_behaves_like 'attribute' do
-        let(:value) { %w(update createTopic) }
+        let(:value) { %w(update viewTopic createTopic) }
+      end
+
+      context 'with only view_linked_issues' do
+        let(:permissions) { %i[view_linked_issues] }
+
+        it_behaves_like 'attribute' do
+          let(:value) { %w[viewTopic] }
+        end
       end
 
       context 'when lacking manage_bcf' do
@@ -150,7 +158,7 @@ describe Bim::Bcf::API::V2_1::ProjectExtensions::Representer, 'rendering' do
         let(:permissions) { %i[manage_bcf] }
 
         it_behaves_like 'attribute' do
-          let(:value) { ['createTopic'] }
+          let(:value) { %w[viewTopic createTopic] }
         end
       end
     end

modules/bim/spec/requests/api/bcf/v2_1/project_extensions_api_spec.rb

@@ -105,7 +105,7 @@ describe 'BCF 2.1 project extensions resource', type: :request, content_type: :j
 
           expect(hash['user_id_type']).to include(other_user.mail, current_user.mail)
 
-          expect(hash['project_actions']).to eq %w[update createTopic]
+          expect(hash['project_actions']).to eq %w[update viewTopic createTopic]
 
           expect(hash['topic_actions']).to eq %w[update updateRelatedTopics updateFiles createViewpoint]
           expect(hash['comment_actions']).to eq []