Skip to content

[DEV ONLY] Weedy - sprint16 #143810729 ui custom reason waiting #1

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.

Sign up for GitHub

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

Open
patmbolger wants to merge 9 commits into
base: sprint16-#143810729-UI-custom-reason-waiting
Choose a base branch
from
Open

[DEV ONLY] Weedy - sprint16 #143810729 ui custom reason waiting #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
75f5e10
interim progress
patmbolger Jun 24, 2017
08ae9ba
refactored some JS
patmbolger Jun 24, 2017
bc1d1be
changed feature steps in accordance with id changes in view
patmbolger Jun 24, 2017
aa7f58b
removed selenium webdriver from Gemfile
patmbolger Jun 24, 2017
2bf19ed
fixed a couple cuke steps
patmbolger Jun 24, 2017
560b1ca
fixed remaining cuke tests
patmbolger Jun 25, 2017
d300dde
changed JS code, converted back to poltergeist from selenium
patmbolger Jun 26, 2017
4dd87d2
removed selenium
patmbolger Jun 27, 2017
31a1bdb
more JS refactor; remove wait from cuke test
patmbolger Jun 27, 2017
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
74 changes: 36 additions & 38 deletions app/views/membership_applications/_reason_waiting.html.haml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,43 +2,42 @@
Do not make user press a "save" or "submit" button for these changes. (use AJAX)


= form_for @membership_application, html: { id: 'member-app-waiting-reasons', multipart: true}, url: membership_application_path do |f|
.row
.container
.reason-waiting-information
= f.label( :member_app_waiting_reasons, t('membership_applications.need_info.reason_title'))
= f.collection_select(:member_app_waiting_reasons_id,
AdminOnly::MemberAppWaitingReason.all,
:id,
reason_name_method,
{ include_blank: t('membership_applications.need_info.select_a_reason') },
{ class: 'reason-waiting-list',
onchange: 'changed_reason()' },
)

.row
.container
#other-text-field
= f.label( :reason_waiting_custom_text, t('membership_applications.need_info.other_reason_label') )
= f.text_field :custom_reason_text, onchange: 'changed_custom_text()'
.row
.container
.reason-waiting-information
= label_tag :member_app_waiting_reasons, t('membership_applications.need_info.reason_title')
- collection = AdminOnly::MemberAppWaitingReason.all.to_a
- collection << AdminOnly::MemberAppWaitingReason.new(id: -1, "name_#{I18n.locale.to_s}": "#{@other_waiting_reason_text}")
- selected = ! @membership_application.custom_reason_text.blank? ? -1 : @membership_application.member_app_waiting_reasons_id
= select_tag(:member_app_waiting_reasons,
options_from_collection_for_select(collection, :id, reason_name_method,
selected),
{ include_blank: t('membership_applications.need_info.select_a_reason'),
class: 'reason-waiting-list' })

.row
.container
#other-text-field
= label_tag :custom_reason_text, t('membership_applications.need_info.other_reason_label')
= text_field_tag :custom_reason_text, @membership_application.custom_reason_text
Copy link
Copy Markdown
Author

@patmbolger patmbolger Jun 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the form as it was not needed and also caused bugs. (in your feature branch, enter a custom reason and hit "enter" key - you will see an error alert - this is because after the AJAX stuff happens, the form is also submitted to the controller action (not what we want).

I added the "custom reason" item to the select_tag collection. I hacked it up here but it should probably be returned from a helper method. (also, note the ugly way that I am finding the current selected value for the select collection. This can be removed when we refactor the custom reason into the AdminOnly::MemberAppWaitingReason model).

I also added the currently-selected option to the options collection.

I refactored the "onchange" stuff to the DOM-ready function.

I also simplified the ID for the select list and the text_field. (this make for easier read of the code, IMO)



:javascript

let reasons_list = document.getElementById('membership_application_member_app_waiting_reasons_id'); // the select HTML element (list)
let custom_text_info = $('#other-text-field');
let custom_text_field = $('#membership_application_custom_reason_text');
var reasons_list = $('#member_app_waiting_reasons'); // the select HTML element (list)
var custom_text_info = $('#other-text-field');
var custom_text_field = $('#custom_reason_text');

// this option is added to the list of reasons and is only used so we can show/hide the custom reason text field
let other_reason_text = "#{@other_waiting_reason_text}";
let other_reason_option = document.createElement("option");
var other_reason_text = "#{@other_waiting_reason_text}";
var other_reason_option = document.createElement("option");
other_reason_option.text = other_reason_text;
other_reason_option.value = -1; // some value that will not be in the database or use by Rails



function selected_is_customOtherReason() {
return reasons_list.value === other_reason_option.value;
return $('#member_app_waiting_reasons option:selected').text() === other_reason_option.text;
Copy link
Copy Markdown
Author

@patmbolger patmbolger Jun 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your statement did not work for me - it could be because I converted to query(?).

}


Expand All @@ -49,7 +48,7 @@

if (!selected_is_customOtherReason()) {
custom_text_field.val('');
send_updated_reason( #{@membership_application.id}, $('#membership_application_member_app_waiting_reasons_id').find('option:selected').val() , null );
send_updated_reason( #{@membership_application.id}, $('#member_app_waiting_reasons').find('option:selected').val() , null );
}
hideOrShowCustomReasonElements();

Expand All @@ -63,15 +62,15 @@
}


let hideOrShowCustomReasonElements = function() {
var hideOrShowCustomReasonElements = function() {

if ( custom_text_field.val() || selected_is_customOtherReason()) {
custom_text_info.show();
reasons_list.value = other_reason_option.value; // make sure this option is selected; important when first displaying the view
}
else {
// clear out any custom reason if the selected reason is not the custom reason
document.getElementById("membership_application_custom_reason_text").value = "";
$('#custom_reason_text').val("");
custom_text_info.hide();
}

Expand All @@ -80,25 +79,24 @@

// Send information to the server
// no need to do anything if it was successful
let send_updated_reason = function(app_id, reason_id, custom_text) {
var send_updated_reason = function(app_id, reason_id, custom_text) {

$.ajax({
url: "#{membership_application_path}",
type: "PUT",
data: { membership_application: { member_app_waiting_reasons_id: reason_id,
custom_reason_text: custom_text },
id: app_id }
}).fail(function(evt, xhr, status, err) {
alert( "#{I18n.t('membership_applications.update.error')}: " + 'Status: ' + evt.statusText );
id: app_id },
error: function(xhr, status, err) {
alert( "#{I18n.t('membership_applications.update.error')}: " + 'Status: ' + status);
}
});

};


let initialize = (function() {
reasons_list.options.add(other_reason_option); // add the other_reason to the list
$(function() {
// reasons_list.options.add(other_reason_option); // add the other_reason to the list
hideOrShowCustomReasonElements();
$('#member_app_waiting_reasons').on('change', changed_reason);
$('#custom_reason_text').on('change', changed_custom_text)
});

$(document).ready(initialize);

60 changes: 30 additions & 30 deletions db/schema.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,38 +15,38 @@
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"

create_table "addresses", force: :cascade do |t|
t.string "street_address"
t.string "post_code"
t.string "city"
t.string "country", default: "Sverige", null: false
t.integer "region_id"
t.string "addressable_type"
t.integer "addressable_id"
t.integer "kommun_id"
t.float "latitude"
t.float "longitude"
create_table "addresses", id: :bigserial, force: :cascade do |t|
t.string "street_address"
t.string "post_code"
t.string "city"
t.string "country", default: "Sverige", null: false
t.bigint "region_id"
t.string "addressable_type"
t.bigint "addressable_id"
t.bigint "kommun_id"
t.float "latitude"
t.float "longitude"
t.index ["addressable_type", "addressable_id"], name: "index_addresses_on_addressable_type_and_addressable_id", using: :btree
t.index ["kommun_id"], name: "index_addresses_on_kommun_id", using: :btree
t.index ["latitude", "longitude"], name: "index_addresses_on_latitude_and_longitude", using: :btree
t.index ["region_id"], name: "index_addresses_on_region_id", using: :btree
end

create_table "business_categories", force: :cascade do |t|
create_table "business_categories", id: :bigserial, force: :cascade do |t|
t.string "name"
t.string "description"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
end

create_table "business_categories_membership_applications", force: :cascade do |t|
t.integer "membership_application_id"
t.integer "business_category_id"
create_table "business_categories_membership_applications", id: :bigserial, force: :cascade do |t|
t.bigint "membership_application_id"
t.bigint "business_category_id"
t.index ["business_category_id"], name: "index_on_categories", using: :btree
t.index ["membership_application_id"], name: "index_on_applications", using: :btree
end

create_table "ckeditor_assets", force: :cascade do |t|
create_table "ckeditor_assets", id: :bigserial, force: :cascade do |t|
t.string "data_file_name", null: false
t.string "data_content_type"
t.integer "data_file_size"
Expand All @@ -56,12 +56,12 @@
t.integer "height"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.integer "company_id"
t.bigint "company_id"
t.index ["company_id"], name: "index_ckeditor_assets_on_company_id", using: :btree
t.index ["type"], name: "index_ckeditor_assets_on_type", using: :btree
end

create_table "companies", force: :cascade do |t|
create_table "companies", id: :bigserial, force: :cascade do |t|
t.string "name"
t.string "company_number"
t.string "phone_number"
Expand All @@ -74,13 +74,13 @@
t.index ["company_number"], name: "index_companies_on_company_number", unique: true, using: :btree
end

create_table "kommuns", force: :cascade do |t|
create_table "kommuns", id: :bigserial, force: :cascade do |t|
t.string "name"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
end

create_table "member_app_waiting_reasons", force: :cascade, comment: "reasons why SHF is waiting for more info from applicant. Add more columns when more locales needed." do |t|
create_table "member_app_waiting_reasons", id: :bigserial, force: :cascade, comment: "reasons why SHF is waiting for more info from applicant. Add more columns when more locales needed." do |t|
t.string "name_sv", comment: "name of the reason in svenska/Swedish"
t.string "description_sv", comment: "description for the reason in svenska/Swedish"
t.string "name_en", comment: "name of the reason in engelsk/English"
Expand All @@ -90,23 +90,23 @@
t.datetime "updated_at", null: false
end

create_table "member_pages", force: :cascade do |t|
create_table "member_pages", id: :bigserial, force: :cascade do |t|
t.string "filename", null: false
t.string "title"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
end

create_table "membership_applications", force: :cascade do |t|
create_table "membership_applications", id: :bigserial, force: :cascade do |t|
t.string "company_number"
t.string "phone_number"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.integer "user_id"
t.bigint "user_id"
t.string "first_name"
t.string "last_name"
t.string "contact_email"
t.integer "company_id"
t.bigint "company_id"
t.string "membership_number"
t.string "state", default: "new"
t.integer "member_app_waiting_reasons_id"
Expand All @@ -116,15 +116,15 @@
t.index ["user_id"], name: "index_membership_applications_on_user_id", using: :btree
end

create_table "regions", force: :cascade do |t|
create_table "regions", id: :bigserial, force: :cascade do |t|
t.string "name"
t.string "code"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
end

create_table "shf_documents", force: :cascade do |t|
t.integer "uploader_id", null: false
create_table "shf_documents", id: :bigserial, force: :cascade do |t|
t.bigint "uploader_id", null: false
t.string "title"
t.text "description"
t.datetime "created_at", null: false
Expand All @@ -136,18 +136,18 @@
t.index ["uploader_id"], name: "index_shf_documents_on_uploader_id", using: :btree
end

create_table "uploaded_files", force: :cascade do |t|
create_table "uploaded_files", id: :bigserial, force: :cascade do |t|
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.string "actual_file_file_name"
t.string "actual_file_content_type"
t.integer "actual_file_file_size"
t.datetime "actual_file_updated_at"
t.integer "membership_application_id"
t.bigint "membership_application_id"
t.index ["membership_application_id"], name: "index_uploaded_files_on_membership_application_id", using: :btree
end

create_table "users", force: :cascade do |t|
create_table "users", id: :bigserial, force: :cascade do |t|
t.string "email", default: "", null: false
t.string "encrypted_password", default: "", null: false
t.string "reset_password_token"
Copy link
Copy Markdown
Author

@patmbolger patmbolger Jun 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how these differences occurred (did not change any migration file). Hopefully they are innocuous ...

Expand Down
6 changes: 3 additions & 3 deletions features/membership-application-status/admin-custom-reason-waiting.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Feature: "Other/Custom" waiting reason comes from locale file and Admin cannot e
Background:

# it is important that this statement is first so that tables are empty, so that things will be seeded
Given the system is seeded with initial data
# Given the system is seeded with initial data
Copy link
Copy Markdown
Author

@patmbolger patmbolger Jun 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we now add the "custom reason" selection directly (no longer from the DB) this background step is not needed.



Given the following users exists
Expand Down Expand Up @@ -55,10 +55,10 @@ Feature: "Other/Custom" waiting reason comes from locale file and Admin cannot e
And I am logged in as "admin@shf.se"


@admin
@admin @javascript
Copy link
Copy Markdown
Author

@patmbolger patmbolger Jun 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't see how the tag @admin also pulled in poltergeist, so added that explicitly.

Scenario: The "other/custom" reason is listed as a reason for the 'waiting for...' status
Given I am on "AnnaWaiting" application page
Then "membership_application_member_app_waiting_reasons_id" should have t("admin_only.member_app_waiting_reasons.other_custom_reason") as an option
Then "member_app_waiting_reasons" should have t("admin_only.member_app_waiting_reasons.other_custom_reason") as an option


@admin
Expand Down
42 changes: 20 additions & 22 deletions features/membership-application-status/admin-sets-custom-waiting-reason.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ Feature: Admin sets or enters the reason they are waiting for info from a user

And the following member app waiting reasons exist
| name_sv | description_sv | name_en | description_en | is_custom |
| Annat (skriv in orsaken) | Annat | Other (enter the reason) | other | false |
| need doc | need doc | need documentation | need more documents proving qualifications | false |
| waiting for payment | still waiting for payment | waiting for payment | still waiting for payment | false |

Expand All @@ -46,56 +45,56 @@ Feature: Admin sets or enters the reason they are waiting for info from a user
@javascript
Scenario: Admin selects 'need more documentation' as the reason SHF is waiting_for_applicant
Given I am on "AnnaWaiting" application page
When I set "membership_application_member_app_waiting_reasons_id" to "need doc"
When I set "member_app_waiting_reasons" to "need doc"
Then "member_app_waiting_reasons" should have "need doc" selected
And I am on the list applications page
And I am on "AnnaWaiting" application page
Then "membership_application_member_app_waiting_reasons_id" should have "need doc" selected
And I should not see t("admin_only.member_app_waiting_reasons.other_custom_reason")

Then "member_app_waiting_reasons" should have "need doc" selected

@javascript
Scenario: Admin selects 'waiting for payment' as the reason SHF is waiting_for_applicant
Given I am on "AnnaWaiting" application page
When I set "membership_application_member_app_waiting_reasons_id" to "waiting for payment"
When I set "member_app_waiting_reasons" to "waiting for payment"
And I am on the list applications page
And I am on "AnnaWaiting" application page
And "membership_application_member_app_waiting_reasons_id" should have "waiting for payment" selected
And I should not see t("admin_only.member_app_waiting_reasons.other_custom_reason")
And "member_app_waiting_reasons" should have "waiting for payment" selected


@javascript
Scenario: Admin selects 'other' and enters text as the reason SHF is waiting_for_applicant
Given I am on "AnnaWaiting" application page
When I set "membership_application_member_app_waiting_reasons_id" to t("admin_only.member_app_waiting_reasons.other_custom_reason")
And I fill in "membership_application_custom_reason_text" with "This is my reason"
When I set "member_app_waiting_reasons" to t("admin_only.member_app_waiting_reasons.other_custom_reason")
And I fill in "custom_reason_text" with "This is my reason"
And I press enter in "custom_reason_text"
And I am on the list applications page
And I am on "AnnaWaiting" application page
#And item t("admin_only.member_app_waiting_reasons.other_custom_reason") should be visible
And I should see t("membership_applications.need_info.other_reason_label")
And the "membership_application_custom_reason_text" field should be set to "This is my reason"
And "membership_application_member_app_waiting_reasons_id" should have t("admin_only.member_app_waiting_reasons.other_custom_reason") selected
And the "custom_reason_text" field should be set to "This is my reason"
And "member_app_waiting_reasons" should have t("admin_only.member_app_waiting_reasons.other_custom_reason") selected


@javascript
Scenario: Admin selects 'other' and fills in custom text but then changes reason to something else
Given I am on "AnnaWaiting" application page
When I set "membership_application_member_app_waiting_reasons_id" to t("admin_only.member_app_waiting_reasons.other_custom_reason")
And I fill in "membership_application_custom_reason_text" with "This is my reason"
And I set "membership_application_member_app_waiting_reasons_id" to "waiting for payment"
When I set "member_app_waiting_reasons" to t("admin_only.member_app_waiting_reasons.other_custom_reason")
And I fill in "custom_reason_text" with "This is my reason"
And I press enter in "custom_reason_text"
And I set "member_app_waiting_reasons" to "waiting for payment"
And I am on the list applications page
And I am on "AnnaWaiting" application page
And "membership_application_member_app_waiting_reasons_id" should have "waiting for payment" selected
And I should not see t("admin_only.member_app_waiting_reasons.other_custom_reason")
And "member_app_waiting_reasons" should have "waiting for payment" selected


@javascript
Scenario: When selected reason is not 'custom other,' the custom text is saved as blank (empty string)
Given I am on "AnnaWaiting" application page
When I set "membership_application_member_app_waiting_reasons_id" to t("admin_only.member_app_waiting_reasons.other_custom_reason")
And I fill in "membership_application_custom_reason_text" with "This is my reason"
And I set "membership_application_member_app_waiting_reasons_id" to "need doc"
When I set "member_app_waiting_reasons" to t("admin_only.member_app_waiting_reasons.other_custom_reason")
And I fill in "custom_reason_text" with "This is my reason"
And I press enter in "custom_reason_text"
And I set "member_app_waiting_reasons" to "need doc"
# change back so the custom reason field shows. it should be blank
And I set "membership_application_member_app_waiting_reasons_id" to t("admin_only.member_app_waiting_reasons.other_custom_reason")
And I set "member_app_waiting_reasons" to t("admin_only.member_app_waiting_reasons.other_custom_reason")
Then I should not see "This is my reason"


Expand All @@ -104,4 +103,3 @@ Feature: Admin sets or enters the reason they are waiting for info from a user
Given I am logged in as "anna_waiting_for_info@nosnarkybarky.se"
And I am on the application page for "AnnaWaiting"
Then I should not see t("membership_applications.need_info.reason_title")

Loading