Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[57423] Progress modal should always prevent save when there are invalid values #16735

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@
id: "progress-form",
html: { autocomplete: "off" },
data: { "application-target": "dynamic",
"work-packages--progress--preview-progress-target": "form",
"work-packages--progress--preview-target": "form",
controller: "work-packages--progress--focus-field " \
"work-packages--progress--preview-progress " \
"work-packages--progress--touched-field-marker" }
"work-packages--progress--preview" }
) do |f| %>
<%= flex_layout do |modal_body| %>
<% modal_body.with_row(classes: "FormControl-horizontalGroup--sm-vertical") do |_fields| %>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@
id: "progress-form",
html: { autocomplete: "off" },
data: { "application-target": "dynamic",
"work-packages--progress--preview-progress-target": "form",
"work-packages--progress--preview-target": "form",
controller: "work-packages--progress--focus-field " \
"work-packages--progress--preview-progress " \
"work-packages--progress--touched-field-marker" }
"work-packages--progress--preview" }
) do |f| %>
<%= flex_layout do |modal_body| %>
<% modal_body.with_row(classes: "FormControl-horizontalGroup--sm-vertical") do |_fields| %>
Expand Down
30 changes: 19 additions & 11 deletions app/controllers/work_packages/progress_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,24 +37,18 @@ class WorkPackages::ProgressController < ApplicationController
layout false
authorization_checked! :new, :edit, :create, :update

helper_method :modal_class

def new
make_fake_initial_work_package
set_progress_attributes_to_work_package

render modal_class.new(@work_package,
focused_field: params[:field],
touched_field_map:)
render progress_modal_component
end

def edit
find_work_package
set_progress_attributes_to_work_package

render modal_class.new(@work_package,
focused_field: params[:field],
touched_field_map:)
render progress_modal_component
end

# rubocop:disable Metrics/AbcSize
Expand All @@ -71,7 +65,9 @@ def create
# Angular has context as to the success or failure of
# the request in order to fetch the new set of Work Package
# attributes in the ancestry solely on success.
render :update, status: :unprocessable_entity
render turbo_stream: [
turbo_stream.morph("work_package_progress_modal", progress_modal_component)
], status: :unprocessable_entity
end
end
# following 3 lines to be removed in 15.0 with :percent_complete_edition feature flag removal
Expand All @@ -95,7 +91,9 @@ def update

if service_call.success?
respond_to do |format|
format.turbo_stream
format.turbo_stream do
render turbo_stream: []
end
end
else
respond_to do |format|
Expand All @@ -104,14 +102,20 @@ def update
# Angular has context as to the success or failure of
# the request in order to fetch the new set of Work Package
# attributes in the ancestry solely on success.
render :update, status: :unprocessable_entity
render turbo_stream: [
turbo_stream.morph("work_package_progress_modal", progress_modal_component)
], status: :unprocessable_entity
end
end
end
end

private

def progress_modal_component
modal_class.new(@work_package, focused_field:, touched_field_map:)
end

def modal_class
if WorkPackage.use_status_for_done_ratio?
WorkPackages::Progress::StatusBased::ModalBodyComponent
Expand All @@ -120,6 +124,10 @@ def modal_class
end
end

def focused_field
params[:field]
end

def find_work_package
@work_package = WorkPackage.visible.find(params[:work_package_id])
end
Expand Down
21 changes: 10 additions & 11 deletions app/forms/work_packages/pre_14_4_progress_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,12 @@ def initialize(work_package:,

group.hidden(name: :status_id_touched,
value: @touched_field_map["status_id_touched"] || false,
data: { "work-packages--progress--touched-field-marker-target": "touchedFieldInput",
"referrer-field": "work_package[status_id]" })
data: { "work-packages--progress--preview-target": "touchedFieldInput",
"referrer-field": "status_id" })
group.hidden(name: :estimated_hours_touched,
value: @touched_field_map["estimated_hours_touched"] || false,
data: { "work-packages--progress--touched-field-marker-target": "touchedFieldInput",
"referrer-field": "work_package[estimated_hours]" })
data: { "work-packages--progress--preview-target": "touchedFieldInput",
"referrer-field": "estimated_hours" })
else
render_text_field(group, name: :estimated_hours, label: I18n.t(:label_work))
render_text_field(group, name: :remaining_hours, label: I18n.t(:label_remaining_work),
Expand All @@ -105,12 +105,12 @@ def initialize(work_package:,

group.hidden(name: :estimated_hours_touched,
value: @touched_field_map["estimated_hours_touched"] || false,
data: { "work-packages--progress--touched-field-marker-target": "touchedFieldInput",
"referrer-field": "work_package[estimated_hours]" })
data: { "work-packages--progress--preview-target": "touchedFieldInput",
"referrer-field": "estimated_hours" })
group.hidden(name: :remaining_hours_touched,
value: @touched_field_map["remaining_hours_touched"] || false,
data: { "work-packages--progress--touched-field-marker-target": "touchedFieldInput",
"referrer-field": "work_package[remaining_hours]" })
data: { "work-packages--progress--preview-target": "touchedFieldInput",
"referrer-field": "remaining_hours" })
end
group.fields_for(:initial) do |builder|
::WorkPackages::ProgressForm::InitialValuesForm.new(builder, work_package:, mode:)
Expand Down Expand Up @@ -195,9 +195,8 @@ def format_to_smallest_fractional_part(number)
end

def default_field_options(name)
data = { "work-packages--progress--preview-progress-target": "progressInput",
"work-packages--progress--touched-field-marker-target": "progressInput",
action: "input->work-packages--progress--touched-field-marker#markFieldAsTouched" }
data = { "work-packages--progress--preview-target": "progressInput",
action: "input->work-packages--progress--preview#markFieldAsTouched" }

if @focused_field == name
data[:"work-packages--progress--focus-field-target"] = "fieldToFocus"
Expand Down
9 changes: 4 additions & 5 deletions app/forms/work_packages/progress_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ def readonly_text_field(group,
def hidden_touched_field(group, name:)
group.hidden(name: :"#{name}_touched",
value: touched(name),
data: { "work-packages--progress--touched-field-marker-target": "touchedFieldInput",
"referrer-field": "work_package[#{name}]" })
data: { "work-packages--progress--preview-target": "touchedFieldInput",
"referrer-field": name })
end

def touched(name)
Expand Down Expand Up @@ -197,9 +197,8 @@ def as_percent(value)
end

def default_field_options(name)
data = { "work-packages--progress--preview-progress-target": "progressInput",
"work-packages--progress--touched-field-marker-target": "progressInput",
action: "work-packages--progress--touched-field-marker#markFieldAsTouched" }
data = { "work-packages--progress--preview-target": "progressInput",
action: "work-packages--progress--preview#markFieldAsTouched" }

if @focused_field == name
data[:"work-packages--progress--focus-field-target"] = "fieldToFocus"
Expand Down
4 changes: 2 additions & 2 deletions app/forms/work_packages/progress_form/initial_values_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ def initialize(work_package:,
def hidden_initial_field(form, name:)
form.hidden(name:,
value: work_package.public_send(:"#{name}_was"),
data: { "work-packages--progress--touched-field-marker-target": "initialValueInput",
"referrer-field": "work_package[#{name}]" })
data: { "work-packages--progress--preview-target": "initialValueInput",
"referrer-field": name })
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ def derive_remaining_work?
end

def status_percent_complete_changed?
work_package.status_id_changed? && work_package.status.default_done_ratio != work_package.done_ratio_was
work_package.status_id.present? && work_package.status_id_came_from_user? \
&& work_package.status.default_done_ratio != work_package.done_ratio_was
end

# Update +% complete+ from the status if the status changed.
Expand Down
3 changes: 0 additions & 3 deletions app/views/work_packages/progress/update.turbo_stream.erb

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ export class ProgressPopoverEditFieldComponent extends ProgressEditFieldComponen

public onModalClosed():void {
this.opened = false;
this.frameSrc = '';

if (!this.handler.inEditMode) {
this.handler.deactivate(false);
Expand Down

This file was deleted.

Loading