From e812741863040ebe6f7d5590cfc2f342548b2b4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 14 Nov 2023 17:32:07 +0100 Subject: [PATCH 1/2] Show meeting tab when user has view_meetings anywhere https://community.openproject.org/work_packages/51010 Also, adds the count to the meeting tab as a sideeffect --- .../work_package_meetings_tab_controller.rb | 15 +++++-- modules/meeting/config/locales/js-en.yml | 31 ++++++++++++++ modules/meeting/config/routes.rb | 4 +- modules/meeting/frontend/module/main.ts | 36 ++++++++++++---- .../lib/open_project/meeting/engine.rb | 2 +- .../patches/api/work_package_representer.rb | 2 +- .../work_package_meetings_tab_spec.rb | 42 ++++++++++++++++++- .../pages/work_package_meetings_tab.rb | 10 ++++- 8 files changed, 126 insertions(+), 16 deletions(-) create mode 100644 modules/meeting/config/locales/js-en.yml diff --git a/modules/meeting/app/controllers/work_package_meetings_tab_controller.rb b/modules/meeting/app/controllers/work_package_meetings_tab_controller.rb index e0afcfb60000..0f4bff44e41c 100644 --- a/modules/meeting/app/controllers/work_package_meetings_tab_controller.rb +++ b/modules/meeting/app/controllers/work_package_meetings_tab_controller.rb @@ -31,7 +31,7 @@ class WorkPackageMeetingsTabController < ApplicationController include Meetings::WorkPackageMeetingsTabComponentStreams before_action :set_work_package - before_action :authorize + before_action :authorize_global def index direction = params[:direction]&.to_sym || :upcoming # default to upcoming @@ -50,6 +50,11 @@ def index ) end + def count + count = get_grouped_agenda_items(:upcoming).count + render json: { count: } + end + def add_work_package_to_meeting_dialog render(WorkPackageMeetingsTab::AddWorkPackageToMeetingFormComponent.new(work_package: @work_package), layout: false) end @@ -97,8 +102,8 @@ def add_work_package_to_meeting_params end def set_agenda_items(direction) - upcoming_agenda_items_grouped_by_meeting = get_agenda_items_of_work_package(:upcoming).group_by(&:meeting) - past_agenda_items_grouped_by_meeting = get_agenda_items_of_work_package(:past).group_by(&:meeting) + upcoming_agenda_items_grouped_by_meeting = get_grouped_agenda_items(:upcoming) + past_agenda_items_grouped_by_meeting = get_grouped_agenda_items(:past) @upcoming_meetings_count = upcoming_agenda_items_grouped_by_meeting.count @past_meetings_count = past_agenda_items_grouped_by_meeting.count @@ -111,6 +116,10 @@ def set_agenda_items(direction) end end + def get_grouped_agenda_items(direction) + get_agenda_items_of_work_package(direction).group_by(&:meeting) + end + def get_agenda_items_of_work_package(direction) agenda_items = MeetingAgendaItem .includes(:meeting) diff --git a/modules/meeting/config/locales/js-en.yml b/modules/meeting/config/locales/js-en.yml new file mode 100644 index 000000000000..48ecdcc08fe8 --- /dev/null +++ b/modules/meeting/config/locales/js-en.yml @@ -0,0 +1,31 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2023 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-2013 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 COPYRIGHT and LICENSE files for more details. +#++ + +en: + js: + label_meetings: 'Meetings' diff --git a/modules/meeting/config/routes.rb b/modules/meeting/config/routes.rb index a27824fb4daa..0993ecbd6208 100644 --- a/modules/meeting/config/routes.rb +++ b/modules/meeting/config/routes.rb @@ -34,7 +34,9 @@ resources :work_packages, only: %i[] do resources :meetings, only: %i[] do collection do - resources :tab, only: %i[index], controller: 'work_package_meetings_tab', as: 'meetings_tab' + resources :tab, only: %i[index], controller: 'work_package_meetings_tab', as: 'meetings_tab' do + get :count, on: :collection + end end end resources :meeting_agenda_items, only: %i[] do diff --git a/modules/meeting/frontend/module/main.ts b/modules/meeting/frontend/module/main.ts index a23cc22a9dde..bb4310557dc9 100644 --- a/modules/meeting/frontend/module/main.ts +++ b/modules/meeting/frontend/module/main.ts @@ -24,23 +24,43 @@ // // See COPYRIGHT and LICENSE files for more details. -import { - Injector, - NgModule, - CUSTOM_ELEMENTS_SCHEMA -} from '@angular/core'; +import { CUSTOM_ELEMENTS_SCHEMA, Injector, NgModule } from '@angular/core'; import { OpSharedModule } from 'core-app/shared/shared.module'; import { OpenprojectTabsModule } from 'core-app/shared/components/tabs/openproject-tabs.module'; -import { WorkPackageTabsService } from 'core-app/features/work-packages/components/wp-tabs/services/wp-tabs/wp-tabs.service'; +import { + WorkPackageTabsService, +} from 'core-app/features/work-packages/components/wp-tabs/services/wp-tabs/wp-tabs.service'; import { MeetingsTabComponent } from './meetings-tab/meetings-tab.component'; +import { WorkPackageResource } from 'core-app/features/hal/resources/work-package-resource'; +import { PathHelperService } from 'core-app/core/path-helper/path-helper.service'; +import { HttpClient } from '@angular/common/http'; +import { map } from 'rxjs/operators'; +import { Observable } from 'rxjs'; +import { I18nService } from 'core-app/core/i18n/i18n.service'; + + +export function workPackageMeetingsCount( + workPackage:WorkPackageResource, + injector:Injector, +):Observable { + const pathHelperService = injector.get(PathHelperService); + const http = injector.get(HttpClient); + return http + .get(`${pathHelperService.workPackagePath(workPackage.id as string)}/meetings/tab/count`) + .pipe( + map((res:{ count:number }) => res.count), + ); +} export function initializeMeetingPlugin(injector:Injector) { const wpTabService = injector.get(WorkPackageTabsService); + const I18n = injector.get(I18nService); wpTabService.register({ component: MeetingsTabComponent, - name: "Meetings", + name: I18n.t('js.label_meetings'), id: 'meetings', - displayable: (workPackage) => !!workPackage.meetings + displayable: (workPackage) => !!workPackage.meetings, + count: workPackageMeetingsCount, }); } diff --git a/modules/meeting/lib/open_project/meeting/engine.rb b/modules/meeting/lib/open_project/meeting/engine.rb index e039cce8c7ef..45e45d3d41a4 100644 --- a/modules/meeting/lib/open_project/meeting/engine.rb +++ b/modules/meeting/lib/open_project/meeting/engine.rb @@ -43,7 +43,7 @@ class Engine < ::Rails::Engine { meetings: %i[index show download_ics], meeting_agendas: %i[history show diff], meeting_minutes: %i[history show diff], - work_package_meetings_tab: %i[index] }, + work_package_meetings_tab: %i[index count] }, permissible_on: :project permission :create_meetings, { meetings: %i[new create copy] }, diff --git a/modules/meeting/lib/open_project/meeting/patches/api/work_package_representer.rb b/modules/meeting/lib/open_project/meeting/patches/api/work_package_representer.rb index 1d420c39564e..7464b3975839 100644 --- a/modules/meeting/lib/open_project/meeting/patches/api/work_package_representer.rb +++ b/modules/meeting/lib/open_project/meeting/patches/api/work_package_representer.rb @@ -7,7 +7,7 @@ module WorkPackageRepresenter def extension ->(*) do link :meetings, - cache_if: -> { current_user.allowed_in_project?(:view_meetings, represented.project) } do + cache_if: -> { current_user.allowed_in_any_project?(:view_meetings) } do next if represented.new_record? { diff --git a/modules/meeting/spec/features/structured_meetings/work_package_meetings_tab_spec.rb b/modules/meeting/spec/features/structured_meetings/work_package_meetings_tab_spec.rb index 367807a0276d..9b1229583c84 100644 --- a/modules/meeting/spec/features/structured_meetings/work_package_meetings_tab_spec.rb +++ b/modules/meeting/spec/features/structured_meetings/work_package_meetings_tab_spec.rb @@ -72,6 +72,24 @@ meetings_tab.expect_tab_not_present end + + context 'when the user has permission in another project' do + let(:other_project) { create(:project, enabled_module_names: %w[meetings]) } + + let(:user) do + create(:user, + member_with_roles: { project => role }, + member_with_permissions: { + other_project => %i(view_work_packages view_meetings) + }) + end + + it 'does show the tab' do + work_package_page.visit! + + meetings_tab.expect_tab_present + end + end end context 'when the user has the permission to see the tab, but the work package is linked in two projects' do @@ -96,6 +114,7 @@ work_package_page.visit! switch_to_meetings_tab + meetings_tab.expect_tab_count(1) meetings_tab.expect_upcoming_counter_to_be(1) meetings_tab.expect_past_counter_to_be(0) @@ -110,13 +129,34 @@ end context 'when the meetings module is not enabled for the project' do - let(:project) { create(:project, disable_modules: 'meetings') } + before do + project.enabled_module_names = ['work_package_tracking'] + project.save! + end it 'does not show the meetings tab' do work_package_page.visit! meetings_tab.expect_tab_not_present end + + context 'when the user has permission in another project' do + let(:other_project) { create(:project, enabled_module_names: %w[meetings]) } + + let(:user) do + create(:user, + member_with_permissions: { + project => %i(view_work_packages), + other_project => %i(view_work_packages view_meetings) + }) + end + + it 'does show the tab' do + work_package_page.visit! + + meetings_tab.expect_tab_present + end + end end context 'when the work_package is not referenced in an upcoming meeting' do diff --git a/modules/meeting/spec/support/pages/work_package_meetings_tab.rb b/modules/meeting/spec/support/pages/work_package_meetings_tab.rb index b7a1436d510b..4aa1278aac67 100644 --- a/modules/meeting/spec/support/pages/work_package_meetings_tab.rb +++ b/modules/meeting/spec/support/pages/work_package_meetings_tab.rb @@ -42,8 +42,16 @@ def path "/work_packages/#{work_package_id}/tabs/meetings" end + def expect_tab_present + expect(page).to have_css('.op-tab-row--link', text: 'MEETINGS') + end + + def expect_tab_count(count) + expect(page).to have_css('.op-tab-row--link', text: "MEETINGS (#{count})", wait: 10) + end + def expect_tab_not_present - expect(page).not_to have_selector('.op-tab-row--link', text: 'MEETINGS') + expect(page).not_to have_css('.op-tab-row--link', text: 'MEETINGS') end def expect_tab_content_rendered From 451c635ea6dd59473c7b87ec4b66ccf578f40d10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Wed, 15 Nov 2023 10:10:58 +0100 Subject: [PATCH 2/2] Reload the meeting tab counter on frame or stream changes --- frontend/src/typings/turbo.d.ts | 5 +++ frontend/tsconfig.base.json | 3 ++ modules/meeting/frontend/module/main.ts | 35 +++++++++++++++---- .../meetings-tab/meetings-tab.component.ts | 25 ++----------- 4 files changed, 39 insertions(+), 29 deletions(-) diff --git a/frontend/src/typings/turbo.d.ts b/frontend/src/typings/turbo.d.ts index d598b03fcc97..e7a6389d8e42 100644 --- a/frontend/src/typings/turbo.d.ts +++ b/frontend/src/typings/turbo.d.ts @@ -1,3 +1,8 @@ export interface TurboElement { reload:() => void; } + +export interface TurboStreamElement extends HTMLElement { + action:string; + target:string; +} diff --git a/frontend/tsconfig.base.json b/frontend/tsconfig.base.json index 5c70499f6627..e79dfae202ed 100644 --- a/frontend/tsconfig.base.json +++ b/frontend/tsconfig.base.json @@ -31,6 +31,9 @@ "paths": { "@ng-select/ng-select": ["../node_modules/@ng-select/ng-select/"], "core-app/*": ["./app/*"], + "core-typings/*": [ + "./typings/*" + ], "core-components/*": ["./app/components/*"], "core-vendor/*": ["./vendor/*"] } diff --git a/modules/meeting/frontend/module/main.ts b/modules/meeting/frontend/module/main.ts index bb4310557dc9..af83bcdb9c0e 100644 --- a/modules/meeting/frontend/module/main.ts +++ b/modules/meeting/frontend/module/main.ts @@ -34,10 +34,10 @@ import { MeetingsTabComponent } from './meetings-tab/meetings-tab.component'; import { WorkPackageResource } from 'core-app/features/hal/resources/work-package-resource'; import { PathHelperService } from 'core-app/core/path-helper/path-helper.service'; import { HttpClient } from '@angular/common/http'; -import { map } from 'rxjs/operators'; -import { Observable } from 'rxjs'; +import { filter, map, startWith, switchMap, throttleTime } from 'rxjs/operators'; +import { fromEvent, merge, Observable } from 'rxjs'; import { I18nService } from 'core-app/core/i18n/i18n.service'; - +import { TurboStreamElement } from 'core-typings/turbo'; export function workPackageMeetingsCount( workPackage:WorkPackageResource, @@ -45,10 +45,33 @@ export function workPackageMeetingsCount( ):Observable { const pathHelperService = injector.get(PathHelperService); const http = injector.get(HttpClient); - return http - .get(`${pathHelperService.workPackagePath(workPackage.id as string)}/meetings/tab/count`) + + return merge( + fromEvent(document, 'turbo:frame-render'), + fromEvent(document, 'turbo:before-stream-render'), + ) .pipe( - map((res:{ count:number }) => res.count), + filter((event:CustomEvent) => { + if (event.type === 'turbo:frame-render') { + return (event.target as HTMLElement).id?.includes('work-package-meetings-tab'); + } + + if (event.type === 'turbo:before-stream-render') { + const stream:TurboStreamElement = (event.detail as { newStream:TurboStreamElement }).newStream; + return stream.target?.includes('work-package-meetings-tab'); + } + + return false; + }), + startWith(null), + throttleTime(1000), + switchMap(() => { + return http + .get(`${pathHelperService.workPackagePath(workPackage.id as string)}/meetings/tab/count`) + .pipe( + map((res:{ count:number }) => res.count), + ); + }), ); } diff --git a/modules/meeting/frontend/module/meetings-tab/meetings-tab.component.ts b/modules/meeting/frontend/module/meetings-tab/meetings-tab.component.ts index 766025714e43..94e953904f7f 100644 --- a/modules/meeting/frontend/module/meetings-tab/meetings-tab.component.ts +++ b/modules/meeting/frontend/module/meetings-tab/meetings-tab.component.ts @@ -26,10 +26,7 @@ // See COPYRIGHT and LICENSE files for more details. //++ -import { - AfterViewInit, ChangeDetectionStrategy, - Component, ElementRef, Input, OnInit, -} from '@angular/core'; +import { ChangeDetectionStrategy, Component, ElementRef, Input, OnInit } from '@angular/core'; import { WorkPackageResource } from 'core-app/features/hal/resources/work-package-resource'; import { TabComponent } from 'core-app/features/work-packages/components/wp-tabs/components/wp-tab-wrapper/tab'; import { I18nService } from 'core-app/core/i18n/i18n.service'; @@ -40,7 +37,7 @@ import { PathHelperService } from 'core-app/core/path-helper/path-helper.service templateUrl: './meetings-tab.template.html', changeDetection: ChangeDetectionStrategy.OnPush, }) -export class MeetingsTabComponent implements OnInit, AfterViewInit, TabComponent { +export class MeetingsTabComponent implements OnInit, TabComponent { @Input() public workPackage:WorkPackageResource; turboFrameSrc:string; @@ -51,24 +48,6 @@ export class MeetingsTabComponent implements OnInit, AfterViewInit, TabComponent ) {} ngOnInit():void { - // TODO: Should we try to restore the last selected tab via localStorage as done in following commented code? - // - // const storedSrc = localStorage.getItem(`turboFrameSrcMeetingsTabForWorkPackage${this.workPackage.id}`); - // this.turboFrameSrc = storedSrc ? storedSrc : `/work_packages/${this.workPackage.id}/meetings/tab`; - this.turboFrameSrc = `${this.PathHelper.staticBase}/work_packages/${this.workPackage.id}/meetings/tab`; } - - ngAfterViewInit():void { - // TODO: Should we try to restore the last selected tab via localStorage as done in following commented code? - // - // const turboFrame = this.elementRef.nativeElement.querySelector('#work-package-meetings-tab-content'); - // if (turboFrame) { - // turboFrame.addEventListener('turbo:frame-load', (event: Event) => { - // const target = event.target as HTMLElement; - // const newSrc = target.getAttribute('src'); - // localStorage.setItem(`turboFrameSrcMeetingsTabForWorkPackage${this.workPackage.id}`, newSrc||''); - // }); - // } - } }