diff --git a/app/controllers/targets_controller.rb b/app/controllers/targets_controller.rb index f170c46..a72f27a 100644 --- a/app/controllers/targets_controller.rb +++ b/app/controllers/targets_controller.rb @@ -5,10 +5,9 @@ class TargetsController < ApplicationController include Concerns::Finders - before_action :find_binding_goal_by_project_id, only: [:edit] before_action :find_project_by_project_id, only: [:subthresholds] before_action :find_quantity_by_quantity_id, only: [:toggle_exposure] - before_action :find_goal_by_goal_id, only: [:index, :new, :create] + before_action :find_goal_by_goal_id, only: [:index, :new, :create, :edit, :update] before_action :find_goal, only: [:toggle_exposure] before_action :authorize #before_action :set_view_params @@ -39,21 +38,32 @@ class TargetsController < ApplicationController end else @targets = @goal.targets.select(&:changed_for_autosave?) - @targets.each { |t| t.thresholds.new unless t.thresholds.present? } + .each { |t| t.thresholds.new unless t.thresholds.present? } render :new end end def edit - @targets = @goal.targets.where(effective_from: params[:date]).to_a + @targets = @goal.targets.joins(:quantity).where(effective_from: params[:date]) + .order('quantities.lft' => :asc).to_a @effective_from = @targets.first&.effective_from end def update # TODO: DRY same code with #create - @goal = @project.goals.find(params[:goal_id]) if params[:goal_id].present? - @goal ||= @project.goals.new - @goal.attributes = goal_params unless @goal.is_binding? + @effective_from = params[:goal].delete(:effective_from) + params[:goal][:targets_attributes].each { |ta| ta[:effective_from] = @effective_from } + + byebug + if @goal.update(targets_params) + flash.now[:notice] = t('.success') + prepare_targets + else + @targets = @goal.targets.where(id: targets_params[:targets_attributes].pluck(:id)) + @targets += @goal.targets.select(&:changed_for_autosave?) + .each { |t| t.thresholds.new unless t.thresholds.present? } + render :edit + end end def destroy diff --git a/app/helpers/targets_helper.rb b/app/helpers/targets_helper.rb index 9d10c89..f3455c1 100644 --- a/app/helpers/targets_helper.rb +++ b/app/helpers/targets_helper.rb @@ -1,9 +1,9 @@ module TargetsHelper - def action_links(d) - link_to(l(:button_reapply), reapply_goal_targets_path(@project, d, @view_params), + def action_links(date) + link_to(l(:button_reapply), reapply_goal_targets_path(@goal, date, @view_params), {remote: true, class: "icon icon-reload"}) + - link_to(l(:button_edit), edit_goal_targets_path(@project, d, @view_params), + link_to(l(:button_edit), edit_goal_targets_path(@goal, date, @view_params), {remote: true, class: "icon icon-edit"}) + - delete_link(target_path(d), {remote: true, data: {}}) + delete_link(target_path(date), {remote: true, data: {}}) end end diff --git a/app/models/target.rb b/app/models/target.rb index f1a9c4c..480f9d0 100644 --- a/app/models/target.rb +++ b/app/models/target.rb @@ -17,6 +17,7 @@ class Target < ActiveRecord::Base errors.add(:thresholds, :quantity_mismatch) unless quantities == ancestors end #validates :scope, inclusion: {in: [:ingredient, :meal, :day], if: -> { quantity&.diet? }} + # FIXME: does not seem to work validates :effective_from, presence: {if: :is_binding?}, absence: {unless: :is_binding?} after_initialize do diff --git a/app/models/validations/nested_uniqueness.rb b/app/models/validations/nested_uniqueness.rb index 1c86be2..dfa1cc2 100644 --- a/app/models/validations/nested_uniqueness.rb +++ b/app/models/validations/nested_uniqueness.rb @@ -44,10 +44,10 @@ module Validations::NestedUniqueness def before_validation_nested_uniqueness nested_uniqueness_options.each do |association_name, options| collection = send(association_name) - was_loaded = collection.loaded? - records = collection.target.select(&:changed?) + records = collection.target.dup unless collection.loaded? - preserved_records = records.reject(&:marked_for_destruction?) + preserved_records = records.select(&:changed?).reject(&:marked_for_destruction?) + # TODO: do scoping on options[:attributes] and remove options[:scope] scope = options[:scope]&.map { |attr| [attr, preserved_records.map(&attr).uniq] }.to_h seen = {} @@ -62,7 +62,7 @@ module Validations::NestedUniqueness end end end - unless was_loaded + if records collection.proxy_association.reset records.each { |r| collection.proxy_association.add_to_target(r) } end diff --git a/app/views/targets/_edit_form.html.erb b/app/views/targets/_edit_form.html.erb index c987643..bd621d2 100644 --- a/app/views/targets/_edit_form.html.erb +++ b/app/views/targets/_edit_form.html.erb @@ -1,9 +1,9 @@ -<%= labelled_form_for @targets, - url: project_targets_path(@project, @view_params), - method: :patch, remote: true, - html: {id: 'edit-target-form', name: 'edit-target-form'} do |f| %> +<%= labelled_form_for @goal, url: goal_targets_path(@goal), method: :patch, remote: true, + html: {id: 'edit-target-form', name: 'edit-target-form'} do |goal_f| %> - <%= render partial: 'targets/form', locals: {f: f} %> +
diff --git a/app/views/targets/_form.html.erb b/app/views/targets/_form.html.erb index 53432ce..eec99bf 100644 --- a/app/views/targets/_form.html.erb +++ b/app/views/targets/_form.html.erb @@ -4,10 +4,11 @@
<% end %> -<%= goal_f.fields_for :targets, @targets, child_index: '' do |target_f| %> +<%= goal_f.fields_for :targets, @targets, index: '', child_index: '' do |target_f| %> <%= error_messages_for target_f.object %>+ <%= target_f.hidden_field :id %> <%= target_f.hidden_field :_destroy %> <%= t ".choose_quantity" %> <%= target_f.select :quantity_id, quantity_options, diff --git a/app/views/targets/_thresholds_form.html.erb b/app/views/targets/_thresholds_form.html.erb index 2c60190..cbd77f8 100644 --- a/app/views/targets/_thresholds_form.html.erb +++ b/app/views/targets/_thresholds_form.html.erb @@ -1,5 +1,5 @@ <% target_f.object.thresholds.new unless last_quantity.leaf? %> -<%= target_f.fields_for :thresholds, child_index: '' do |threshold_f| %> +<%= target_f.fields_for :thresholds, index: '', child_index: '' do |threshold_f| %> <% threshold_q = threshold_f.object.quantity %> <% parent_id = threshold_q.nil? ? last_quantity.id : threshold_q.parent_id %> <%= threshold_f.collection_select :quantity_id, @@ -16,6 +16,7 @@ } }); return false;"} %> + <%# TODO: update above ajax with -1/no change/+1 logic based on copying existing threshold and proper ID/destroy handling %> <%# {autocomplete: 'off', data: {remote: true, url: subthresholds_project_targets_path(parent_id: parent_id)}} %> diff --git a/config/routes.rb b/config/routes.rb index 5ab5884..65d6ef1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -11,9 +11,10 @@ resources :projects, shallow: true do member do post 'toggle_exposure', controller: :targets end - resources :targets, shallow: true, except: [:show, :edit, :update] do + resources :targets, except: [:show, :edit, :update] do collection do get 'edit/:date', action: :edit, as: :edit + patch '', action: :update post 'reapply/:date', action: :reapply, as: :reapply end end diff --git a/test/system/targets_test.rb b/test/system/targets_test.rb index 3ee9d1a..8aeffb5 100644 --- a/test/system/targets_test.rb +++ b/test/system/targets_test.rb @@ -86,6 +86,7 @@ class TargetsTest < BodyTrackingSystemTestCase '@project.targets.reload.count' => 1, 'Threshold.count' => 1 do visit goal_targets_path(@project.goals.binding) click_link t('targets.contextual.link_new_target') + within 'form#new-target-form' do fill_in t(:field_effective_from), with: date within 'p.target' do @@ -142,6 +143,7 @@ class TargetsTest < BodyTrackingSystemTestCase 'Threshold.count' => thresholds.length do visit goal_targets_path(@project.goals.binding) click_link t('targets.contextual.link_new_target') + within 'form#new-target-form' do within 'p.target' do select quantity.name @@ -167,9 +169,13 @@ class TargetsTest < BodyTrackingSystemTestCase end def test_create_multiple_targets + # TODO end - def test_create_form_retains_data_on_failure + def test_create_properly_handles_data_on_failure + # TODO + # * restoring non-empty targets values + # * removing empty targets end def test_create_duplicate_for_persisted_target_should_fail @@ -206,8 +212,10 @@ class TargetsTest < BodyTrackingSystemTestCase assert_no_difference 'Target.count' do visit goal_targets_path(@project.goals.binding) click_link t('targets.contextual.link_new_target') + within 'form#new-target-form' do fill_in t(:field_effective_from), with: date + within :xpath, '//p[@class="target"][1]' do select quantity.name select @project.quantities.target.roots.sample.name @@ -215,6 +223,7 @@ class TargetsTest < BodyTrackingSystemTestCase select @project.units.sample.shortname end click_link t('targets.form.button_new_target') + within :xpath, '//p[@class="target"][2]' do select quantity.name select @project.quantities.target.roots.sample.name @@ -222,42 +231,67 @@ class TargetsTest < BodyTrackingSystemTestCase select @project.units.sample.shortname end click_on t(:button_create) + assert_selector :xpath, '//p[@class="target"][last()]//preceding-sibling::div', text: msg end end end - def test_create_and_simultaneously_remove_persisted_duplicate - end - - # TODO: test_create_failure(s) - # * restoring non-empty targets values - # * removing empty targets - def test_edit_binding_target + # TODO: extend with item + scope target = @project.goals.binding.targets.sample + date = Date.current + rand(-10..10).days + quantity = @project.quantities.except_targets.where.not(id: target.quantity.id).sample + thresholds = + @project.quantities.target.where.not(parent: nil).sample.self_and_ancestors.map do |q| + [q, rand(-2000.0..2000.0).to_d(4), @project.units.sample] + end + visit goal_targets_path(@project.goals.binding) assert_no_selector 'form#edit-target-form' within find('td', text: target.effective_from).ancestor('tr') do click_link t(:button_edit) + assert_selector :xpath, 'following-sibling::tr//form[@id="edit-target-form"]' + end - within :xpath, 'following-sibling::tr//form' do + assert_no_difference 'Target.count' do + within 'form#edit-target-form' do assert has_field?(t(:field_effective_from), with: target.effective_from) + fill_in t(:field_effective_from), with: date - # NOTE: this assumes target.quantity is unique which is not enforced now - threshold = t.thresholds.first - within find('select option[selected]', exact_text: threshold.quantity.name) - .ancestor('p') do - assert has_select?(selected: t.condition) - assert has_field?(with: threshold.value) - assert has_select?(selected: threshold.unit.shortname) + within find('select option[selected]', exact_text: target.quantity.name) + .ancestor('p.target') do + target.thresholds.each do |threshold| + assert has_select?(selected: threshold.quantity.name) + assert has_field?(with: threshold.value) + assert has_select?(selected: threshold.unit.shortname) + end + + select quantity.name + thresholds.each do |t_quantity, t_value, t_unit| + within select(t_quantity.name).ancestor('select') do + find(:xpath, 'following-sibling::input').set(t_value) + find(:xpath, 'following-sibling::select[1]').select(t_unit.shortname) + end + end end + + click_on t(:button_save) end end - assert_selector 'form#edit-target-form', count: 1 + target.reload + assert_equal date, target.effective_from + assert_equal quantity, target.quantity + assert_equal thresholds.length, target.thresholds.length + thresholds.each_with_index do |threshold, index| + t_quantity, t_value, t_unit = threshold + assert_equal t_quantity, target.thresholds[index].quantity + assert_equal t_value, target.thresholds[index].value + assert_equal t_unit, target.thresholds[index].unit + end end def test_update @@ -293,5 +327,10 @@ class TargetsTest < BodyTrackingSystemTestCase end def test_update_swap_targets + # TODO + end + + def test_update_add_and_simultaneously_remove_persisted_duplicate + # TODO end end