From dba88090574f904ee4c81520e368dba78d5c5e07 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 25 Jul 2023 14:28:51 +1000 Subject: [PATCH 1/6] Handle deleting tag when directive destroyed --- .../directives/obj_for_update.js.coffee | 36 +++++++++++++++---- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/admin/index_utils/directives/obj_for_update.js.coffee b/app/assets/javascripts/admin/index_utils/directives/obj_for_update.js.coffee index 18c800ce7f..e9da4c19d5 100644 --- a/app/assets/javascripts/admin/index_utils/directives/obj_for_update.js.coffee +++ b/app/assets/javascripts/admin/index_utils/directives/obj_for_update.js.coffee @@ -11,14 +11,9 @@ angular.module("admin.indexUtils").directive "objForUpdate", (switchClass, pendi pendingChanges.remove(scope.object().id, scope.attr) scope.clear() else - change = - object: scope.object() - type: scope.type - attr: scope.attr - value: if value? then value else "" - scope: scope scope.pending() - pendingChanges.add(scope.object().id, scope.attr, change) + value = if value? then value else "" + addPendingChange(scope.attr, value) scope.reset = (value) -> scope.savedValue = value @@ -34,3 +29,30 @@ angular.module("admin.indexUtils").directive "objForUpdate", (switchClass, pendi scope.clear = -> switchClass( element, "", ["update-pending", "update-error", "update-success"], false ) + + # In the particular case of a list of customer filtered by a tag, we want to make sure the + # tag is removed when deleting the tag the list is filtered by. + # As the list is filter by tag, deleting the tag will remove the customer entry, thus + # removing "objForUpdate" directive from the active scope. That means $watch won't pick up + # the tag_list changed. + # To ensure the tag is still deleted, we check on the $destroy event to see if the tag_list has + # changed, if so we queue up deleting the tag. + scope.$on '$destroy', (value) -> + return if scope.attr != 'tag_list' + + # No tag has been deleted + return if scope.object()['tag_list'] == scope.savedValue + + # Queuing up change to delete tag + addPendingChange('tag_list', scope.object()['tag_list']) + + # private + + addPendingChange = (attr, value) -> + change = + object: scope.object() + type: scope.type + attr: attr + value: value + scope: scope + pendingChanges.add(scope.object().id, attr, change) From f96bd51344cae12ac0bba8467b88e1da9a26bf39 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 25 Jul 2023 14:29:29 +1000 Subject: [PATCH 2/6] Rework form to allow displaying the save bar when no result When filtering by a tag, and then removing the tag on the filtered customers, it let us show the save bar once the last customer has been removed from the filtered list. --- app/views/admin/customers/index.html.haml | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/app/views/admin/customers/index.html.haml b/app/views/admin/customers/index.html.haml index 3bacae5a4e..e0e2aaf02c 100644 --- a/app/views/admin/customers/index.html.haml +++ b/app/views/admin/customers/index.html.haml @@ -39,16 +39,15 @@ %h1 = t :loading_customers - .row{ :class => "sixteen columns alpha", 'ng-show' => '!RequestMonitor.loading && filteredCustomers.length == 0'} - %h1#no_results - =t :no_customers_found - - .row.margin-bottom-50{ ng: { show: "!RequestMonitor.loading && filteredCustomers.length > 0" } } + .row.margin-bottom-50{ ng: { show: "!RequestMonitor.loading" } } %form{ name: "customers_form" } + %h1#no_results{ 'ng-show' => '!RequestMonitor.loading && filteredCustomers.length == 0' } + =t :no_customers_found + %save-bar{ dirty: "customers_form.$dirty", persist: "false" } %input.red{ type: "button", value: t(:save_changes), ng: { click: "submitAll(customers_form)" } } - %table.index#customers + %table.index#customers{ 'ng-show' => '!RequestMonitor.loading && filteredCustomers.length > 0' } %col.email{ width: "20%", 'ng-show' => 'columns.email.visible' } %col.first_name{ width: "20%", 'ng-show' => 'columns.first_name.visible' } %col.last_name{ width: "20%", 'ng-show' => 'columns.last_name.visible' } From 41dbad629b96c82bb7e1e07919b59f88e3f95f07 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 8 Aug 2023 14:53:54 +1000 Subject: [PATCH 3/6] Add system spec to cover deleting data when filtering covers tag scenario and code scenario --- spec/system/admin/customers_spec.rb | 74 ++++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 2 deletions(-) diff --git a/spec/system/admin/customers_spec.rb b/spec/system/admin/customers_spec.rb index fc7172697b..e50c0c71db 100644 --- a/spec/system/admin/customers_spec.rb +++ b/spec/system/admin/customers_spec.rb @@ -37,8 +37,7 @@ describe 'Customers' do # Prompts for a hub for a list of my managed enterprises expect(page) .to have_select2 "shop_id", with_options: [managed_distributor1.name, - managed_distributor2.name], - without_options: [unmanaged_distributor.name] + managed_distributor2.name], without_options: [unmanaged_distributor.name] select2_select managed_distributor2.name, from: "shop_id" @@ -185,6 +184,77 @@ describe 'Customers' do end end + describe "filtering" do + before do + customer4.update enterprise: managed_distributor1 + end + + context "when filtering by code" do + before do + customer4.update code: 12345 + + select2_select managed_distributor1.name, from: "shop_id" + + fill_in "quick_search", with: customer4.code + end + + it "displays only customer matching the code" do + expect(page).to have_content(customer4.email) + expect(page).not_to have_content(customer1.email) + expect(page).not_to have_content(customer2.email) + end + + context "when updating code" do + pending "allows user to save changes" do + fill_in "code", with: "" + + expect(page).not_to have_content("12345") + expect(page).to have_content 'You have unsaved changes' + + click_button "Save Changes" + + # changes are saved in the database + expect(customer4.reload.code).to eq(nil) + end + end + end + + context "when filtering by tag" do + before do + # Add test_tag to customer4 + select2_select managed_distributor1.name, from: "shop_id" + within "tr#c_#{customer4.id}" do + find(:css, "tags-input .tags input").set "test_tag\n" + end + click_button "Save Changes" + + # Reload the page + visit admin_customers_path + select2_select managed_distributor1.name, from: "shop_id" + fill_in "quick_search", with: "test_tag" + end + + it "displays only customer matching the tag" do + expect(page).to have_content(customer4.email) + expect(page).not_to have_content(customer1.email) + expect(page).not_to have_content(customer2.email) + end + + context "when removing tag" do + it "allows user to save changes" do + find("tags-input li.tag-item a.remove-button").click + + expect(page).to have_content("No customers found") + expect(page).to have_content 'You have unsaved changes' + + click_button "Save Changes" + + expect(customer4.reload.tag_list).to be_empty + end + end + end + end + it "allows updating of attributes" do select2_select managed_distributor1.name, from: "shop_id" From 4e468c81f9e7b789e086eb480086f16882a64911 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 8 Aug 2023 15:21:23 +1000 Subject: [PATCH 4/6] Handle updating customer attribute when directive is destroyed Handle any customer attribute not just tags --- .../directives/obj_for_update.js.coffee | 25 +++++++++++-------- spec/system/admin/customers_spec.rb | 2 +- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/app/assets/javascripts/admin/index_utils/directives/obj_for_update.js.coffee b/app/assets/javascripts/admin/index_utils/directives/obj_for_update.js.coffee index e9da4c19d5..f170d92437 100644 --- a/app/assets/javascripts/admin/index_utils/directives/obj_for_update.js.coffee +++ b/app/assets/javascripts/admin/index_utils/directives/obj_for_update.js.coffee @@ -30,21 +30,24 @@ angular.module("admin.indexUtils").directive "objForUpdate", (switchClass, pendi scope.clear = -> switchClass( element, "", ["update-pending", "update-error", "update-success"], false ) - # In the particular case of a list of customer filtered by a tag, we want to make sure the - # tag is removed when deleting the tag the list is filtered by. - # As the list is filter by tag, deleting the tag will remove the customer entry, thus + # When a list of customer is filtered and we removed the "filtered value" from a customer, we + # want to make sure the customer is updated. IE. filtering by tag, and removing said tag. + # Deleting the "filtered value" from a customer will remove the customer entry, thus # removing "objForUpdate" directive from the active scope. That means $watch won't pick up - # the tag_list changed. - # To ensure the tag is still deleted, we check on the $destroy event to see if the tag_list has - # changed, if so we queue up deleting the tag. + # the attribute changed. + # To ensure the customer is still updated, we check on the $destroy event to see if + # the attribute has changed, if so we queue up the change. scope.$on '$destroy', (value) -> - return if scope.attr != 'tag_list' + # No update + return if scope.object()[scope.attr] is scope.savedValue - # No tag has been deleted - return if scope.object()['tag_list'] == scope.savedValue + # For some reason the code attribute is removed from the object when cleared, so we add + # an emptyvalue so it gets updated properly + if scope.attr is "code" and scope.object()[scope.attr] is undefined + scope.object()["code"] = "" - # Queuing up change to delete tag - addPendingChange('tag_list', scope.object()['tag_list']) + # Queuing up change + addPendingChange(scope.attr, scope.object()[scope.attr]) # private diff --git a/spec/system/admin/customers_spec.rb b/spec/system/admin/customers_spec.rb index e50c0c71db..22b6125732 100644 --- a/spec/system/admin/customers_spec.rb +++ b/spec/system/admin/customers_spec.rb @@ -205,7 +205,7 @@ describe 'Customers' do end context "when updating code" do - pending "allows user to save changes" do + it "allows user to save changes" do fill_in "code", with: "" expect(page).not_to have_content("12345") From a64023c3df9a5bd74038f338caa45bc822c40534 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 8 Aug 2023 15:22:48 +1000 Subject: [PATCH 5/6] Apply review suggestion, simplify code slightly --- .../admin/index_utils/directives/obj_for_update.js.coffee | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/assets/javascripts/admin/index_utils/directives/obj_for_update.js.coffee b/app/assets/javascripts/admin/index_utils/directives/obj_for_update.js.coffee index f170d92437..3ed4e6c98f 100644 --- a/app/assets/javascripts/admin/index_utils/directives/obj_for_update.js.coffee +++ b/app/assets/javascripts/admin/index_utils/directives/obj_for_update.js.coffee @@ -12,8 +12,7 @@ angular.module("admin.indexUtils").directive "objForUpdate", (switchClass, pendi scope.clear() else scope.pending() - value = if value? then value else "" - addPendingChange(scope.attr, value) + addPendingChange(scope.attr, value ? "") scope.reset = (value) -> scope.savedValue = value From 6f10907555536e3bbadddd9cab82fd18b5858a12 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 8 Aug 2023 15:37:05 +1000 Subject: [PATCH 6/6] Fix rubocop warnings --- spec/system/admin/customers_spec.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/spec/system/admin/customers_spec.rb b/spec/system/admin/customers_spec.rb index 22b6125732..f0c278b0fc 100644 --- a/spec/system/admin/customers_spec.rb +++ b/spec/system/admin/customers_spec.rb @@ -35,9 +35,11 @@ describe 'Customers' do it "passes the smoke test" do # Prompts for a hub for a list of my managed enterprises - expect(page) - .to have_select2 "shop_id", with_options: [managed_distributor1.name, - managed_distributor2.name], without_options: [unmanaged_distributor.name] + expect(page).to have_select2( + "shop_id", + with_options: [managed_distributor1.name, managed_distributor2.name], + without_options: [unmanaged_distributor.name] + ) select2_select managed_distributor2.name, from: "shop_id" @@ -191,7 +193,7 @@ describe 'Customers' do context "when filtering by code" do before do - customer4.update code: 12345 + customer4.update code: 12_345 select2_select managed_distributor1.name, from: "shop_id"