From 82be5057a9657a77f0568233013fbde21f508554 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Wed, 11 Dec 2024 16:53:54 +0100 Subject: [PATCH] Make the dialog more usable --- .../core/path-helper/path-helper.service.ts | 66 ++++++----- .../dynamic/time-entry.controller.ts | 27 ++++- .../time_entries/time_entry_form.rb | 38 +++++-- .../projects/time_entries_controller.rb | 92 ---------------- .../controllers/time_entries_controller.rb | 104 ++++++++++++++++++ modules/costs/config/locales/en.yml | 1 + modules/costs/config/routes.rb | 11 +- 7 files changed, 209 insertions(+), 130 deletions(-) delete mode 100644 modules/costs/app/controllers/projects/time_entries_controller.rb create mode 100644 modules/costs/app/controllers/time_entries_controller.rb diff --git a/frontend/src/app/core/path-helper/path-helper.service.ts b/frontend/src/app/core/path-helper/path-helper.service.ts index 442dacf5e612..6603fc5e6ed4 100644 --- a/frontend/src/app/core/path-helper/path-helper.service.ts +++ b/frontend/src/app/core/path-helper/path-helper.service.ts @@ -41,7 +41,7 @@ export class PathHelperService { return this.appBasePath; } - public attachmentDownloadPath(attachmentIdentifier:string, slug:string|undefined) { + public attachmentDownloadPath(attachmentIdentifier:string, slug:string | undefined) { const path = `${this.staticBase}/attachments/${attachmentIdentifier}`; if (slug) { @@ -50,7 +50,7 @@ export class PathHelperService { return path; } - public attachmentContentPath(attachmentIdentifier:number|string) { + public attachmentContentPath(attachmentIdentifier:number | string) { return `${this.staticBase}/attachments/${attachmentIdentifier}/content`; } @@ -66,15 +66,15 @@ export class PathHelperService { return `${this.ifcModelsPath(projectIdentifier)}/new`; } - public ifcModelsEditPath(projectIdentifier:string, modelId:number|string) { + public ifcModelsEditPath(projectIdentifier:string, modelId:number | string) { return `${this.ifcModelsPath(projectIdentifier)}/${modelId}/edit`; } - public ifcModelsDeletePath(projectIdentifier:string, modelId:number|string) { + public ifcModelsDeletePath(projectIdentifier:string, modelId:number | string) { return `${this.ifcModelsPath(projectIdentifier)}/${modelId}`; } - public bimDetailsPath(projectIdentifier:string, workPackageId:string, viewpoint:number|string|null = null) { + public bimDetailsPath(projectIdentifier:string, workPackageId:string, viewpoint:number | string | null = null) { let path = `${this.projectPath(projectIdentifier)}/bcf/details/${workPackageId}`; if (viewpoint !== null) { @@ -168,7 +168,7 @@ export class PathHelperService { return `${this.projectPath(projectId)}/wiki`; } - public projectWorkPackagePath(projectId:string, wpId:string|number) { + public projectWorkPackagePath(projectId:string, wpId:string | number) { return `${this.projectWorkPackagesPath(projectId)}/${wpId}`; } @@ -180,14 +180,14 @@ export class PathHelperService { return `${this.projectWorkPackagesPath(projectId)}/new`; } - public boardsPath(projectIdentifier:string|null) { + public boardsPath(projectIdentifier:string | null) { if (projectIdentifier) { return `${this.projectPath(projectIdentifier)}/boards`; } return `${this.staticBase}/boards`; } - public newBoardsPath(projectIdentifier:string|null) { + public newBoardsPath(projectIdentifier:string | null) { return `${this.boardsPath(projectIdentifier)}/new`; } @@ -195,7 +195,7 @@ export class PathHelperService { return `${this.projectPath(projectIdentifier)}/dashboards`; } - public timeEntriesPath(workPackageId:string|number) { + public timeEntriesPath(workPackageId:string | number) { const suffix = '/time_entries'; if (workPackageId) { @@ -216,19 +216,19 @@ export class PathHelperService { return `${this.staticBase}/placeholder_users`; } - public userPath(id:string|number) { + public userPath(id:string | number) { return `${this.usersPath()}/${id}`; } - public userHoverCardPath(id:string|number) { + public userHoverCardPath(id:string | number) { return `${this.usersPath()}/${id}/hover_card`; } - public placeholderUserPath(id:string|number) { + public placeholderUserPath(id:string | number) { return `${this.placeholderUsersPath()}/${id}`; } - public groupPath(id:string|number) { + public groupPath(id:string | number) { return `${this.groupsPath()}/${id}`; } @@ -236,7 +236,7 @@ export class PathHelperService { return `${this.staticBase}/roles`; } - public rolePath(id:string|number) { + public rolePath(id:string | number) { return `${this.rolesPath()}/${id}`; } @@ -244,11 +244,11 @@ export class PathHelperService { return `${this.staticBase}/versions`; } - public versionEditPath(id:string|number) { + public versionEditPath(id:string | number) { return `${this.staticBase}/versions/${id}/edit`; } - public versionShowPath(id:string|number) { + public versionShowPath(id:string | number) { return `${this.staticBase}/versions/${id}`; } @@ -256,19 +256,19 @@ export class PathHelperService { return `${this.staticBase}/work_packages`; } - public workPackagePath(id:string|number) { + public workPackagePath(id:string | number) { return `${this.staticBase}/work_packages/${id}`; } - public workPackageShortPath(id:string|number) { + public workPackageShortPath(id:string | number) { return `${this.staticBase}/wp/${id}`; } - public workPackageCopyPath(workPackageId:string|number) { + public workPackageCopyPath(workPackageId:string | number) { return `${this.workPackagePath(workPackageId)}/copy`; } - public workPackageDetailsPath(projectIdentifier:string, workPackageId:string|number, tab?:string) { + public workPackageDetailsPath(projectIdentifier:string, workPackageId:string | number, tab?:string) { if (tab) { return `${this.projectWorkPackagePath(projectIdentifier, workPackageId)}/details/${tab}`; } @@ -276,19 +276,19 @@ export class PathHelperService { return `${this.projectWorkPackagesPath(projectIdentifier)}/details/${workPackageId}`; } - public workPackageDetailsCopyPath(projectIdentifier:string, workPackageId:string|number) { + public workPackageDetailsCopyPath(projectIdentifier:string, workPackageId:string | number) { return this.workPackageDetailsPath(projectIdentifier, workPackageId, 'copy'); } - public workPackageSharePath(workPackageId:string|number) { + public workPackageSharePath(workPackageId:string | number) { return `${this.workPackagePath(workPackageId)}/shares`; } - public workPackageHoverCardPath(workPackageId:string|number) { + public workPackageHoverCardPath(workPackageId:string | number) { return `${this.workPackagePath(workPackageId)}/hover_card`; } - public workPackageProgressModalPath(workPackageId:string|number) { + public workPackageProgressModalPath(workPackageId:string | number) { if (workPackageId === 'new') { return `${this.workPackagePath(workPackageId)}/progress/new`; } @@ -296,7 +296,7 @@ export class PathHelperService { return `${this.workPackagePath(workPackageId)}/progress/edit`; } - public workPackageUpdateCounterPath(workPackageId:string|number, counter:string) { + public workPackageUpdateCounterPath(workPackageId:string | number, counter:string) { return `${this.workPackagePath(workPackageId)}/split_view/update_counter?counter=${counter}`; } @@ -325,4 +325,20 @@ export class PathHelperService { public jobStatusModalPath(jobId:string) { return `${this.staticBase}/job_statuses/${jobId}/dialog`; } + + public timeEntriesUserTimezoneCaption(userId:string) { + return `${this.staticBase}/time_entries/users/${userId}/tz_caption`; + } + + public timeEntriesWorkPackageActicity(workPackageId:string) { + return `${this.staticBase}/time_entries/work_packages/${workPackageId}/time_entry_activities`; + } + + public timeEntryWorkPackageDialog(workPackageId:string) { + return `${this.workPackagePath(workPackageId)}/time_entries/dialog`; + } + + public timeEntryProjectDialog(projectId:string) { + return `${this.projectPath(projectId)}/time_entries/dialog`; + } } diff --git a/frontend/src/stimulus/controllers/dynamic/time-entry.controller.ts b/frontend/src/stimulus/controllers/dynamic/time-entry.controller.ts index c77232fcbe87..bbb2364a9662 100644 --- a/frontend/src/stimulus/controllers/dynamic/time-entry.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/time-entry.controller.ts @@ -30,6 +30,8 @@ import { Controller } from '@hotwired/stimulus'; import { parseChronicDuration, outputChronicDuration } from 'core-app/shared/helpers/chronic_duration'; +import { TurboRequestsService } from 'core-app/core/turbo/turbo-requests.service'; +import { PathHelperService } from 'core-app/core/path-helper/path-helper.service'; export default class TimeEntryController extends Controller { static targets = ['startTimeInput', 'endTimeInput', 'hoursInput']; @@ -40,6 +42,21 @@ export default class TimeEntryController extends Controller { declare readonly hasEndTimeInputTarget:boolean; declare readonly hoursInputTarget:HTMLInputElement; + private turboRequests:TurboRequestsService; + private pathHelper:PathHelperService; + + async connect() { + const context = await window.OpenProject.getPluginContext(); + this.turboRequests = context.services.turboRequests; + this.pathHelper = context.services.pathHelperService; + } + + userChanged(event:InputEvent) { + const userId = (event.currentTarget as HTMLInputElement).value; + + void this.turboRequests.request(this.pathHelper.timeEntriesUserTimezoneCaption(userId), { method: 'GET' }); + } + timeInputChanged(event:InputEvent) { this.datesChanged(event.currentTarget as HTMLInputElement); } @@ -62,7 +79,9 @@ export default class TimeEntryController extends Controller { if (startTimeInMinutes && endTimeInMinutes && (hoursInMinutes === 0 || initiatedBy === this.endTimeInputTarget)) { hoursInMinutes = endTimeInMinutes - startTimeInMinutes; if (hoursInMinutes <= 0) { hoursInMinutes += 24 * 60; } - this.hoursInputTarget.value = outputChronicDuration(hoursInMinutes * 60, { format: 'hours_only' }) || ''; + + const formattedHours = outputChronicDuration(hoursInMinutes * 60, { format: 'hours_only' }) || ''; + this.hoursInputTarget.value = formattedHours; } else if (startTimeInMinutes && hoursInMinutes) { const newEndTime = (startTimeInMinutes + hoursInMinutes) % (24 * 60); @@ -92,9 +111,11 @@ export default class TimeEntryController extends Controller { toggleEndTimePlusCaption(startTimeInMinutes:number, hoursInMinutes:number) { const formControl = this.endTimeInputTarget.closest('.FormControl') as HTMLElement; - formControl.querySelectorAll('.FormControl-caption').forEach((caption) => caption.remove()); + formControl + .querySelectorAll('.FormControl-caption') + .forEach((caption) => caption.remove()); - if (startTimeInMinutes + hoursInMinutes >= (24 * 60)) { + if (startTimeInMinutes + hoursInMinutes >= 24 * 60) { const diffInDays = Math.floor((startTimeInMinutes + hoursInMinutes) / (60 * 24)); const span = document.createElement('span'); span.className = 'FormControl-caption'; diff --git a/modules/costs/app/components/time_entries/time_entry_form.rb b/modules/costs/app/components/time_entries/time_entry_form.rb index 2e76e20851bc..43242765a043 100644 --- a/modules/costs/app/components/time_entries/time_entry_form.rb +++ b/modules/costs/app/components/time_entries/time_entry_form.rb @@ -5,15 +5,15 @@ class TimeEntryForm < ApplicationForm form do |f| f.autocompleter( name: :user_id, + id: "time_entry_user_id", label: TimeEntry.human_attribute_name(:user), required: true, autocomplete_options: { defaultData: true, + hiddenFieldAction: "change->time-entry#userChanged", component: "opce-user-autocompleter", url: ::API::V3::Utilities::PathHelper::ApiV3Path.principals, - filters: [{ name: "type", operator: "=", values: %w[User Group] }, - { name: "member", operator: "=", values: [model.project_id] }, - { name: "status", operator: "=", values: [Principal.statuses[:active], Principal.statuses[:invited]] }], + filters: user_completer_filter_options, searchKey: "any_name_attribute", resource: "principals", focusDirectly: false, @@ -57,12 +57,13 @@ class TimeEntryForm < ApplicationForm f.text_field name: :hours, required: true, label: TimeEntry.human_attribute_name(:hours), - value: ChronicDuration.output(model.hours * 3600, format: :hours_only), + value: model.hours.present? ? ChronicDuration.output(model.hours * 3600, format: :hours_only) : "", data: { "time-entry-target" => "hoursInput", "action" => "blur->time-entry#hoursChanged keypress.enter->time-entry#hoursKeyEnterPress" } f.work_package_autocompleter name: :work_package_id, label: TimeEntry.human_attribute_name(:work_package), + required: true, autocomplete_options: { focusDirectly: false, append_to: "#time-entry-dialog", @@ -71,10 +72,20 @@ class TimeEntryForm < ApplicationForm ] } - f.select_list name: :activity_id, label: TimeEntry.human_attribute_name(:activity), include_blank: true do |list| + f.autocompleter( + name: :activity_id, + label: TimeEntry.human_attribute_name(:activity), + required: false, + include_blank: true, + autocomplete_options: { + focusDirectly: false, + multiple: false, + decorated: true, + append_to: "#time-entry-dialog" + } + ) do |select| activities.each do |activity| - selected = (model.activity_id == activity.id) || (model.activity_id.blank? && activity.is_default?) - list.option(value: activity.id, label: activity.name, selected:) + select.option(value: activity.id, label: activity.name, selected: (model.activity_id == activity.id)) end end @@ -108,5 +119,18 @@ def show_start_and_end_time_fields? def activities TimeEntryActivity.active_in_project(project) end + + def user_autocompleter_filter_options + filters = [ + { name: "type", operator: "=", values: %w[User Group] }, + { name: "status", operator: "=", values: [Principal.statuses[:active], Principal.statuses[:invited]] } + ] + + if model.project_id + filters << { name: "member", operator: "=", values: [model.project_id] } + end + + filters + end end end diff --git a/modules/costs/app/controllers/projects/time_entries_controller.rb b/modules/costs/app/controllers/projects/time_entries_controller.rb deleted file mode 100644 index 7cd85daa8694..000000000000 --- a/modules/costs/app/controllers/projects/time_entries_controller.rb +++ /dev/null @@ -1,92 +0,0 @@ -#-- copyright -# OpenProject is an open source project management software. -# Copyright (C) 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. -#++ - -module Projects - class TimeEntriesController < ApplicationController - include OpTurbo::ComponentStream - include OpTurbo::DialogStreamHelper - - before_action :require_login - before_action :find_project_by_project_id - - authorization_checked! :dialog, :create, :update - - def dialog - @time_entry = if params[:time_entry_id] - # TODO: Properly handle authorization - TimeEntry.find_by(id: params[:time_entry_id]) - else - TimeEntry.new(project: @project, user: User.current) - end - end - - def create - call = TimeEntries::CreateService - .new(user: current_user) - .call(time_entry_params) - - @time_entry = call.result - - if call.success? - # TODO: just close here? - else - form_component = TimeEntries::TimeEntryFormComponent.new(time_entry: @time_entry) - update_via_turbo_stream(component: form_component, status: :bad_request) - - respond_with_turbo_streams - end - end - - def update - time_entry = TimeEntry.find_by(id: params[:id]) - - call = TimeEntries::UpdateService - .new(user: current_user, model: time_entry) - .call(time_entry_params) - - @time_entry = call.result - - if call.success? - # TODO: just close here? - else - form_component = TimeEntries::TimeEntryFormComponent.new(time_entry: @time_entry) - update_via_turbo_stream(component: form_component, status: :bad_request) - - respond_with_turbo_streams - end - end - - private - - def time_entry_params - permitted_params.time_entries.merge( - project_id: @project.id - ) - end - end -end diff --git a/modules/costs/app/controllers/time_entries_controller.rb b/modules/costs/app/controllers/time_entries_controller.rb new file mode 100644 index 000000000000..a3a71d5f264a --- /dev/null +++ b/modules/costs/app/controllers/time_entries_controller.rb @@ -0,0 +1,104 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 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. +#++ + +class TimeEntriesController < ApplicationController + include OpTurbo::ComponentStream + include OpTurbo::DialogStreamHelper + + before_action :require_login + before_action :find_project_by_project_id + + authorization_checked! :dialog, :create, :update, :user_caption + + def dialog + @time_entry = if params[:time_entry_id] + # TODO: Properly handle authorization + TimeEntry.find_by(id: params[:time_entry_id]) + else + TimeEntry.new(project: @project, user: User.current) + end + end + + def user_caption + user = User.visible.find_by(id: params[:user_id]) + caption = if user && user.time_zone != User.current.time_zone + I18n.t("notice_different_time_zones", tz: user.time_zone.name) + else + "" + end + + add_caption_to_input_element_via_turbo_stream("input[name=\"time_entry[user_id]\"]", + caption:, + clean_other_captions: true) + respond_with_turbo_streams + end + + def create + call = TimeEntries::CreateService + .new(user: current_user) + .call(time_entry_params) + + @time_entry = call.result + + if call.success? + # TODO: just close here? + else + form_component = TimeEntries::TimeEntryFormComponent.new(time_entry: @time_entry) + update_via_turbo_stream(component: form_component, status: :bad_request) + + respond_with_turbo_streams + end + end + + def update + time_entry = TimeEntry.find_by(id: params[:id]) + + call = TimeEntries::UpdateService + .new(user: current_user, model: time_entry) + .call(time_entry_params) + + @time_entry = call.result + + if call.success? + # TODO: just close here? + else + form_component = TimeEntries::TimeEntryFormComponent.new(time_entry: @time_entry) + update_via_turbo_stream(component: form_component, status: :bad_request) + + respond_with_turbo_streams + end + end + + private + + def time_entry_params + permitted_params.time_entries.merge( + project_id: @project.id + ) + end +end diff --git a/modules/costs/config/locales/en.yml b/modules/costs/config/locales/en.yml index 39542b6d1af9..e3f8c8db750e 100644 --- a/modules/costs/config/locales/en.yml +++ b/modules/costs/config/locales/en.yml @@ -147,6 +147,7 @@ en: notice_successful_restore: "Successful restore." notice_successful_lock: "Locked successfully." notice_cost_logged_successfully: "Unit cost logged successfully." + notice_different_time_zones: "This user has a different time zone (%{tz}). Time will be logged using their time zone." permission_edit_cost_entries: "Edit booked unit costs" permission_edit_own_cost_entries: "Edit own booked unit costs" diff --git a/modules/costs/config/routes.rb b/modules/costs/config/routes.rb index 07ff015f75e1..de104c706016 100644 --- a/modules/costs/config/routes.rb +++ b/modules/costs/config/routes.rb @@ -27,6 +27,12 @@ #++ Rails.application.routes.draw do + resources :time_entries, only: %i[create update] do + get :dialog, on: :collection + get "users/:user_id/tz_caption", action: :user_caption, on: :collection + get "work_packages/:work_package_id/time_entry_activities", action: :change_activities_input, on: :collection + end + scope "projects/:project_id", as: "projects" do resources :cost_entries, controller: "costlog", only: %i[new create] @@ -44,13 +50,12 @@ resource :time_entry_activities, only: %i[show update] end - resources :time_entries, only: %i[create update] do - get :dialog, on: :collection - end + get "/time_entries/dialog" => "time_entries#dialog" end scope "work_packages/:work_package_id", as: "work_packages" do resources :cost_entries, controller: "costlog", only: %i[new] + get "/time_entries/dialog" => "time_entries#dialog" end resources :cost_entries, controller: "costlog", only: %i[edit update destroy]