From 6b2dd5a4433a83f34a258dd242d9264d4ee810a3 Mon Sep 17 00:00:00 2001 From: merefield Date: Thu, 6 Apr 2023 17:04:01 +0100 Subject: [PATCH 1/5] FIX: users not being added to group as part of create action --- lib/custom_wizard/action.rb | 6 +++++- spec/components/custom_wizard/action_spec.rb | 5 +++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/custom_wizard/action.rb b/lib/custom_wizard/action.rb index 34f81455..eb614076 100644 --- a/lib/custom_wizard/action.rb +++ b/lib/custom_wizard/action.rb @@ -460,7 +460,11 @@ class CustomWizard::Action user_ids.each { |user_id| group.group_users.build(user_id: user_id) } end - log_success("Group created", group.name) + if group.save + log_success("Group created", group.name) + else + log_error("Group users creation failed", group.errors.messages) + end result.output = group.name else diff --git a/spec/components/custom_wizard/action_spec.rb b/spec/components/custom_wizard/action_spec.rb index 79c64520..d2fde3df 100644 --- a/spec/components/custom_wizard/action_spec.rb +++ b/spec/components/custom_wizard/action_spec.rb @@ -2,6 +2,7 @@ describe CustomWizard::Action do fab!(:user) { Fabricate(:user, name: "Angus", username: 'angus', email: "angus@email.com", trust_level: TrustLevel[2]) } + fab!(:user1) { Fabricate(:user, name: "Angus One", username: 'angus1', email: "angus_one@email.com", trust_level: TrustLevel[2]) } fab!(:category) { Fabricate(:category, name: 'cat1', slug: 'cat-slug') } fab!(:tag) { Fabricate(:tag, name: 'tag1') } fab!(:group) { Fabricate(:group) } @@ -350,7 +351,11 @@ describe CustomWizard::Action do wizard = CustomWizard::Builder.new(@template[:id], user).build wizard.create_updater(wizard.steps[0].id, step_1_field_1: "Text input").update + group_id = Group.where(name: wizard.current_submission.fields['action_9']).first.id + user_id = User.find_by(username: wizard_template['actions'][4]['usernames'][0]["output"][0]).id + expect(Group.where(name: wizard.current_submission.fields['action_9']).exists?).to eq(true) + expect(GroupUser.where(group_id: group_id, user_id: user_id).exists?).to eq(true) end it '#add_to_group' do From 766cae92bad45c6d04d59ef79917674407e7d241 Mon Sep 17 00:00:00 2001 From: merefield Date: Thu, 6 Apr 2023 17:05:06 +0100 Subject: [PATCH 2/5] bump patch --- plugin.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin.rb b/plugin.rb index 3a65d315..ab596540 100644 --- a/plugin.rb +++ b/plugin.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # name: discourse-custom-wizard # about: Forms for Discourse. Better onboarding, structured posting, data enrichment, automated actions and much more. -# version: 2.3.0 +# version: 2.3.1 # authors: Angus McLeod, Faizaan Gagan, Robert Barrow, Keegan George, Kaitlin Maddever # url: https://github.com/paviliondev/discourse-custom-wizard # contact_emails: development@pavilion.tech From 37cef2ccc261b3f254b4a12fe3227ab5040652dd Mon Sep 17 00:00:00 2001 From: merefield Date: Tue, 11 Apr 2023 09:54:37 +0100 Subject: [PATCH 3/5] IMPROVE: warn in logs when at least one user in wizard did not exist --- lib/custom_wizard/action.rb | 5 +- spec/components/custom_wizard/action_spec.rb | 15 +++ .../actions/create_group_bad_user.json | 104 ++++++++++++++++++ 3 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 spec/fixtures/actions/create_group_bad_user.json diff --git a/lib/custom_wizard/action.rb b/lib/custom_wizard/action.rb index eb614076..ac0799f3 100644 --- a/lib/custom_wizard/action.rb +++ b/lib/custom_wizard/action.rb @@ -456,14 +456,15 @@ class CustomWizard::Action if new_group_params[:usernames].present? user_ids = get_user_ids(new_group_params[:usernames]) + if user_ids.count < new_group_params[:usernames].count + log_error("Warning, group creation: some users were not found!") + end user_ids -= owner_ids if owner_ids user_ids.each { |user_id| group.group_users.build(user_id: user_id) } end if group.save log_success("Group created", group.name) - else - log_error("Group users creation failed", group.errors.messages) end result.output = group.name diff --git a/spec/components/custom_wizard/action_spec.rb b/spec/components/custom_wizard/action_spec.rb index d2fde3df..9a89e5b4 100644 --- a/spec/components/custom_wizard/action_spec.rb +++ b/spec/components/custom_wizard/action_spec.rb @@ -13,6 +13,7 @@ describe CustomWizard::Action do let(:watch_categories) { get_wizard_fixture("actions/watch_categories") } let(:watch_tags) { get_wizard_fixture("actions/watch_tags") } let(:create_group) { get_wizard_fixture("actions/create_group") } + let(:create_group_with_nonexistant_user) { get_wizard_fixture("actions/create_group_bad_user") } let(:add_to_group) { get_wizard_fixture("actions/add_to_group") } let(:send_message) { get_wizard_fixture("actions/send_message") } let(:send_message_multi) { get_wizard_fixture("actions/send_message_multi") } @@ -358,6 +359,20 @@ describe CustomWizard::Action do expect(GroupUser.where(group_id: group_id, user_id: user_id).exists?).to eq(true) end + it '#create_group completes successfully when user included in usernames does not exist but excludes users who do not exist and includes warning in log' do + wizard_template['actions'] << create_group_with_nonexistant_user + update_template(wizard_template) + + wizard = CustomWizard::Builder.new(@template[:id], user).build + wizard.create_updater(wizard.steps[0].id, step_1_field_1: "Text input").update + + group_id = Group.where(name: wizard.current_submission.fields['action_9']).first.id + + expect(CustomWizard::Log.list_query.all.last.value.include? "some users were not found").to eq(true) + expect(Group.where(name: wizard.current_submission.fields['action_9']).exists?).to eq(true) + expect(GroupUser.where(group_id: group_id).count).to eq(1) + end + it '#add_to_group' do wizard_template['actions'] << create_group wizard_template['actions'] << add_to_group diff --git a/spec/fixtures/actions/create_group_bad_user.json b/spec/fixtures/actions/create_group_bad_user.json new file mode 100644 index 00000000..96d6796d --- /dev/null +++ b/spec/fixtures/actions/create_group_bad_user.json @@ -0,0 +1,104 @@ +{ + "id": "action_9", + "run_after": "step_1", + "type": "create_group", + "title": [ + { + "type": "assignment", + "output": "New Group Member", + "output_type": "text", + "output_connector": "set" + } + ], + "custom_fields": [ + { + "type": "association", + "pairs": [ + { + "index": 0, + "key": "group_custom_field", + "key_type": "text", + "value": "step_3_field_1", + "value_type": "wizard_field", + "connector": "association" + } + ] + } + ], + "name": [ + { + "type": "assignment", + "output": "step_1_field_1", + "output_type": "wizard_field", + "output_connector": "set" + } + ], + "full_name": [ + { + "type": "assignment", + "output": "step_1_field_1", + "output_type": "wizard_field", + "output_connector": "set" + } + ], + "usernames": [ + { + "type": "assignment", + "output_type": "user", + "output_connector": "set", + "output": [ + "angus3" + ] + } + ], + "owner_usernames": [ + { + "type": "assignment", + "output_type": "user", + "output_connector": "set", + "output": [ + "angus" + ] + } + ], + "grant_trust_level": [ + { + "type": "assignment", + "output": "3", + "output_type": "text", + "output_connector": "set" + } + ], + "mentionable_level": [ + { + "type": "assignment", + "output": "1", + "output_type": "text", + "output_connector": "set" + } + ], + "messageable_level": [ + { + "type": "assignment", + "output": "2", + "output_type": "text", + "output_connector": "set" + } + ], + "visibility_level": [ + { + "type": "assignment", + "output": "3", + "output_type": "text", + "output_connector": "set" + } + ], + "members_visibility_level": [ + { + "type": "assignment", + "output": "99", + "output_type": "text", + "output_connector": "set" + } + ] +} \ No newline at end of file From f7cdc77a065d1636460198a3ca4a3bdad1833b6f Mon Sep 17 00:00:00 2001 From: merefield Date: Tue, 11 Apr 2023 09:59:22 +0100 Subject: [PATCH 4/5] fix spelling --- spec/components/custom_wizard/action_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/components/custom_wizard/action_spec.rb b/spec/components/custom_wizard/action_spec.rb index 9a89e5b4..a2690208 100644 --- a/spec/components/custom_wizard/action_spec.rb +++ b/spec/components/custom_wizard/action_spec.rb @@ -13,7 +13,7 @@ describe CustomWizard::Action do let(:watch_categories) { get_wizard_fixture("actions/watch_categories") } let(:watch_tags) { get_wizard_fixture("actions/watch_tags") } let(:create_group) { get_wizard_fixture("actions/create_group") } - let(:create_group_with_nonexistant_user) { get_wizard_fixture("actions/create_group_bad_user") } + let(:existentcreate_group_with_nonexistent_user) { get_wizard_fixture("actions/create_group_bad_user") } let(:add_to_group) { get_wizard_fixture("actions/add_to_group") } let(:send_message) { get_wizard_fixture("actions/send_message") } let(:send_message_multi) { get_wizard_fixture("actions/send_message_multi") } @@ -360,7 +360,7 @@ describe CustomWizard::Action do end it '#create_group completes successfully when user included in usernames does not exist but excludes users who do not exist and includes warning in log' do - wizard_template['actions'] << create_group_with_nonexistant_user + wizard_template['actions'] << existentcreate_group_with_nonexistent_user update_template(wizard_template) wizard = CustomWizard::Builder.new(@template[:id], user).build From 93b574beb794a549728ec0b54a35fca02b40c331 Mon Sep 17 00:00:00 2001 From: merefield Date: Tue, 11 Apr 2023 10:00:42 +0100 Subject: [PATCH 5/5] fix spelling --- spec/components/custom_wizard/action_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/components/custom_wizard/action_spec.rb b/spec/components/custom_wizard/action_spec.rb index a2690208..c450582d 100644 --- a/spec/components/custom_wizard/action_spec.rb +++ b/spec/components/custom_wizard/action_spec.rb @@ -13,7 +13,7 @@ describe CustomWizard::Action do let(:watch_categories) { get_wizard_fixture("actions/watch_categories") } let(:watch_tags) { get_wizard_fixture("actions/watch_tags") } let(:create_group) { get_wizard_fixture("actions/create_group") } - let(:existentcreate_group_with_nonexistent_user) { get_wizard_fixture("actions/create_group_bad_user") } + let(:create_group_with_nonexistent_user) { get_wizard_fixture("actions/create_group_bad_user") } let(:add_to_group) { get_wizard_fixture("actions/add_to_group") } let(:send_message) { get_wizard_fixture("actions/send_message") } let(:send_message_multi) { get_wizard_fixture("actions/send_message_multi") } @@ -360,7 +360,7 @@ describe CustomWizard::Action do end it '#create_group completes successfully when user included in usernames does not exist but excludes users who do not exist and includes warning in log' do - wizard_template['actions'] << existentcreate_group_with_nonexistent_user + wizard_template['actions'] << create_group_with_nonexistent_user update_template(wizard_template) wizard = CustomWizard::Builder.new(@template[:id], user).build