From 472b0453a9501fa23d11b869cd8c360d826c9d17 Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Wed, 9 Mar 2022 11:58:50 -0600 Subject: [PATCH 01/30] 12132: first pass at migration code, make_answers method working for one response set example --- app/models/answer.rb | 23 +++++++++++ db/migrate/20220305194344_create_answers.rb | 16 ++++++++ spec/factories/answer_factory.rb | 6 +++ spec/models/answer_spec.rb | 44 +++++++++++++++++++++ 4 files changed, 89 insertions(+) create mode 100644 app/models/answer.rb create mode 100644 db/migrate/20220305194344_create_answers.rb create mode 100644 spec/factories/answer_factory.rb create mode 100644 spec/models/answer_spec.rb diff --git a/app/models/answer.rb b/app/models/answer.rb new file mode 100644 index 000000000..91a4cf302 --- /dev/null +++ b/app/models/answer.rb @@ -0,0 +1,23 @@ +class Answer < ApplicationRecord + belongs_to :response_set + belongs_to :question + delegate :data_type, to: :question + + with_options if: ->(answer) { answer&.question&.data_type == "boolean" } do |boolean_answer| + boolean_answer.validates :boolean_data, presence: true + end + with_options if: ->(answer) { %w(rating range currency number percentage).include?(answer&.question&.data_type) } do |numeric_answer| + numeric_answer.validates :numeric_data, presence: true + end + with_options if: ->(answer) { %w(range text).include?(answer&.question&.data_type) } do |text_answer| + text_answer.validates :text_data, presence: true + end + with_options if: ->(answer) { %w(range text).include?(answer&.question&.data_type) } do |text_answer| + text_answer.validates :text_data, presence: true + end + validate :question_is_not_group + + def question_is_not_group + question.data_type != "group" + end +end diff --git a/db/migrate/20220305194344_create_answers.rb b/db/migrate/20220305194344_create_answers.rb new file mode 100644 index 000000000..8da1a7a52 --- /dev/null +++ b/db/migrate/20220305194344_create_answers.rb @@ -0,0 +1,16 @@ +class CreateAnswers < ActiveRecord::Migration[6.1] + def change + create_table :answers do |t| + t.references :response_set, index: true, foreign_key: true + t.references :question, index: true, foreign_key: true + t.boolean :not_applicable, null: false, default: false + t.string :text_data + t.string :numeric_data + t.boolean :boolean_data + t.json :linked_document_data + t.json :business_canvas_data + t.json :breakeven_data + t.timestamps + end + end +end diff --git a/spec/factories/answer_factory.rb b/spec/factories/answer_factory.rb new file mode 100644 index 000000000..6ba2acf1a --- /dev/null +++ b/spec/factories/answer_factory.rb @@ -0,0 +1,6 @@ +FactoryBot.define do + factory :answer do + response_set + question + end +end diff --git a/spec/models/answer_spec.rb b/spec/models/answer_spec.rb new file mode 100644 index 000000000..e7b4dcba9 --- /dev/null +++ b/spec/models/answer_spec.rb @@ -0,0 +1,44 @@ +require 'rails_helper' + +describe Answer, type: :model do + let!(:question_set) { create(:question_set)} + let!(:question) { create(:question, data_type: type) } + let!(:response_set) { create(:response_set, question_set: question_set) } + let(:text_data) { nil } + let(:numeric_data) { nil } + let(:boolean_data) { nil } + let(:breakeven_data) { nil } + let(:business_canvas_data) { nil } + let(:linked_document_data) { nil } + let(:answer) { + Answer.create({ + response_set: response_set, + question: question, + not_applicable: false, + text_data: text_data, + boolean_data: boolean_data, + breakeven_data: breakeven_data, + business_canvas_data: business_canvas_data, + linked_document_data: linked_document_data + }) + } + subject { answer } + + context "text_answer" do + let(:type) { "text"} + + context "has text_data" do + let(:text_data) { "test" } + it "is valid" do + expect(subject.valid?).to be_truthy + end + end + + context "has no text_data" do + let(:text_data) { nil } + it "is valid" do + expect(subject.valid?).to be_falsey + end + end + end +end From 8d055ca3f5eaaf1664de2fbd61344dc5fbba87a2 Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Mon, 18 Apr 2022 15:00:24 -0500 Subject: [PATCH 02/30] 12132: notes on how things currently work and changes to make --- app/models/response_set.rb | 32 +++++++++++++++++-- .../loans/questionnaires/_answer.html.slim | 3 ++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/app/models/response_set.rb b/app/models/response_set.rb index 50576e4fc..c02f5f205 100644 --- a/app/models/response_set.rb +++ b/app/models/response_set.rb @@ -26,12 +26,23 @@ def root_response # Ensures `question` is decorated before passing to Response. def response(question) question = ensure_decorated(question) + # TODO: replace raw_value with + # 1) lookup answer record based on question.id and self.id + # 2) call answer model method to compose json expected for raw_value raw_value = (custom_data || {})[question.json_key] Response.new(loan: loan, question: question, response_set: self, data: raw_value) end # Change/assign custom field value, but don't save. + # WHY do we not save here? probably just to save db writes + # THIS is where the question internal_name (e.g. field_110) coming form jqtree gets converted back to + # the question id that is the key in the response set's custom_data. + # so we can use the question.id and self.id to find an answer record. def set_response(question, value) + #TODO: find or create answer record by question id and self.id + # call a new method on answer that takes value and saves the fields + # i don't THINK we actually need to return custom data, but if we + # have to, we'll call a method on answer model that composes custom_data equivalent. self.custom_data ||= {} custom_data[question.json_key] = value custom_data @@ -55,13 +66,27 @@ def embedded_urls # def foo=(value) # set_response('foo', value) # end + # This method is used to save response_sets in the controler. They come + # back to the server with params that are internal names of questions e.g. "field_110=" + # Rails calls method_missing since these aren't attrs of a response set, + # and this method then calls response(q) and set_response(q) instead of erroring. + # it basically uses Rail's under the hood iteration over params from the request + # in lieu of writing our own. + # so far I am unclear where the get version happens . . . def method_missing(method_sym, *arguments, &block) attribute_name, action, field = match_dynamic_method(method_sym) if action + # the question is retrieved based on the internal name coming back + # from jqtree as the fake "attribute" prompting the method_missing call. in set_response, + # we then convert from internal name to the question id in set_response. q = question(attribute_name) case action - when :get then return response(q) - when :set then return set_response(q, arguments.first) + when :get + puts "GET in method missing with #{q}" + #return response(q) + when :set + puts "SET in method missing with #{q}" + return set_response(q, arguments.first) end end super @@ -101,6 +126,9 @@ def match_dynamic_method(method_sym) action = :get end + # the attribute name here is internal_name coming from the _answer.html.slim + # where the questionnaire form uses "question.attribute_sym" and attribute_sym is the internal name + # (see question.rb line 72) field = question(attribute_name, required: false) if field [attribute_name, action, field] diff --git a/app/views/admin/loans/questionnaires/_answer.html.slim b/app/views/admin/loans/questionnaires/_answer.html.slim index 67daaeaf5..7d64b7c16 100644 --- a/app/views/admin/loans/questionnaires/_answer.html.slim +++ b/app/views/admin/loans/questionnaires/_answer.html.slim @@ -10,6 +10,7 @@ / so that if the box is unchecked, the submitted value will be 'no'. This is a / standard Rails trick: / https://apidock.com/rails/ActionView/Helpers/FormHelper/check_box + / attribute_sym is the internal name = hidden_field_tag :"response_set[#{question.attribute_sym}][not_applicable]", "no" = check_box_tag :"response_set[#{question.attribute_sym}][not_applicable]", @@ -57,6 +58,8 @@ .form-element.answer.no-response class=(!response.text.present? ? "" : "hidden") = t("loan.no_answer") + / attribute_sym is the internal name + / does the [text] create something like nested attrs? = f.input_field :"#{question.attribute_sym}[text]", value: sanitize(response.text, tags: tags, attributes: attrs), as: :hidden, class: "rt-response" From 2f409ed777d179da81e4e36acbcbc99e2488b133 Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Mon, 18 Apr 2022 15:01:24 -0500 Subject: [PATCH 03/30] 12132: adds answer table in schema.rb --- db/schema.rb | 231 +++++++++++++++++++++++++++------------------------ 1 file changed, 124 insertions(+), 107 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index a93c4a672..5cd56825f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,111 +10,111 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2022_02_28_235520) do +ActiveRecord::Schema.define(version: 2022_03_05_194344) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" create_table "accounting_accounts", id: :serial, force: :cascade do |t| + t.datetime "created_at", null: false t.string "name", null: false t.string "qb_account_classification" t.string "qb_id", null: false t.jsonb "quickbooks_data" - t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["qb_id"], name: "index_accounting_accounts_on_qb_id" end create_table "accounting_customers", force: :cascade do |t| - t.string "qb_id", null: false + t.datetime "created_at", null: false t.string "name", null: false + t.string "qb_id", null: false t.jsonb "quickbooks_data" - t.datetime "created_at", null: false t.datetime "updated_at", null: false end create_table "accounting_line_items", id: :serial, force: :cascade do |t| - t.integer "qb_line_id" - t.integer "accounting_transaction_id", null: false t.integer "accounting_account_id", null: false - t.string "posting_type", null: false - t.string "description" + t.integer "accounting_transaction_id", null: false t.decimal "amount", null: false t.datetime "created_at", null: false + t.string "description" + t.string "posting_type", null: false + t.integer "qb_line_id" t.datetime "updated_at", null: false t.index ["accounting_account_id"], name: "index_accounting_line_items_on_accounting_account_id" t.index ["accounting_transaction_id"], name: "index_accounting_line_items_on_accounting_transaction_id" end create_table "accounting_qb_connections", id: :serial, force: :cascade do |t| + t.string "access_token", null: false + t.datetime "created_at", null: false t.integer "division_id", null: false + t.boolean "invalid_grant", default: false, null: false + t.datetime "last_updated_at" t.string "realm_id", null: false + t.string "refresh_token", null: false t.datetime "token_expires_at", null: false - t.datetime "last_updated_at" - t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.string "access_token", null: false - t.string "refresh_token", null: false - t.boolean "invalid_grant", default: false, null: false t.index ["division_id"], name: "index_accounting_qb_connections_on_division_id", unique: true end create_table "accounting_qb_departments", force: :cascade do |t| - t.string "qb_id", null: false + t.datetime "created_at", null: false + t.bigint "division_id" t.string "name", null: false + t.string "qb_id", null: false t.jsonb "quickbooks_data" - t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.bigint "division_id" t.index ["division_id"], name: "index_accounting_qb_departments_on_division_id" end create_table "accounting_qb_vendors", force: :cascade do |t| - t.string "qb_id", null: false + t.datetime "created_at", null: false t.string "name", null: false + t.string "qb_id", null: false t.jsonb "quickbooks_data" - t.datetime "created_at", null: false t.datetime "updated_at", null: false end create_table "accounting_sync_issues", force: :cascade do |t| - t.bigint "project_id" t.bigint "accounting_transaction_id" t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.string "message", null: false - t.string "level" t.jsonb "custom_data" + t.string "level" + t.string "message", null: false + t.bigint "project_id" + t.datetime "updated_at", null: false t.index ["accounting_transaction_id"], name: "index_plt_on_txn_id" t.index ["project_id"], name: "index_accounting_sync_issues_on_project_id" end create_table "accounting_transactions", id: :serial, force: :cascade do |t| - t.string "qb_id" - t.string "qb_object_type", default: "JournalEntry", null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false t.integer "accounting_account_id" - t.integer "project_id" - t.jsonb "quickbooks_data" + t.string "accounting_customer_id" t.decimal "amount" - t.date "txn_date" - t.decimal "total" - t.string "private_note" - t.string "description" - t.string "loan_transaction_type_value" + t.decimal "change_in_interest", precision: 15, scale: 2 + t.decimal "change_in_principal", precision: 15, scale: 2 + t.string "check_number" + t.datetime "created_at", null: false t.integer "currency_id" - t.decimal "principal_balance", default: "0.0" + t.string "description" + t.string "disbursement_type", default: "other" t.decimal "interest_balance", default: "0.0" - t.boolean "needs_qb_push", default: true, null: false + t.string "loan_transaction_type_value" t.boolean "managed", default: false, null: false - t.decimal "change_in_principal", precision: 15, scale: 2 - t.decimal "change_in_interest", precision: 15, scale: 2 - t.string "accounting_customer_id" - t.string "check_number" + t.boolean "needs_qb_push", default: true, null: false + t.decimal "principal_balance", default: "0.0" + t.string "private_note" + t.integer "project_id" + t.string "qb_id" + t.string "qb_object_type", default: "JournalEntry", null: false t.integer "qb_vendor_id" - t.string "disbursement_type", default: "other" + t.jsonb "quickbooks_data" t.string "sync_token" + t.decimal "total" + t.date "txn_date" + t.datetime "updated_at", null: false t.index ["accounting_account_id"], name: "index_accounting_transactions_on_accounting_account_id" t.index ["currency_id"], name: "index_accounting_transactions_on_currency_id" t.index ["project_id"], name: "index_accounting_transactions_on_project_id" @@ -123,6 +123,22 @@ t.index ["qb_object_type"], name: "index_accounting_transactions_on_qb_object_type" end + create_table "answers", force: :cascade do |t| + t.boolean "boolean_data" + t.json "breakeven_data" + t.json "business_canvas_data" + t.datetime "created_at", precision: 6, null: false + t.json "linked_document_data" + t.boolean "not_applicable", default: false, null: false + t.string "numeric_data" + t.bigint "question_id" + t.bigint "response_set_id" + t.string "text_data" + t.datetime "updated_at", precision: 6, null: false + t.index ["question_id"], name: "index_answers_on_question_id" + t.index ["response_set_id"], name: "index_answers_on_response_set_id" + end + create_table "countries", id: :serial, force: :cascade do |t| t.datetime "created_at", null: false t.integer "default_currency_id", null: false @@ -133,23 +149,23 @@ create_table "currencies", id: :serial, force: :cascade do |t| t.string "code" + t.string "country_code" t.datetime "created_at", null: false t.string "name" t.string "short_symbol" t.string "symbol" t.datetime "updated_at", null: false - t.string "country_code" end create_table "data_exports", force: :cascade do |t| + t.datetime "created_at", null: false + t.jsonb "data" t.bigint "division_id", null: false - t.string "name", null: false - t.datetime "start_date" t.datetime "end_date" t.string "locale_code", null: false - t.jsonb "data" + t.string "name", null: false + t.datetime "start_date" t.string "type", null: false - t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["division_id"], name: "index_data_exports_on_division_id" end @@ -167,36 +183,36 @@ t.string "accent_main_color" t.string "banner_bg_color" t.string "banner_fg_color" + t.date "closed_books_date" t.datetime "created_at", null: false t.integer "currency_id" t.jsonb "custom_data" t.text "description" + t.text "homepage" + t.integer "interest_income_account_id" + t.integer "interest_receivable_account_id" t.string "internal_name" + t.jsonb "locales" + t.string "logo" t.string "logo_content_type" t.string "logo_file_name" t.integer "logo_file_size" t.string "logo_text" t.datetime "logo_updated_at" + t.string "membership_status", default: "ally", null: false t.string "name" t.boolean "notify_on_new_logs", default: false t.integer "organization_id" t.integer "parent_id" - t.datetime "updated_at", null: false - t.jsonb "locales" t.integer "principal_account_id" - t.integer "interest_receivable_account_id" - t.integer "interest_income_account_id" t.boolean "public", default: false, null: false - t.string "short_name" - t.string "qb_parent_class_id" - t.date "closed_books_date" - t.boolean "qb_read_only", default: true, null: false - t.string "logo" - t.text "homepage" + t.string "public_accent_color" t.string "public_primary_color" t.string "public_secondary_color" - t.string "public_accent_color" - t.string "membership_status", default: "ally", null: false + t.string "qb_parent_class_id" + t.boolean "qb_read_only", default: true, null: false + t.string "short_name" + t.datetime "updated_at", null: false t.index ["currency_id"], name: "index_divisions_on_currency_id" t.index ["interest_income_account_id"], name: "index_divisions_on_interest_income_account_id" t.index ["interest_receivable_account_id"], name: "index_divisions_on_interest_receivable_account_id" @@ -206,50 +222,50 @@ end create_table "documentations", force: :cascade do |t| - t.string "html_identifier" - t.string "calling_controller" t.string "calling_action" + t.string "calling_controller" t.datetime "created_at", null: false - t.datetime "updated_at", null: false t.bigint "division_id" + t.string "html_identifier" + t.datetime "updated_at", null: false t.index ["division_id"], name: "index_documentations_on_division_id" t.index ["html_identifier"], name: "index_documentations_on_html_identifier", unique: true end create_table "loan_health_checks", id: :serial, force: :cascade do |t| + t.datetime "created_at", null: false + t.boolean "has_late_steps" + t.boolean "has_sporadic_updates" + t.date "last_log_date" t.integer "loan_id", null: false t.boolean "missing_contract" t.decimal "progress_pct" - t.date "last_log_date" - t.boolean "has_late_steps" - t.boolean "has_sporadic_updates" - t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["loan_id"], name: "index_loan_health_checks_on_loan_id" end create_table "loan_question_requirements", id: :serial, force: :cascade do |t| t.decimal "amount" - t.integer "question_id" t.integer "option_id" + t.integer "question_id" end create_table "media", id: :serial, force: :cascade do |t| t.datetime "created_at", null: false + t.boolean "featured", default: false, null: false t.string "item" t.string "item_content_type" t.integer "item_file_size" t.integer "item_height" t.integer "item_width" t.string "kind_value" + t.integer "legacy_id" + t.string "legacy_path" t.integer "media_attachable_id" t.string "media_attachable_type" t.integer "sort_order" t.datetime "updated_at", null: false t.integer "uploader_id" - t.boolean "featured", default: false, null: false - t.string "legacy_path" - t.integer "legacy_id" t.index ["media_attachable_type", "media_attachable_id"], name: "index_media_on_media_attachable_type_and_media_attachable_id" end @@ -285,17 +301,22 @@ create_table "organizations", id: :serial, force: :cascade do |t| t.string "alias" + t.string "census_tract_code" t.string "city" t.text "contact_notes" t.integer "country_id", null: false t.datetime "created_at", null: false t.jsonb "custom_data" + t.date "date_established" t.integer "division_id" t.string "email" + t.string "entity_structure", default: "not_selected", null: false t.string "fax" + t.string "inception_value" t.string "industry" t.string "last_name" t.string "legal_name" + t.string "naics_code" t.string "name" t.string "neighborhood" t.string "postal_code" @@ -309,11 +330,6 @@ t.string "tax_no" t.datetime "updated_at", null: false t.string "website" - t.string "inception_value" - t.string "entity_structure", default: "not_selected", null: false - t.date "date_established" - t.string "census_tract_code" - t.string "naics_code" t.index ["division_id"], name: "index_organizations_on_division_id" end @@ -329,6 +345,7 @@ t.string "first_name" t.boolean "has_system_access", default: false, null: false t.string "last_name" + t.integer "legacy_id" t.string "legal_name" t.string "name" t.string "neighborhood" @@ -341,7 +358,6 @@ t.string "tax_no" t.datetime "updated_at", null: false t.string "website" - t.integer "legacy_id" t.index ["division_id"], name: "index_people_on_division_id" end @@ -350,40 +366,40 @@ t.datetime "created_at", null: false t.date "date" t.date "date_changed_to" + t.integer "legacy_id" t.string "progress_metric_value" t.integer "project_step_id" t.datetime "updated_at", null: false - t.integer "legacy_id" t.index ["agent_id"], name: "index_project_logs_on_agent_id" t.index ["project_step_id"], name: "index_project_logs_on_project_step_id" end create_table "projects", id: :serial, force: :cascade do |t| + t.date "actual_end_date" + t.date "actual_first_payment_date" t.decimal "amount" t.datetime "created_at", null: false t.integer "currency_id" t.jsonb "custom_data" t.integer "division_id", null: false - t.date "actual_first_payment_date" + t.text "final_repayment_formula" t.integer "length_months" t.string "loan_type_value" t.string "name" t.integer "organization_id" + t.integer "original_id" t.integer "primary_agent_id" + t.date "projected_end_date" + t.date "projected_first_payment_date" t.string "public_level_value", null: false t.decimal "rate" t.integer "secondary_agent_id" t.date "signing_date" + t.string "source_of_capital", default: "shared", null: false t.string "status_value" - t.date "projected_end_date" - t.datetime "updated_at", null: false - t.string "type", null: false - t.integer "original_id" - t.date "projected_first_payment_date" - t.date "actual_end_date" t.string "txn_handling_mode", default: "automatic", null: false - t.text "final_repayment_formula" - t.string "source_of_capital", default: "shared", null: false + t.string "type", null: false + t.datetime "updated_at", null: false t.index ["currency_id"], name: "index_projects_on_currency_id" t.index ["division_id"], name: "index_projects_on_division_id" t.index ["organization_id"], name: "index_projects_on_organization_id" @@ -399,28 +415,28 @@ create_table "question_sets", id: :serial, force: :cascade do |t| t.datetime "created_at", null: false + t.bigint "division_id", null: false t.string "kind" t.datetime "updated_at", null: false - t.bigint "division_id", null: false t.index ["division_id"], name: "index_question_sets_on_division_id" end create_table "questions", id: :serial, force: :cascade do |t| + t.boolean "active", default: true, null: false t.datetime "created_at", null: false t.string "data_type", null: false + t.boolean "display_in_summary", default: false, null: false + t.integer "division_id", null: false t.boolean "has_embeddable_media", default: false, null: false t.string "internal_name" - t.integer "question_set_id" + t.integer "legacy_id" t.integer "migration_position" + t.integer "number" t.boolean "override_associations", default: false, null: false t.integer "parent_id" t.integer "position" + t.integer "question_set_id" t.datetime "updated_at", null: false - t.integer "number" - t.integer "division_id", null: false - t.boolean "display_in_summary", default: false, null: false - t.boolean "active", default: true, null: false - t.integer "legacy_id" t.index "question_set_id, ((parent_id IS NULL))", name: "index_questions_on_question_set_id_parent_id_IS_NULL", unique: true, where: "(parent_id IS NULL)", comment: "Ensures max one root per question set" t.index ["question_set_id"], name: "index_questions_on_question_set_id" end @@ -428,14 +444,13 @@ create_table "response_sets", id: :serial, force: :cascade do |t| t.datetime "created_at", null: false t.jsonb "custom_data" + t.integer "legacy_id" t.integer "loan_id", null: false - t.datetime "updated_at", null: false - t.integer "updater_id" t.integer "lock_version", default: 0, null: false t.bigint "question_set_id", null: false - t.integer "legacy_id" - t.date "response_date" - t.index ["loan_id", "question_set_id", "response_date"], name: "index_response_sets_on_loan_qs_date", unique: true + t.datetime "updated_at", null: false + t.integer "updater_id" + t.index ["loan_id", "question_set_id"], name: "index_response_sets_on_loan_id_and_question_set_id" t.index ["question_set_id"], name: "index_response_sets_on_question_set_id" end @@ -450,20 +465,20 @@ end create_table "tasks", force: :cascade do |t| - t.string "job_class", limit: 255, null: false - t.string "provider_job_id" - t.string "job_type_value", limit: 255, null: false + t.jsonb "activity_message_data" t.string "activity_message_value", limit: 65536, null: false + t.datetime "created_at", null: false + t.jsonb "custom_error_data" + t.string "job_class", limit: 255, null: false t.datetime "job_first_started_at" t.datetime "job_last_failed_at" t.datetime "job_succeeded_at" + t.string "job_type_value", limit: 255, null: false t.integer "num_attempts", default: 0, null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.jsonb "custom_error_data" - t.string "taskable_type" + t.string "provider_job_id" t.bigint "taskable_id" - t.jsonb "activity_message_data" + t.string "taskable_type" + t.datetime "updated_at", null: false t.index ["taskable_type", "taskable_id"], name: "index_tasks_on_taskable_type_and_taskable_id" end @@ -474,6 +489,7 @@ t.integer "date_change_count", default: 0, null: false t.datetime "finalized_at" t.boolean "is_finalized" + t.integer "legacy_id" t.integer "old_duration_days", default: 0 t.date "old_start_date" t.integer "parent_id" @@ -484,7 +500,6 @@ t.string "step_type_value", null: false t.string "type", null: false t.datetime "updated_at", null: false - t.integer "legacy_id" t.index ["agent_id"], name: "index_timeline_entries_on_agent_id" t.index ["project_id"], name: "index_timeline_entries_on_project_id" end @@ -498,6 +513,7 @@ end create_table "translations", id: :serial, force: :cascade do |t| + t.boolean "allow_html", default: false t.datetime "created_at" t.string "locale" t.text "text" @@ -505,7 +521,6 @@ t.integer "translatable_id" t.string "translatable_type" t.datetime "updated_at" - t.boolean "allow_html", default: false t.index ["translatable_type", "translatable_id"], name: "index_translations_on_translatable_type_and_translatable_id" end @@ -517,13 +532,13 @@ t.string "encrypted_password", default: "", null: false t.datetime "last_sign_in_at" t.inet "last_sign_in_ip" + t.string "notification_source", default: "home_only", null: false t.integer "profile_id" t.datetime "remember_created_at" t.datetime "reset_password_sent_at" t.string "reset_password_token" t.integer "sign_in_count", default: 0, null: false t.datetime "updated_at", null: false - t.string "notification_source", default: "home_only", null: false t.index ["email"], name: "index_users_on_email", unique: true t.index ["profile_id"], name: "index_users_on_profile_id" t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true @@ -544,6 +559,8 @@ add_foreign_key "accounting_transactions", "accounting_accounts" add_foreign_key "accounting_transactions", "currencies" add_foreign_key "accounting_transactions", "projects" + add_foreign_key "answers", "questions" + add_foreign_key "answers", "response_sets" add_foreign_key "countries", "currencies", column: "default_currency_id" add_foreign_key "data_exports", "divisions" add_foreign_key "divisions", "accounting_accounts", column: "interest_income_account_id" From 5a0c013049e1168b8b17d06529d565bdedc84a3c Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Mon, 18 Apr 2022 15:01:53 -0500 Subject: [PATCH 04/30] 12132: make answers method on response set --- app/models/response_set.rb | 39 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/app/models/response_set.rb b/app/models/response_set.rb index c02f5f205..b646fe53b 100644 --- a/app/models/response_set.rb +++ b/app/models/response_set.rb @@ -2,6 +2,7 @@ class ResponseSet < ApplicationRecord belongs_to :loan belongs_to :updater, class_name: 'User' belongs_to :question_set, inverse_of: :response_sets + has_many :answers, dependent: :destroy validates :loan, presence: true @@ -54,6 +55,44 @@ def embedded_urls custom_data.values.map { |v| v["url"].presence }.compact end + def json_blob + qset = self.question_set + + end + + def make_answers + custom_data.each do |q_id, response_data| + question = Question.find(q_id) + if question.present? && question.data_type != "group" + response = Response.new(loan: loan, question: question, + response_set: self, data: response_data) + text_data = response.text.present? ? response.text : nil + numeric_data = if response.has_number? + response.number + elsif response.has_rating? + response.rating + else + nil + end + boolean_data = response.has_boolean? ? (response.boolean == "yes") : nil + breakeven_data = response.has_breakeven? ? response.breakeven : nil + business_canvas_data = response.has_business_canvas? ? response.business_canvas : nil + linked_document_data = question.has_embeddable_media? ? {url: response.url, start_cell: response.start_cell, end_cell: response.end_cell} : nil + Answer.create({ + response_set: self, + question: question, + not_applicable: response.not_applicable?, + text_data: text_data, + numeric_data: numeric_data, + boolean_data: boolean_data, + breakeven_data: breakeven_data, + business_canvas_data: business_canvas_data, + linked_document_data: linked_document_data + }) + end + end + end + # Defines dynamic method handlers for custom fields as if they were natural attributes, including special # awareness of translatable custom fields. # From 27d45f8031fa553038be43a64989764fda933204 Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Mon, 18 Apr 2022 15:53:10 -0500 Subject: [PATCH 05/30] 12132: remove duplicate field in schema from merge, and add wait time on spec that was failing --- db/schema.rb | 1 - spec/system/admin/questions_flow_spec.rb | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index e39e1cbd2..5cd56825f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -448,7 +448,6 @@ t.integer "loan_id", null: false t.integer "lock_version", default: 0, null: false t.bigint "question_set_id", null: false - t.integer "legacy_id" t.datetime "updated_at", null: false t.integer "updater_id" t.index ["loan_id", "question_set_id"], name: "index_response_sets_on_loan_id_and_question_set_id" diff --git a/spec/system/admin/questions_flow_spec.rb b/spec/system/admin/questions_flow_spec.rb index 1ed503532..62481bc48 100644 --- a/spec/system/admin/questions_flow_spec.rb +++ b/spec/system/admin/questions_flow_spec.rb @@ -117,8 +117,8 @@ find(%([data-id="#{q113.id}"] > .jqtree-element .edit-action)).click fill_in("Title", with: "Stonks") click_button("Save") - - expect(page).to have_css(".jqtree-title", text: "Stonks", wait: 40) + + expect(page).to have_css(".jqtree-title", text: "Stonks", wait: 80) expect(page).not_to have_css(".jqtree-title", text: "q113") accept_confirm { find(%([data-id="#{q113.id}"] > .jqtree-element .delete-action)).click } From 92de0e3d0aa3f9abf6b21f27195e4dd0da3de5a0 Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Thu, 21 Apr 2022 13:18:57 -0500 Subject: [PATCH 06/30] WIP 12132: attempting specs on answers table to custom data and back conversion --- app/models/answer.rb | 25 +++++++++++++ app/models/question.rb | 4 +++ app/models/response_set.rb | 8 +++++ ...0220418205710_add_uniq_index_to_answers.rb | 5 +++ db/schema.rb | 3 +- spec/models/answer_spec.rb | 8 +++++ spec/models/response_set_spec.rb | 23 ++++++++++++ spec/system/admin/questionnaire_spec.rb | 35 +++++++++++++++++++ 8 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20220418205710_add_uniq_index_to_answers.rb create mode 100644 spec/models/response_set_spec.rb diff --git a/app/models/answer.rb b/app/models/answer.rb index 91a4cf302..781a47fea 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -20,4 +20,29 @@ class Answer < ApplicationRecord def question_is_not_group question.data_type != "group" end + + # return the value of json that would be in legacy custom_data field on response set for this answer's question + def custom_data_json + json = {} + json["not_applicable"] = self.not_applicable + if self.text_data.present? + json["text"] = self.text_data + end + if self.boolean_data.present? + json["boolean"] = self.boolean_data ? "yes" : "no" + end + if self.breakeven_data.present? + json["breakeven"] = self.breakeven_data + end + if self.business_canvas_data.present? + json["business_canvas"] = self.business_canvas_data + end + if self.numeric_data.present? + json["number"] = self.numeric_data + end + if self.linked_document_data.present? + json["linked_document"] = linked_document_data + end + return {"#{self.question.json_key}": json} + end end diff --git a/app/models/question.rb b/app/models/question.rb index 7207b4fc6..c07f6a736 100644 --- a/app/models/question.rb +++ b/app/models/question.rb @@ -52,6 +52,10 @@ def top_level? parent.root? end + def data_types + DATA_TYPES + end + # This is a duck type method, and is overridden by LoanFilteredQuestion, which implements # the real logic about which questions are required contextually. # We need to implement this here because plan Questions need to be serialized for the manage diff --git a/app/models/response_set.rb b/app/models/response_set.rb index b646fe53b..918828fe1 100644 --- a/app/models/response_set.rb +++ b/app/models/response_set.rb @@ -60,6 +60,14 @@ def json_blob end + def custom_data_from_answers + response_custom_data_json = {} + answers.each do |answer| + response_custom_data_json[answer.question.json_key] = answer.custom_data_json + end + return response_custom_data_json + end + def make_answers custom_data.each do |q_id, response_data| question = Question.find(q_id) diff --git a/db/migrate/20220418205710_add_uniq_index_to_answers.rb b/db/migrate/20220418205710_add_uniq_index_to_answers.rb new file mode 100644 index 000000000..bdc8e7386 --- /dev/null +++ b/db/migrate/20220418205710_add_uniq_index_to_answers.rb @@ -0,0 +1,5 @@ +class AddUniqIndexToAnswers < ActiveRecord::Migration[6.1] + def change + add_index :answers, %i[response_set_id question_id], unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 5cd56825f..950b75a4b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2022_03_05_194344) do +ActiveRecord::Schema.define(version: 2022_04_18_205710) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -136,6 +136,7 @@ t.string "text_data" t.datetime "updated_at", precision: 6, null: false t.index ["question_id"], name: "index_answers_on_question_id" + t.index ["response_set_id", "question_id"], name: "index_answers_on_response_set_id_and_question_id", unique: true t.index ["response_set_id"], name: "index_answers_on_response_set_id" end diff --git a/spec/models/answer_spec.rb b/spec/models/answer_spec.rb index e7b4dcba9..6f1d91a99 100644 --- a/spec/models/answer_spec.rb +++ b/spec/models/answer_spec.rb @@ -41,4 +41,12 @@ end end end + + context "migration code" do + context "one answer" do + + end + + + end end diff --git a/spec/models/response_set_spec.rb b/spec/models/response_set_spec.rb new file mode 100644 index 000000000..bc1e92ddc --- /dev/null +++ b/spec/models/response_set_spec.rb @@ -0,0 +1,23 @@ +require 'rails_helper' + +describe ResponseSet, js: true do + let(:division) { create(:division) } + let!(:question_set) { create(:question_set, :with_questions, kind: "loan_criteria", division: division)} + let(:user) { create_member(division) } + let(:loan) { create(:loan, division: division) } + + before do + login_as(user, scope: :user) + end + + + + def fill_and_save(type, value) + first(".edit-all").click + expect(page).to have_content("Now editing") + find("input[type=#{type}]").set(value) + find("body").click # Trigger blur event so that Save Changes button is properly clickable + click_button("Save Changes") + expect(page).not_to have_content("Now editing") + end +end diff --git a/spec/system/admin/questionnaire_spec.rb b/spec/system/admin/questionnaire_spec.rb index 77f48edbb..ec6e9bc6c 100644 --- a/spec/system/admin/questionnaire_spec.rb +++ b/spec/system/admin/questionnaire_spec.rb @@ -77,6 +77,41 @@ end end + context "refactoring to answers table" do + let!(:question_set) { create(:question_set, kind: "loan_criteria", division: division)} + let!(:root_q) {create(:question, data_type: "group", question_set: question_set, division: division)} + let!(:questions) { {} } + before do + Question::DATA_TYPES.each do |t| + questions[t] = Question.create(data_type: t, question_set: question_set, parent: root_q, division: division) + end + end + + scenario "custom data and answers table have same info" do + visit admin_loan_tab_path(loan, "questions") + first(".edit-all").click + save_and_open_page + #page.find('.select', wait: 10, visible: false) + select_el = page.find("#response_set_#{questions[:boolean].attribute_sym}\\[boolean\\]", wait: 10, visible: false) + #select_el.find(:xpath, 'option[1]').select_option + #page.select("Yes", from: "response_set_#{questions[:boolean].attribute_sym}\\[boolean\\]", visible: false) + find("#response_set_#{questions[:boolean].attribute_sym}\\[boolean\\] option[value='']").click + find("#response_set_#{questions[:boolean].attribute_sym}\\[boolean\\] option[value='yes']", visible: false).select_option + fill_qtype_with_value("number", "123") + fill_qtype_with_value("url", "www.example.com") + fill_qtype_with_value("start_cell", "A2") + fill_qtype_with_value("end_cell", "Z100") + click_button("Save Changes") + response_set = ResponseSet.find_by(question_set_id: question_set.id) + response_set.make_answers + expect(response_set.custom_data).to match_unordered_json(response_set.custom_data_from_answers) + end + end + + def fill_qtype_with_value(type, value) + find("input[type=#{type}]").set(value) + end + def fill_and_save(value) first(".edit-all").click expect(page).to have_content("Now editing") From 737a5dcb493b0f16942fb2519d97a8b9435e8173 Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Thu, 21 Apr 2022 16:16:28 -0500 Subject: [PATCH 07/30] 12132: saving form data to answers table, needs auto tests --- .../admin/response_sets_controller.rb | 1 + app/models/answer.rb | 25 +++++++++++++++++++ app/models/response_set.rb | 9 +++++++ 3 files changed, 35 insertions(+) diff --git a/app/controllers/admin/response_sets_controller.rb b/app/controllers/admin/response_sets_controller.rb index 580617acf..1f0ff9bb2 100644 --- a/app/controllers/admin/response_sets_controller.rb +++ b/app/controllers/admin/response_sets_controller.rb @@ -45,6 +45,7 @@ def update redirect_to display_path else @response_set.update!(adjusted_params) + @response_set.save_answers(adjusted_params) redirect_to display_path, notice: I18n.t(:notice_updated) end rescue ActiveRecord::StaleObjectError diff --git a/app/models/answer.rb b/app/models/answer.rb index 781a47fea..1fb53e631 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -17,6 +17,31 @@ class Answer < ApplicationRecord end validate :question_is_not_group + # this method is temporary for spr 2022 overhaul + def self.create_from_form_field_params(q_internal_name, fields, response_set) + q = Question.find_by(internal_name: q_internal_name) + not_applicable = fields.key?("not_applicable") ? (fields["not_applicable"] == "yes") : nil + text_data = fields.key?("text") ? fields["text"] : nil + numeric_data = fields.key?("number") ? fields["number"] : nil + boolean_data = field.key?("boolean") ? (fields["boolean"] == "yes") : nil + breakeven_data = fields.key?("breakeven") ? fields["breakeven"] : nil + business_canvas_data = fields.key?("business_canvas") ? fields["business_canvas"] : nil + linked_document_data = fields.key?("url") ? {"url": fields["url"] } : nil + linked_document_data["start_cell"] = fields["start_cell"] if fields.key("start_cell") + linked_document_data["end_cell"] = fields["end_cell"] if fields.key("end_cell") + Answer.create({ + response_set: response_set, + question: q, + not_applicable: not_applicable, + text_data: text_data, + numeric_data: numeric_data, + boolean_data: boolean_data, + breakeven_data: breakeven_data, + business_canvas_data: business_canvas_data, + linked_document_data: linked_document_data + }) + end + def question_is_not_group question.data_type != "group" end diff --git a/app/models/response_set.rb b/app/models/response_set.rb index 918828fe1..933c51479 100644 --- a/app/models/response_set.rb +++ b/app/models/response_set.rb @@ -34,6 +34,14 @@ def response(question) Response.new(loan: loan, question: question, response_set: self, data: raw_value) end + def save_answers(form_hash) + form_hash.each do |key, value| + if key.include?("field") # key is an internal_name of a question + Answer.create_from_form_field_params(key, value, self) + end + end + end + # Change/assign custom field value, but don't save. # WHY do we not save here? probably just to save db writes # THIS is where the question internal_name (e.g. field_110) coming form jqtree gets converted back to @@ -44,6 +52,7 @@ def set_response(question, value) # call a new method on answer that takes value and saves the fields # i don't THINK we actually need to return custom data, but if we # have to, we'll call a method on answer model that composes custom_data equivalent. + self.custom_data ||= {} custom_data[question.json_key] = value custom_data From cccc5d2926ccc46ce9a4de5358e521864793f323 Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Fri, 22 Apr 2022 12:01:14 -0500 Subject: [PATCH 08/30] 12132: adding answers and saving, reloading questionnaire for that loan, and editing answers works, using answers table, needs auto testing --- app/models/answer.rb | 58 ++++++++++--------- app/models/response_set.rb | 7 ++- ...ire_question_and_response_set_on_answer.rb | 10 ++++ ..._answers_for_response_set_and_questions.rb | 5 ++ db/schema.rb | 6 +- 5 files changed, 55 insertions(+), 31 deletions(-) create mode 100644 db/migrate/20220421220934_require_question_and_response_set_on_answer.rb create mode 100644 db/migrate/20220422165601_re_add_uniq_index_on_answers_for_response_set_and_questions.rb diff --git a/app/models/answer.rb b/app/models/answer.rb index 1fb53e631..a41d23665 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -1,37 +1,36 @@ class Answer < ApplicationRecord - belongs_to :response_set - belongs_to :question + belongs_to :response_set, optional: false + belongs_to :question, optional: false delegate :data_type, to: :question - with_options if: ->(answer) { answer&.question&.data_type == "boolean" } do |boolean_answer| - boolean_answer.validates :boolean_data, presence: true - end - with_options if: ->(answer) { %w(rating range currency number percentage).include?(answer&.question&.data_type) } do |numeric_answer| - numeric_answer.validates :numeric_data, presence: true - end - with_options if: ->(answer) { %w(range text).include?(answer&.question&.data_type) } do |text_answer| - text_answer.validates :text_data, presence: true - end - with_options if: ->(answer) { %w(range text).include?(answer&.question&.data_type) } do |text_answer| - text_answer.validates :text_data, presence: true - end + # with_options if: ->(answer) { answer&.question&.data_type == "boolean" } do |boolean_answer| + # boolean_answer.validates :boolean_data, presence: true + # end + # with_options if: ->(answer) { %w(rating range currency number percentage).include?(answer&.question&.data_type) } do |numeric_answer| + # numeric_answer.validates :numeric_data, presence: true + # end + # with_options if: ->(answer) { %w(range text).include?(answer&.question&.data_type) } do |text_answer| + # text_answer.validates :text_data, presence: true + # end + # with_options if: ->(answer) { %w(range text).include?(answer&.question&.data_type) } do |text_answer| + # text_answer.validates :text_data, presence: true + # end validate :question_is_not_group # this method is temporary for spr 2022 overhaul - def self.create_from_form_field_params(q_internal_name, fields, response_set) + def self.save_from_form_field_params(q_internal_name, fields, response_set) q = Question.find_by(internal_name: q_internal_name) not_applicable = fields.key?("not_applicable") ? (fields["not_applicable"] == "yes") : nil text_data = fields.key?("text") ? fields["text"] : nil numeric_data = fields.key?("number") ? fields["number"] : nil - boolean_data = field.key?("boolean") ? (fields["boolean"] == "yes") : nil + boolean_data = fields.key?("boolean") ? (fields["boolean"] == "yes") : nil breakeven_data = fields.key?("breakeven") ? fields["breakeven"] : nil business_canvas_data = fields.key?("business_canvas") ? fields["business_canvas"] : nil linked_document_data = fields.key?("url") ? {"url": fields["url"] } : nil - linked_document_data["start_cell"] = fields["start_cell"] if fields.key("start_cell") - linked_document_data["end_cell"] = fields["end_cell"] if fields.key("end_cell") - Answer.create({ - response_set: response_set, - question: q, + linked_document_data["start_cell"] = fields["start_cell"] if fields.key?("start_cell") + linked_document_data["end_cell"] = fields["end_cell"] if fields.key?("end_cell") + answer = Answer.find_or_create_by(response_set: response_set, question: q) + answer.update!({ not_applicable: not_applicable, text_data: text_data, numeric_data: numeric_data, @@ -46,10 +45,10 @@ def question_is_not_group question.data_type != "group" end - # return the value of json that would be in legacy custom_data field on response set for this answer's question - def custom_data_json + # temp method for spr 2022 overhaul + def raw_value json = {} - json["not_applicable"] = self.not_applicable + json["not_applicable"] = self.not_applicable ? "yes" : "no" if self.text_data.present? json["text"] = self.text_data end @@ -66,8 +65,15 @@ def custom_data_json json["number"] = self.numeric_data end if self.linked_document_data.present? - json["linked_document"] = linked_document_data + json["linked_document"] = self.linked_document_data end - return {"#{self.question.json_key}": json} + puts json + json + end + + # temp method for spr 2022 overhaul + # return the value of json that would be in legacy custom_data field on response set for this answer's question + def custom_data_json + return {"#{self.question.json_key}": self.raw_value} end end diff --git a/app/models/response_set.rb b/app/models/response_set.rb index 933c51479..d2a987e63 100644 --- a/app/models/response_set.rb +++ b/app/models/response_set.rb @@ -26,18 +26,21 @@ def root_response # Fetches a custom value from the json field. # Ensures `question` is decorated before passing to Response. def response(question) + puts "Answer for rs #{self.id} q #{question.id}" question = ensure_decorated(question) # TODO: replace raw_value with # 1) lookup answer record based on question.id and self.id # 2) call answer model method to compose json expected for raw_value - raw_value = (custom_data || {})[question.json_key] + answer = Answer.find_by(response_set: self, question: question) + raw_value = answer.present? ? answer.raw_value : nil + puts raw_value Response.new(loan: loan, question: question, response_set: self, data: raw_value) end def save_answers(form_hash) form_hash.each do |key, value| if key.include?("field") # key is an internal_name of a question - Answer.create_from_form_field_params(key, value, self) + Answer.save_from_form_field_params(key, value, self) end end end diff --git a/db/migrate/20220421220934_require_question_and_response_set_on_answer.rb b/db/migrate/20220421220934_require_question_and_response_set_on_answer.rb new file mode 100644 index 000000000..c0854a0f3 --- /dev/null +++ b/db/migrate/20220421220934_require_question_and_response_set_on_answer.rb @@ -0,0 +1,10 @@ +class RequireQuestionAndResponseSetOnAnswer < ActiveRecord::Migration[6.1] + def change + Answer.where(response_set_id: nil).delete_all + Answer.where(question_id: nil).delete_all + remove_reference :answers, :response_set, index: true, foreign_key: true + add_reference :answers, :response_set, index: true, foreign_key: true, null: false + remove_reference :answers, :question, index: true, foreign_key: true + add_reference :answers, :question, index: true, foreign_key: true, null: false + end +end diff --git a/db/migrate/20220422165601_re_add_uniq_index_on_answers_for_response_set_and_questions.rb b/db/migrate/20220422165601_re_add_uniq_index_on_answers_for_response_set_and_questions.rb new file mode 100644 index 000000000..66be87f2b --- /dev/null +++ b/db/migrate/20220422165601_re_add_uniq_index_on_answers_for_response_set_and_questions.rb @@ -0,0 +1,5 @@ +class ReAddUniqIndexOnAnswersForResponseSetAndQuestions < ActiveRecord::Migration[6.1] + def change + add_index :answers, %i[response_set_id question_id], unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 950b75a4b..cf2aeffe6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2022_04_18_205710) do +ActiveRecord::Schema.define(version: 2022_04_22_165601) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -131,8 +131,8 @@ t.json "linked_document_data" t.boolean "not_applicable", default: false, null: false t.string "numeric_data" - t.bigint "question_id" - t.bigint "response_set_id" + t.bigint "question_id", null: false + t.bigint "response_set_id", null: false t.string "text_data" t.datetime "updated_at", precision: 6, null: false t.index ["question_id"], name: "index_answers_on_question_id" From f779c1d46db41b861359a13dcbb1cc8156872a31 Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Mon, 25 Apr 2022 13:03:49 -0500 Subject: [PATCH 09/30] 12179: version with answer row for every question on a response set --- app/models/answer.rb | 29 +++++++++++++++++++++++------ app/models/response.rb | 2 +- app/models/response_set.rb | 14 ++++++++++++-- lib/tasks/one_time_changes.rake | 17 +++++++++++++++++ 4 files changed, 53 insertions(+), 9 deletions(-) diff --git a/app/models/answer.rb b/app/models/answer.rb index a41d23665..28d8bf98f 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -3,6 +3,8 @@ class Answer < ApplicationRecord belongs_to :question, optional: false delegate :data_type, to: :question + # TODO consider bringing back when frontend can support no answer existing + #========================================================================= # with_options if: ->(answer) { answer&.question&.data_type == "boolean" } do |boolean_answer| # boolean_answer.validates :boolean_data, presence: true # end @@ -17,18 +19,33 @@ class Answer < ApplicationRecord # end validate :question_is_not_group + # this method is temporary for spr 2022 overhaul + def compare_to_custom_data + custom_data_raw_data = response_set.custom_data[question.id.to_s] + custom_data_response = Response.new(loan: response_set.loan, question: question, response_set: response_set, data: custom_data_raw_data) + answer_response = Response.new(loan: response_set.loan, question: question, response_set: response_set, data: raw_value) + methods = [:loan, :question, :response_set, :text, :number, :boolean, :rating, :url, :start_cell, :end_cell, :owner, :breakeven, :business_canvas, :not_applicable] + methods.each do |m| + custom_data_value = custom_data_response.send(m) + answer_value = answer_response.send(m) + unless custom_data_value == answer_value + raise "ERROR for answer #{id}: for RS #{rs.id} custom data value for #{m} is #{custom_data_value} but is #{answer_value} for answer #{id}" + end + end + end + # this method is temporary for spr 2022 overhaul def self.save_from_form_field_params(q_internal_name, fields, response_set) q = Question.find_by(internal_name: q_internal_name) - not_applicable = fields.key?("not_applicable") ? (fields["not_applicable"] == "yes") : nil + not_applicable = fields.key?("not_applicable") ? (fields["not_applicable"] == "yes") : "no" text_data = fields.key?("text") ? fields["text"] : nil numeric_data = fields.key?("number") ? fields["number"] : nil - boolean_data = fields.key?("boolean") ? (fields["boolean"] == "yes") : nil + boolean_data = fields.key?("boolean") ? (fields["boolean"] == "yes") : "no" breakeven_data = fields.key?("breakeven") ? fields["breakeven"] : nil business_canvas_data = fields.key?("business_canvas") ? fields["business_canvas"] : nil - linked_document_data = fields.key?("url") ? {"url": fields["url"] } : nil - linked_document_data["start_cell"] = fields["start_cell"] if fields.key?("start_cell") - linked_document_data["end_cell"] = fields["end_cell"] if fields.key?("end_cell") + linked_document_data = fields.key?("url") ? {"url": fields["url"] } : {"url": ""} + linked_document_data["start_cell"] = fields.key?("start_cell") ? fields["start_cell"] : "" + linked_document_data["end_cell"] = fields.key?("end_cell") ? fields["end_cell"] : "" answer = Answer.find_or_create_by(response_set: response_set, question: q) answer.update!({ not_applicable: not_applicable, @@ -67,7 +84,7 @@ def raw_value if self.linked_document_data.present? json["linked_document"] = self.linked_document_data end - puts json + #puts json json end diff --git a/app/models/response.rb b/app/models/response.rb index 3563df225..5b76a516d 100644 --- a/app/models/response.rb +++ b/app/models/response.rb @@ -22,7 +22,7 @@ def initialize(loan:, question:, response_set:, data:) ALL_VALUE_TYPES.each do |type| instance_variable_set("@#{type}", data[type.to_sym]) end - @breakeven = remove_blanks(@breakeven) + @breakeven = (@breakeven) end def model_name diff --git a/app/models/response_set.rb b/app/models/response_set.rb index d2a987e63..328adaf1b 100644 --- a/app/models/response_set.rb +++ b/app/models/response_set.rb @@ -26,17 +26,27 @@ def root_response # Fetches a custom value from the json field. # Ensures `question` is decorated before passing to Response. def response(question) - puts "Answer for rs #{self.id} q #{question.id}" + #puts "Answer for rs #{self.id} q #{question.id}" question = ensure_decorated(question) # TODO: replace raw_value with # 1) lookup answer record based on question.id and self.id # 2) call answer model method to compose json expected for raw_value answer = Answer.find_by(response_set: self, question: question) raw_value = answer.present? ? answer.raw_value : nil - puts raw_value Response.new(loan: loan, question: question, response_set: self, data: raw_value) end + # for migration + def ensure_all_answers_copied + answer_q_ids = answers.pluck(:question_id).sort + custom_data_q_ids = custom_data.keys.map{|k| k.to_i}.sort + unless answer_q_ids == custom_data_q_ids + qs_in_answers_only = answer_q_ids - custom_data_q_ids + qs_in_custom_data_only = custom_data_q_ids - answer_q_ids + raise "ERROR for rs #{id}: Diff between questions in answers and custom data. In answer only: #{qs_in_answers_only}. In custom data only: #{qs_in_custom_data_only}" + end + end + def save_answers(form_hash) form_hash.each do |key, value| if key.include?("field") # key is an internal_name of a question diff --git a/lib/tasks/one_time_changes.rake b/lib/tasks/one_time_changes.rake index 696fb77cb..49017f43e 100644 --- a/lib/tasks/one_time_changes.rake +++ b/lib/tasks/one_time_changes.rake @@ -1,4 +1,21 @@ namespace :one_time_changes do + desc "A one time task to create Answers for all existing response sets" + task migrate_from_response_custom_data_to_answers: :environment do + ResponseSet.find_each do |rs| + begin + rs.make_answers + rs.ensure_all_answers_copied + rs.answers.each do |a| + a.compare_to_custom_data + end + rescue => e + puts e.message + Rails.logger.info e.message + # don't re-raise, keep going + end + end + end + desc "A one time task responding to Nov 2019 request to update loan date fields." task adjust_loan_dates: :environment do Loan.find_each do |l| From 0db9d7976fc64653e7468ed0c0b5c98fef0b695a Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Mon, 25 Apr 2022 16:50:15 -0500 Subject: [PATCH 10/30] 12179: one time changes rake task to make answers --- app/models/answer.rb | 95 ++++++++++++++++++++++++-------------- app/models/response_set.rb | 16 +++---- 2 files changed, 67 insertions(+), 44 deletions(-) diff --git a/app/models/answer.rb b/app/models/answer.rb index 28d8bf98f..2d6b49ef2 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -4,19 +4,22 @@ class Answer < ApplicationRecord delegate :data_type, to: :question # TODO consider bringing back when frontend can support no answer existing - #========================================================================= - # with_options if: ->(answer) { answer&.question&.data_type == "boolean" } do |boolean_answer| - # boolean_answer.validates :boolean_data, presence: true - # end - # with_options if: ->(answer) { %w(rating range currency number percentage).include?(answer&.question&.data_type) } do |numeric_answer| - # numeric_answer.validates :numeric_data, presence: true - # end - # with_options if: ->(answer) { %w(range text).include?(answer&.question&.data_type) } do |text_answer| - # text_answer.validates :text_data, presence: true - # end - # with_options if: ->(answer) { %w(range text).include?(answer&.question&.data_type) } do |text_answer| - # text_answer.validates :text_data, presence: true - # end + # ========================================================================= + with_options if: ->(answer) { answer&.question&.data_type == "boolean" } do |boolean_answer| + boolean_answer.validates :boolean_data, presence: true + end + with_options if: ->(answer) { %w(rating range currency number percentage).include?(answer&.question&.data_type) } do |numeric_answer| + numeric_answer.validates :numeric_data, presence: true + end + with_options if: ->(answer) { %w(text).include?(answer&.question&.data_type) } do |text_answer| + text_answer.validates :text_data, presence: true + end + with_options if: ->(answer) { %w(text).include?(answer&.question&.data_type) } do |text_answer| + text_answer.validates :text_data, presence: true + end + with_options if: ->(answer) { %w(range).include?(answer&.question&.data_type) } do |range_answer| + range_answer.validate :has_rating_or_text + end validate :question_is_not_group # this method is temporary for spr 2022 overhaul @@ -29,39 +32,59 @@ def compare_to_custom_data custom_data_value = custom_data_response.send(m) answer_value = answer_response.send(m) unless custom_data_value == answer_value - raise "ERROR for answer #{id}: for RS #{rs.id} custom data value for #{m} is #{custom_data_value} but is #{answer_value} for answer #{id}" + raise "ERROR for answer #{id}: for RS #{response_set.id} custom data value for #{m} is #{custom_data_value} but is #{answer_value} for answer #{id}" end end end + def self.no_answer?(q, fields, response_set) + puts fields + puts fields.class + response = Response.new(loan: response_set.loan, response_set: response_set, question: q, data: fields) + return response.blank? + end + # this method is temporary for spr 2022 overhaul + # doesn't save blank answers def self.save_from_form_field_params(q_internal_name, fields, response_set) q = Question.find_by(internal_name: q_internal_name) - not_applicable = fields.key?("not_applicable") ? (fields["not_applicable"] == "yes") : "no" - text_data = fields.key?("text") ? fields["text"] : nil - numeric_data = fields.key?("number") ? fields["number"] : nil - boolean_data = fields.key?("boolean") ? (fields["boolean"] == "yes") : "no" - breakeven_data = fields.key?("breakeven") ? fields["breakeven"] : nil - business_canvas_data = fields.key?("business_canvas") ? fields["business_canvas"] : nil - linked_document_data = fields.key?("url") ? {"url": fields["url"] } : {"url": ""} - linked_document_data["start_cell"] = fields.key?("start_cell") ? fields["start_cell"] : "" - linked_document_data["end_cell"] = fields.key?("end_cell") ? fields["end_cell"] : "" - answer = Answer.find_or_create_by(response_set: response_set, question: q) - answer.update!({ - not_applicable: not_applicable, - text_data: text_data, - numeric_data: numeric_data, - boolean_data: boolean_data, - breakeven_data: breakeven_data, - business_canvas_data: business_canvas_data, - linked_document_data: linked_document_data - }) + unless self.no_answer?(q, fields.to_h, response_set) + not_applicable = fields.key?("not_applicable") ? (fields["not_applicable"] == "yes") : "no" + text_data = fields.key?("text") ? fields["text"] : nil + numeric_data = if fields.key?("number") + fields["number"] + elsif fields.key?("rating") + fields["rating"] + else + nil + end + boolean_data = fields.key?("boolean") ? (fields["boolean"] == "yes") : "no" + breakeven_data = fields.key?("breakeven") ? fields["breakeven"] : nil + business_canvas_data = fields.key?("business_canvas") ? fields["business_canvas"] : nil + linked_document_data = fields.key?("url") ? {"url": fields["url"] } : {"url": ""} + linked_document_data["start_cell"] = fields.key?("start_cell") ? fields["start_cell"] : "" + linked_document_data["end_cell"] = fields.key?("end_cell") ? fields["end_cell"] : "" + answer = Answer.find_or_create_by(response_set: response_set, question: q) + answer.update!({ + not_applicable: not_applicable, + text_data: text_data, + numeric_data: numeric_data, + boolean_data: boolean_data, + breakeven_data: breakeven_data, + business_canvas_data: business_canvas_data, + linked_document_data: linked_document_data + }) + end end def question_is_not_group question.data_type != "group" end + def has_rating_or_text + numeric_data.present? || text_data.present? + end + # temp method for spr 2022 overhaul def raw_value json = {} @@ -79,7 +102,11 @@ def raw_value json["business_canvas"] = self.business_canvas_data end if self.numeric_data.present? - json["number"] = self.numeric_data + if self.question.data_type == "range" + json["rating"] = self.numeric_data + else + json["number"] = self.numeric_data + end end if self.linked_document_data.present? json["linked_document"] = self.linked_document_data diff --git a/app/models/response_set.rb b/app/models/response_set.rb index 328adaf1b..23b204ad0 100644 --- a/app/models/response_set.rb +++ b/app/models/response_set.rb @@ -26,11 +26,7 @@ def root_response # Fetches a custom value from the json field. # Ensures `question` is decorated before passing to Response. def response(question) - #puts "Answer for rs #{self.id} q #{question.id}" question = ensure_decorated(question) - # TODO: replace raw_value with - # 1) lookup answer record based on question.id and self.id - # 2) call answer model method to compose json expected for raw_value answer = Answer.find_by(response_set: self, question: question) raw_value = answer.present? ? answer.raw_value : nil Response.new(loan: loan, question: question, response_set: self, data: raw_value) @@ -39,7 +35,11 @@ def response(question) # for migration def ensure_all_answers_copied answer_q_ids = answers.pluck(:question_id).sort - custom_data_q_ids = custom_data.keys.map{|k| k.to_i}.sort + # select q ids where the response is not blank + custom_data_q_ids = custom_data.keys.select do |q_id| + rs = Response.new(loan: loan, question: Question.find(q_id), response_set: self, data: custom_data[q_id]) + return !rs.blank? + end.sort unless answer_q_ids == custom_data_q_ids qs_in_answers_only = answer_q_ids - custom_data_q_ids qs_in_custom_data_only = custom_data_q_ids - answer_q_ids @@ -65,7 +65,6 @@ def set_response(question, value) # call a new method on answer that takes value and saves the fields # i don't THINK we actually need to return custom data, but if we # have to, we'll call a method on answer model that composes custom_data equivalent. - self.custom_data ||= {} custom_data[question.json_key] = value custom_data @@ -77,10 +76,6 @@ def embedded_urls custom_data.values.map { |v| v["url"].presence }.compact end - def json_blob - qset = self.question_set - - end def custom_data_from_answers response_custom_data_json = {} @@ -96,6 +91,7 @@ def make_answers if question.present? && question.data_type != "group" response = Response.new(loan: loan, question: question, response_set: self, data: response_data) + next if response.blank? text_data = response.text.present? ? response.text : nil numeric_data = if response.has_number? response.number From f8de5b593bc12afaea0e8f997d3712eafe7e3c12 Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Tue, 26 Apr 2022 15:40:57 -0500 Subject: [PATCH 11/30] WIP failing specs on 12179 --- app/models/answer.rb | 3 +- app/models/response.rb | 42 +++++++++++++++++-- app/models/response_set.rb | 5 +++ spec/factories/response_sets_factory.rb | 1 + spec/models/loan_response_progress_spec.rb | 10 ++--- spec/models/response_spec.rb | 12 +++++- spec/support/helpers/question_spec_helpers.rb | 22 +++++----- 7 files changed, 71 insertions(+), 24 deletions(-) diff --git a/app/models/answer.rb b/app/models/answer.rb index 2d6b49ef2..d9a3a2866 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -38,8 +38,6 @@ def compare_to_custom_data end def self.no_answer?(q, fields, response_set) - puts fields - puts fields.class response = Response.new(loan: response_set.loan, response_set: response_set, question: q, data: fields) return response.blank? end @@ -47,6 +45,7 @@ def self.no_answer?(q, fields, response_set) # this method is temporary for spr 2022 overhaul # doesn't save blank answers def self.save_from_form_field_params(q_internal_name, fields, response_set) + puts fields q = Question.find_by(internal_name: q_internal_name) unless self.no_answer?(q, fields.to_h, response_set) not_applicable = fields.key?("not_applicable") ? (fields["not_applicable"] == "yes") : "no" diff --git a/app/models/response.rb b/app/models/response.rb index 5b76a516d..3b3b2f891 100644 --- a/app/models/response.rb +++ b/app/models/response.rb @@ -19,10 +19,16 @@ def initialize(loan:, question:, response_set:, data:) @question = question @response_set = response_set - ALL_VALUE_TYPES.each do |type| - instance_variable_set("@#{type}", data[type.to_sym]) + # ALL_VALUE_TYPES.each do |type| + # instance_variable_set("@#{type}", data[type.to_sym]) + # end + @breakeven = remove_blanks(@breakeven) + # start overhaul code + @answer = nil + if @question.present? && @response_set.present? + @answer = Answer.find_by(question_id: @question.id, response_set_id: @response_set.id) end - @breakeven = (@breakeven) + #end overhaul code end def model_name @@ -37,6 +43,34 @@ def linked_document end end + #===== START temp methods for 2022 overhaul ===# + + def boolean + @answer.boolean_data ? "yes" : "no" if @answer.present? + end + + def text + @answer.text_data if @answer.present? + end + + def number + @answer.numeric_data if @answer.present? + end + + def not_applicable + @answer.not_applicable ? "yes" : "no" if @answer.present? + end + + def url + @answer.linked_document_data["url"] if @answer.present? + end + + def group? + @question.present? && @question.group? + end + + #===== START temp methods for 2022 overhaul ===# + def breakeven_table @breakeven_table ||= BreakevenTableQuestion.new(breakeven) end @@ -84,7 +118,7 @@ def text_form_field_type # Boolean attributes are currently stored as "yes"/"no" in the ResponseSet data. This could # get refactored in future to use actual booleans. def not_applicable? - not_applicable == "yes" + not_applicable end private diff --git a/app/models/response_set.rb b/app/models/response_set.rb index 23b204ad0..101c76d3b 100644 --- a/app/models/response_set.rb +++ b/app/models/response_set.rb @@ -55,6 +55,11 @@ def save_answers(form_hash) end end + # for specs in overhaul + def set_answer_from_custom_data_style_json(question, value) + Answer.save_from_form_field_params(question.id.to_s, value, self) + end + # Change/assign custom field value, but don't save. # WHY do we not save here? probably just to save db writes # THIS is where the question internal_name (e.g. field_110) coming form jqtree gets converted back to diff --git a/spec/factories/response_sets_factory.rb b/spec/factories/response_sets_factory.rb index e4960b3cc..8e9781233 100644 --- a/spec/factories/response_sets_factory.rb +++ b/spec/factories/response_sets_factory.rb @@ -1,5 +1,6 @@ FactoryBot.define do factory :response_set do loan + question_set end end diff --git a/spec/models/loan_response_progress_spec.rb b/spec/models/loan_response_progress_spec.rb index b7e84de4c..42cf59a24 100644 --- a/spec/models/loan_response_progress_spec.rb +++ b/spec/models/loan_response_progress_spec.rb @@ -59,11 +59,11 @@ let!(:q32) { create_question(parent: q3, name: "q32", type: "boolean", required: false) } # answered before do - rset.set_response(q1, {"text" => "foo"}) - rset.set_response(q2, {"text" => ""}) # required - rset.set_response(q3, {"text" => "stuff"}) # required - rset.set_response(q31, {"text" => "junk"}) # required - rset.set_response(q32, {"boolean" => "no"}) + rset.set_answer_from_custom_data_style_json(q1, {"text" => "foo"}) + rset.set_answer_from_custom_data_style_json(q2, {"text" => ""}) # required + rset.set_answer_from_custom_data_style_json(q3, {"text" => "stuff"}) # required + rset.set_answer_from_custom_data_style_json(q31, {"text" => "junk"}) # required + rset.set_answer_from_custom_data_style_json(q32, {"boolean" => "no"}) end it "should be correct" do diff --git a/spec/models/response_spec.rb b/spec/models/response_spec.rb index 72668e39f..ab0230265 100644 --- a/spec/models/response_spec.rb +++ b/spec/models/response_spec.rb @@ -1,17 +1,22 @@ require 'rails_helper' describe Response do + let(:response_set) { create(:response_set)} let(:question) { create(:question, data_type: type) } let(:response) do Response.new( loan: nil, question: question, - response_set: nil, + response_set: response_set, data: data ) end subject { response } + before do + Answer.save_from_form_field_params(question.internal_name, data, response_set) + end + describe '#blank?' do context 'for non-group questions' do let(:type) { 'number' } @@ -25,7 +30,10 @@ context 'non-empty response' do let(:data) { {'number' => 1} } - it { is_expected.not_to be_blank } + it do + puts Answer.find_by(question_id: question.id, response_set_id: response_set.id) + is_expected.not_to be_blank + end end end diff --git a/spec/support/helpers/question_spec_helpers.rb b/spec/support/helpers/question_spec_helpers.rb index 444c756bc..85f42441b 100644 --- a/spec/support/helpers/question_spec_helpers.rb +++ b/spec/support/helpers/question_spec_helpers.rb @@ -7,7 +7,7 @@ module QuestionSpecHelpers let!(:loan2) { create(:loan, loan_type_value: lt2.value)} let!(:qset) { create(:question_set, kind: 'loan_criteria') } let!(:root) { qset.root_group } - let(:rset) { build(:response_set, loan: loan1, question_set: qset) } + let!(:rset) { create(:response_set, loan: loan1, question_set: qset) } end shared_context "full question set and responses" do @@ -44,16 +44,16 @@ module QuestionSpecHelpers let!(:q52) { create_question(parent: q5, name: "q52", type: "boolean", required: false) } before do - rset.set_response(q1, {"text" => "foo"}) - rset.set_response(q2, {"text" => ""}) # required - rset.set_response(q31, {"text" => "junk"}) # required - rset.set_response(q32, {"boolean" => "no"}) # required - rset.set_response(q331, {"boolean" => "yes"}) - rset.set_response(q39, {"text" => "inactive question"}) - rset.set_response(q41, {"text" => ""}) - rset.set_response(q42, {"text" => "pants"}) - rset.set_response(q43, {"text" => ""}) - rset.set_response(q51, {"text" => "inactive group"}) + rset.set_answer_from_custom_data_style_json(q1, {"text" => "foo"}) + rset.set_answer_from_custom_data_style_json(q2, {"text" => ""}) # required + rset.set_answer_from_custom_data_style_json(q31, {"text" => "junk"}) # required + rset.set_answer_from_custom_data_style_json(q32, {"boolean" => "no"}) # required + rset.set_answer_from_custom_data_style_json(q331, {"boolean" => "yes"}) + rset.set_answer_from_custom_data_style_json(q39, {"text" => "inactive question"}) + rset.set_answer_from_custom_data_style_json(q41, {"text" => ""}) + rset.set_answer_from_custom_data_style_json(q42, {"text" => "pants"}) + rset.set_answer_from_custom_data_style_json(q43, {"text" => ""}) + rset.set_answer_from_custom_data_style_json(q51, {"text" => "inactive group"}) rset.save! # Reload groups so they see their children! From 506add72d115d872f70799c908e04d367a66484c Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Tue, 26 Apr 2022 17:23:21 -0500 Subject: [PATCH 12/30] WIP 12197: more specs passing --- app/models/answer.rb | 58 +++++++++++++++++++++++----- app/models/response.rb | 66 +++++++++++++------------------- app/models/response_set.rb | 13 ++++++- spec/models/response_set_spec.rb | 32 ++++++++++------ spec/models/response_spec.rb | 20 ---------- 5 files changed, 107 insertions(+), 82 deletions(-) diff --git a/app/models/answer.rb b/app/models/answer.rb index d9a3a2866..2aac40975 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -6,7 +6,7 @@ class Answer < ApplicationRecord # TODO consider bringing back when frontend can support no answer existing # ========================================================================= with_options if: ->(answer) { answer&.question&.data_type == "boolean" } do |boolean_answer| - boolean_answer.validates :boolean_data, presence: true + boolean_answer.validate :valid_boolean end with_options if: ->(answer) { %w(rating range currency number percentage).include?(answer&.question&.data_type) } do |numeric_answer| numeric_answer.validates :numeric_data, presence: true @@ -37,17 +37,52 @@ def compare_to_custom_data end end - def self.no_answer?(q, fields, response_set) - response = Response.new(loan: response_set.loan, response_set: response_set, question: q, data: fields) - return response.blank? + def blank? + text_data.empty? && + numeric_data.empty? && + boolean_data.empty? && + linked_document_data_blank? && + business_canvas_blank? && + breakeven_data_blank? + end + + def linked_document_data_blank? + linked_document_data.empty? || json_answer_blank?(linked_document_data) + end + + def business_canvas_blank? + business_canvas_data.blank? || json_answer_blank?(business_canvas_data) + end + + def breakeven_data_blank? + breakeven_data.blank? || json_answer_blank?(breakeven_data) + end + + def json_answer_blank?(answer_json) + answer_json.values.all?{|v| v.blank?} + end + + # expects 'raw_value' type json e.g. the value of a "field_110" key in form submission + def self.contains_answer_data?(hash_data) + hash_data.each do |key, value| + if %w(text number rating boolean url start_cell end_cell).include?(key) + return true unless value.blank? + elsif key == "not_applicable" + return true if value == "yes" + elsif key == "boolean" + return true unless value.blank? + elsif %w(breakeven business_canvas).include?(key) + return true unless json_answer_blank(value) + end + end + return false end # this method is temporary for spr 2022 overhaul # doesn't save blank answers - def self.save_from_form_field_params(q_internal_name, fields, response_set) + def self.save_from_form_field_params(question, fields, response_set) puts fields - q = Question.find_by(internal_name: q_internal_name) - unless self.no_answer?(q, fields.to_h, response_set) + unless question.group? || !self.contains_answer_data?(fields) not_applicable = fields.key?("not_applicable") ? (fields["not_applicable"] == "yes") : "no" text_data = fields.key?("text") ? fields["text"] : nil numeric_data = if fields.key?("number") @@ -57,13 +92,14 @@ def self.save_from_form_field_params(q_internal_name, fields, response_set) else nil end - boolean_data = fields.key?("boolean") ? (fields["boolean"] == "yes") : "no" + boolean_data = fields.key?("boolean") ? (fields["boolean"] == "yes") : nil breakeven_data = fields.key?("breakeven") ? fields["breakeven"] : nil business_canvas_data = fields.key?("business_canvas") ? fields["business_canvas"] : nil linked_document_data = fields.key?("url") ? {"url": fields["url"] } : {"url": ""} linked_document_data["start_cell"] = fields.key?("start_cell") ? fields["start_cell"] : "" linked_document_data["end_cell"] = fields.key?("end_cell") ? fields["end_cell"] : "" - answer = Answer.find_or_create_by(response_set: response_set, question: q) + answer = Answer.find_or_create_by(response_set: response_set, question: question) + puts boolean_data answer.update!({ not_applicable: not_applicable, text_data: text_data, @@ -80,6 +116,10 @@ def question_is_not_group question.data_type != "group" end + def valid_boolean + !boolean_data.nil? + end + def has_rating_or_text numeric_data.present? || text_data.present? end diff --git a/app/models/response.rb b/app/models/response.rb index 3b3b2f891..acbf28939 100644 --- a/app/models/response.rb +++ b/app/models/response.rb @@ -18,17 +18,10 @@ def initialize(loan:, question:, response_set:, data:) @loan = loan @question = question @response_set = response_set - - # ALL_VALUE_TYPES.each do |type| - # instance_variable_set("@#{type}", data[type.to_sym]) - # end - @breakeven = remove_blanks(@breakeven) - # start overhaul code - @answer = nil - if @question.present? && @response_set.present? - @answer = Answer.find_by(question_id: @question.id, response_set_id: @response_set.id) + ALL_VALUE_TYPES.each do |type| + instance_variable_set("@#{type}", data[type.to_sym]) end - #end overhaul code + @breakeven = remove_blanks(@breakeven) end def model_name @@ -45,29 +38,29 @@ def linked_document #===== START temp methods for 2022 overhaul ===# - def boolean - @answer.boolean_data ? "yes" : "no" if @answer.present? - end - - def text - @answer.text_data if @answer.present? - end - - def number - @answer.numeric_data if @answer.present? - end - - def not_applicable - @answer.not_applicable ? "yes" : "no" if @answer.present? - end - - def url - @answer.linked_document_data["url"] if @answer.present? - end - - def group? - @question.present? && @question.group? - end + # def boolean + # @answer.boolean_data ? "yes" : "no" if @answer.present? + # end + # + # def text + # @answer.text_data if @answer.present? + # end + # + # def number + # @answer.numeric_data if @answer.present? + # end + # + # def not_applicable + # @answer.not_applicable ? "yes" : "no" if @answer.present? + # end + # + # def url + # @answer.linked_document_data["url"] if @answer.present? + # end + # + # def group? + # @question.present? && @question.group? + # end #===== START temp methods for 2022 overhaul ===# @@ -94,12 +87,7 @@ def breakeven_report # Checks if response is blank, including any descendants if this is a group. def blank? - if group? - children.all?(&:blank?) || children.all?(&:not_applicable?) - else - !not_applicable? && text.blank? && number.blank? && rating.blank? && - boolean.blank? && url.blank? && breakeven_report.blank? && business_canvas_blank? - end + @response_set.present? && @response_set.question_blank?(@question) end def business_canvas_blank? diff --git a/app/models/response_set.rb b/app/models/response_set.rb index 101c76d3b..75e229ca8 100644 --- a/app/models/response_set.rb +++ b/app/models/response_set.rb @@ -19,6 +19,14 @@ def recalculate_loan_health RecalculateLoanHealthJob.perform_later(loan_id: loan_id) end + def question_blank?(question) + if question.group? + question.children.all?{|c| question_blank?(c)} + else + Answer.where(question_id: question.id, response_set_id: self.id).blank? + end + end + def root_response response(question(:root)) end @@ -50,14 +58,15 @@ def ensure_all_answers_copied def save_answers(form_hash) form_hash.each do |key, value| if key.include?("field") # key is an internal_name of a question - Answer.save_from_form_field_params(key, value, self) + question = Question.find_by(internal_name: key) + Answer.save_from_form_field_params(question, value, self) end end end # for specs in overhaul def set_answer_from_custom_data_style_json(question, value) - Answer.save_from_form_field_params(question.id.to_s, value, self) + Answer.save_from_form_field_params(question, value, self) end # Change/assign custom field value, but don't save. diff --git a/spec/models/response_set_spec.rb b/spec/models/response_set_spec.rb index bc1e92ddc..388a9be0e 100644 --- a/spec/models/response_set_spec.rb +++ b/spec/models/response_set_spec.rb @@ -1,23 +1,31 @@ require 'rails_helper' -describe ResponseSet, js: true do +describe ResponseSet do let(:division) { create(:division) } let!(:question_set) { create(:question_set, :with_questions, kind: "loan_criteria", division: division)} - let(:user) { create_member(division) } let(:loan) { create(:loan, division: division) } - before do - login_as(user, scope: :user) - end + describe "question_blank?" do + context "answer to this question is blank" do + it "should be true" do + + end + end + + context "answer to this question exists" do + it "should be false" do + end + end + context "only one grandchild question is answered" do + it "should be false" do + end + end - def fill_and_save(type, value) - first(".edit-all").click - expect(page).to have_content("Now editing") - find("input[type=#{type}]").set(value) - find("body").click # Trigger blur event so that Save Changes button is properly clickable - click_button("Save Changes") - expect(page).not_to have_content("Now editing") + context "this quesion is a group that has groups but no descendent qs are answered" do + it "should be true" do + end + end end end diff --git a/spec/models/response_spec.rb b/spec/models/response_spec.rb index ab0230265..1808b0e91 100644 --- a/spec/models/response_spec.rb +++ b/spec/models/response_spec.rb @@ -17,26 +17,6 @@ Answer.save_from_form_field_params(question.internal_name, data, response_set) end - describe '#blank?' do - context 'for non-group questions' do - let(:type) { 'number' } - - context 'empty response' do - let(:data) { {} } - - it { is_expected.to be_blank } - end - - context 'non-empty response' do - let(:data) { {'number' => 1} } - - it do - puts Answer.find_by(question_id: question.id, response_set_id: response_set.id) - is_expected.not_to be_blank - end - end - end - context 'for group questions' do let(:type) { 'group' } let(:q1) { create(:question, parent: question, data_type: 'number') } From 7cba41a35727ce3db384a75e4cb2bee5473d4549 Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Wed, 27 Apr 2022 12:22:38 -0500 Subject: [PATCH 13/30] 12197: remove response blank specs, and fix some of the boolean handling between answer and old style json for not applicable and boolean data type questions --- app/models/answer.rb | 20 ++++++++++++------ spec/models/response_spec.rb | 28 ------------------------- spec/system/admin/questionnaire_spec.rb | 20 ------------------ 3 files changed, 14 insertions(+), 54 deletions(-) diff --git a/app/models/answer.rb b/app/models/answer.rb index 2aac40975..203871a94 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -58,21 +58,29 @@ def breakeven_data_blank? breakeven_data.blank? || json_answer_blank?(breakeven_data) end - def json_answer_blank?(answer_json) + def self.json_answer_blank?(answer_json) answer_json.values.all?{|v| v.blank?} end # expects 'raw_value' type json e.g. the value of a "field_110" key in form submission def self.contains_answer_data?(hash_data) hash_data.each do |key, value| - if %w(text number rating boolean url start_cell end_cell).include?(key) + if %w(text number rating url start_cell end_cell).include?(key) return true unless value.blank? elsif key == "not_applicable" return true if value == "yes" elsif key == "boolean" - return true unless value.blank? - elsif %w(breakeven business_canvas).include?(key) - return true unless json_answer_blank(value) + return true unless value.nil? + elsif %w(business_canvas).include?(key) + return true unless self.json_answer_blank?(value) + elsif %w(breakeven).include?(key) + value.each do |subkey, subvalue| + if %w(products fixed_costs).include?(subkey) + subvalue.each {|i| return true unless self.json_answer_blank?(i)} + else + return true unless subvalue.empty? + end + end end end return false @@ -131,7 +139,7 @@ def raw_value if self.text_data.present? json["text"] = self.text_data end - if self.boolean_data.present? + unless self.boolean_data.nil? json["boolean"] = self.boolean_data ? "yes" : "no" end if self.breakeven_data.present? diff --git a/spec/models/response_spec.rb b/spec/models/response_spec.rb index 1808b0e91..1a5d1c3e9 100644 --- a/spec/models/response_spec.rb +++ b/spec/models/response_spec.rb @@ -16,32 +16,4 @@ before do Answer.save_from_form_field_params(question.internal_name, data, response_set) end - - context 'for group questions' do - let(:type) { 'group' } - let(:q1) { create(:question, parent: question, data_type: 'number') } - let(:q2) { create(:question, parent: question, data_type: 'number') } - let(:r1) { Response.new(loan: nil, question: q1, response_set: nil, data: data1) } - let(:r2) { Response.new(loan: nil, question: q2, response_set: nil, data: data2) } - let(:data) { {} } - let(:data1) { {} } - - before do - question.reload - allow(response).to receive(:children) { [r1, r2] } - end - - context 'both child responses blank' do - let(:data2) { {} } - - it { is_expected.to be_blank } - end - - context 'non-empty response' do - let(:data2) { { 'number' => 3 } } - - it { is_expected.not_to be_blank } - end - end - end end diff --git a/spec/system/admin/questionnaire_spec.rb b/spec/system/admin/questionnaire_spec.rb index ec6e9bc6c..bdf897a4a 100644 --- a/spec/system/admin/questionnaire_spec.rb +++ b/spec/system/admin/questionnaire_spec.rb @@ -86,26 +86,6 @@ questions[t] = Question.create(data_type: t, question_set: question_set, parent: root_q, division: division) end end - - scenario "custom data and answers table have same info" do - visit admin_loan_tab_path(loan, "questions") - first(".edit-all").click - save_and_open_page - #page.find('.select', wait: 10, visible: false) - select_el = page.find("#response_set_#{questions[:boolean].attribute_sym}\\[boolean\\]", wait: 10, visible: false) - #select_el.find(:xpath, 'option[1]').select_option - #page.select("Yes", from: "response_set_#{questions[:boolean].attribute_sym}\\[boolean\\]", visible: false) - find("#response_set_#{questions[:boolean].attribute_sym}\\[boolean\\] option[value='']").click - find("#response_set_#{questions[:boolean].attribute_sym}\\[boolean\\] option[value='yes']", visible: false).select_option - fill_qtype_with_value("number", "123") - fill_qtype_with_value("url", "www.example.com") - fill_qtype_with_value("start_cell", "A2") - fill_qtype_with_value("end_cell", "Z100") - click_button("Save Changes") - response_set = ResponseSet.find_by(question_set_id: question_set.id) - response_set.make_answers - expect(response_set.custom_data).to match_unordered_json(response_set.custom_data_from_answers) - end end def fill_qtype_with_value(type, value) From 7119f494a0cafe8b28439b3abfd25ea5f6ef4bee Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Wed, 27 Apr 2022 12:44:06 -0500 Subject: [PATCH 14/30] 12179: fixing not applicable boolean madness --- app/models/response.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/app/models/response.rb b/app/models/response.rb index acbf28939..99bc30eaf 100644 --- a/app/models/response.rb +++ b/app/models/response.rb @@ -85,6 +85,10 @@ def breakeven_report end end + def not_applicable? + not_applicable == "yes" + end + # Checks if response is blank, including any descendants if this is a group. def blank? @response_set.present? && @response_set.question_blank?(@question) @@ -103,12 +107,6 @@ def text_form_field_type question.data_type == 'text' ? :text : :string end - # Boolean attributes are currently stored as "yes"/"no" in the ResponseSet data. This could - # get refactored in future to use actual booleans. - def not_applicable? - not_applicable - end - private # Responses are made up of one or more value types. e.g. a `range` is composed of a `rating` and a `text` From d2348c7c722244f6e55101128339e575f66a6358 Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Wed, 27 Apr 2022 12:48:42 -0500 Subject: [PATCH 15/30] 12179: fix displaying linked document --- app/models/answer.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/models/answer.rb b/app/models/answer.rb index 203871a94..1b123335f 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -156,7 +156,9 @@ def raw_value end end if self.linked_document_data.present? - json["linked_document"] = self.linked_document_data + json["url"] = self.linked_document_data["url"] + json["start_cell"] = self.linked_document_data["start_cell"] + json["end_cell"] = self.linked_document_data["end_cell"] end #puts json json From 344213641131730a8ba7309329bf43da96aa955f Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Wed, 27 Apr 2022 15:06:49 -0500 Subject: [PATCH 16/30] 12179: make validations on answer more expansive; just check there is some meaningful answer data --- app/models/answer.rb | 50 +++++++++++++++----------------------- app/models/response_set.rb | 35 ++++++-------------------- spec/models/answer_spec.rb | 10 +------- 3 files changed, 29 insertions(+), 66 deletions(-) diff --git a/app/models/answer.rb b/app/models/answer.rb index 1b123335f..5978e35ad 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -2,24 +2,7 @@ class Answer < ApplicationRecord belongs_to :response_set, optional: false belongs_to :question, optional: false delegate :data_type, to: :question - - # TODO consider bringing back when frontend can support no answer existing - # ========================================================================= - with_options if: ->(answer) { answer&.question&.data_type == "boolean" } do |boolean_answer| - boolean_answer.validate :valid_boolean - end - with_options if: ->(answer) { %w(rating range currency number percentage).include?(answer&.question&.data_type) } do |numeric_answer| - numeric_answer.validates :numeric_data, presence: true - end - with_options if: ->(answer) { %w(text).include?(answer&.question&.data_type) } do |text_answer| - text_answer.validates :text_data, presence: true - end - with_options if: ->(answer) { %w(text).include?(answer&.question&.data_type) } do |text_answer| - text_answer.validates :text_data, presence: true - end - with_options if: ->(answer) { %w(range).include?(answer&.question&.data_type) } do |range_answer| - range_answer.validate :has_rating_or_text - end + validate :has_data validate :question_is_not_group # this method is temporary for spr 2022 overhaul @@ -37,17 +20,23 @@ def compare_to_custom_data end end + def to_s + "NA: #{not_applicable}; text: #{text_data}; numeric: #{numeric_data}; boolean: #{boolean_data}; doc: #{linked_document_data}; breakeven: #{breakeven_data}; canvas: #{business_canvas_data}" + end + def blank? - text_data.empty? && - numeric_data.empty? && - boolean_data.empty? && + puts self.to_s + #!not_applicable? && + text_data.blank? && + numeric_data.blank? && + boolean_data.blank? && linked_document_data_blank? && business_canvas_blank? && breakeven_data_blank? end def linked_document_data_blank? - linked_document_data.empty? || json_answer_blank?(linked_document_data) + linked_document_data.blank? || json_answer_blank?(linked_document_data) end def business_canvas_blank? @@ -89,7 +78,6 @@ def self.contains_answer_data?(hash_data) # this method is temporary for spr 2022 overhaul # doesn't save blank answers def self.save_from_form_field_params(question, fields, response_set) - puts fields unless question.group? || !self.contains_answer_data?(fields) not_applicable = fields.key?("not_applicable") ? (fields["not_applicable"] == "yes") : "no" text_data = fields.key?("text") ? fields["text"] : nil @@ -107,7 +95,6 @@ def self.save_from_form_field_params(question, fields, response_set) linked_document_data["start_cell"] = fields.key?("start_cell") ? fields["start_cell"] : "" linked_document_data["end_cell"] = fields.key?("end_cell") ? fields["end_cell"] : "" answer = Answer.find_or_create_by(response_set: response_set, question: question) - puts boolean_data answer.update!({ not_applicable: not_applicable, text_data: text_data, @@ -124,12 +111,15 @@ def question_is_not_group question.data_type != "group" end - def valid_boolean - !boolean_data.nil? - end - - def has_rating_or_text - numeric_data.present? || text_data.present? + def has_data + errors.add("Answer contains no data") unless + not_applicable || + text_data.present? || + numeric_data.present? || + !boolean_data.nil? || + linked_document_data.present? || + business_canvas_data.present? || + breakeven_data.present? end # temp method for spr 2022 overhaul diff --git a/app/models/response_set.rb b/app/models/response_set.rb index 75e229ca8..4de028042 100644 --- a/app/models/response_set.rb +++ b/app/models/response_set.rb @@ -102,33 +102,14 @@ def custom_data_from_answers def make_answers custom_data.each do |q_id, response_data| question = Question.find(q_id) - if question.present? && question.data_type != "group" - response = Response.new(loan: loan, question: question, - response_set: self, data: response_data) - next if response.blank? - text_data = response.text.present? ? response.text : nil - numeric_data = if response.has_number? - response.number - elsif response.has_rating? - response.rating - else - nil - end - boolean_data = response.has_boolean? ? (response.boolean == "yes") : nil - breakeven_data = response.has_breakeven? ? response.breakeven : nil - business_canvas_data = response.has_business_canvas? ? response.business_canvas : nil - linked_document_data = question.has_embeddable_media? ? {url: response.url, start_cell: response.start_cell, end_cell: response.end_cell} : nil - Answer.create({ - response_set: self, - question: question, - not_applicable: response.not_applicable?, - text_data: text_data, - numeric_data: numeric_data, - boolean_data: boolean_data, - breakeven_data: breakeven_data, - business_canvas_data: business_canvas_data, - linked_document_data: linked_document_data - }) + if question.present? + begin + Answer.save_from_form_field_params(question, response_data, self) + rescue => e + puts "Q #{question.id} #{question.data_type}" + puts response_data + raise e + end end end end diff --git a/spec/models/answer_spec.rb b/spec/models/answer_spec.rb index 6f1d91a99..d42ce2340 100644 --- a/spec/models/answer_spec.rb +++ b/spec/models/answer_spec.rb @@ -36,17 +36,9 @@ context "has no text_data" do let(:text_data) { nil } - it "is valid" do + it "is not valid" do expect(subject.valid?).to be_falsey end end end - - context "migration code" do - context "one answer" do - - end - - - end end From 5a21afe1eaefc3b161ed28352041fc594d3b0a12 Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Mon, 2 May 2022 10:52:23 -0500 Subject: [PATCH 17/30] 12197: WIP enhanced data export working --- .../admin/data_exports_controller.rb | 1 + app/models/answer.rb | 18 ++++++ app/models/data_export.rb | 38 ++++++++---- app/models/enhanced_loan_data_export.rb | 36 +++-------- app/models/numeric_answer_data_export.rb | 4 -- spec/models/enhanced_loan_data_export_spec.rb | 60 +++++++------------ 6 files changed, 79 insertions(+), 78 deletions(-) diff --git a/app/controllers/admin/data_exports_controller.rb b/app/controllers/admin/data_exports_controller.rb index d1da2bbb3..9c8d55a1a 100644 --- a/app/controllers/admin/data_exports_controller.rb +++ b/app/controllers/admin/data_exports_controller.rb @@ -16,6 +16,7 @@ def new end def create + Rails::Debug.logger.info "hello debug " @export_class = data_export_create_params[:type].constantize @data_export = @export_class.new(data_export_create_params) @data_export.division = selected_division_or_root diff --git a/app/models/answer.rb b/app/models/answer.rb index 5978e35ad..c449f74bc 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -52,6 +52,7 @@ def self.json_answer_blank?(answer_json) end # expects 'raw_value' type json e.g. the value of a "field_110" key in form submission + # or the value of a q_id key e.g. "5126" in custom_data def self.contains_answer_data?(hash_data) hash_data.each do |key, value| if %w(text number rating url start_cell end_cell).include?(key) @@ -159,4 +160,21 @@ def raw_value def custom_data_json return {"#{self.question.json_key}": self.raw_value} end + + def answer_for_csv + return nil if not_applicable + + case question.data_type + when "text" + text_data + when "number", "currency", "percentage", "range" + numeric_data.to_s + when "boolean" + boolean_data.nil? ? nil : (boolean_data ? "yes" : "no") + when "breakeven", "business_canvas" + nil + else + raise "invalid question data type #{question.data_type}" + end + end end diff --git a/app/models/data_export.rb b/app/models/data_export.rb index 902d1d99a..4587790e2 100644 --- a/app/models/data_export.rb +++ b/app/models/data_export.rb @@ -21,19 +21,20 @@ def self.model_name end def process_data + logger.debug("LOGGING FROM data_export.rb process_data") @child_errors = [] data = [] data.concat(header_rows) scope.find_each do |object| - begin + #begin data << hash_to_row(object_data_as_hash(object)) - rescue => e - Rails.logger.error("Error for loan #{object.id} in data export #{self.name}: #{e}") - - # TODO generalize object beyond loan here and in task show if non-loans exported - @child_errors << {loan_id: object.id, message: e.message} - next - end + # rescue => e + # Rails.logger.error("Error for loan #{object.id} in data export #{self.name}: #{e}") + # + # # TODO generalize object beyond loan here and in task show if non-loans exported + # @child_errors << {loan_id: object.id, message: e.message} + # next + # end end self.update(data: data) @@ -43,6 +44,7 @@ def process_data end def to_csv! + logger.debug("LOGGING FROM data_export.rb") raise ArgumentError, "No data found" if data.blank? raise TypeError, "Data should be a 2D Array" unless (data.is_a?(Array) && data.first.is_a?(Array)) @@ -76,7 +78,23 @@ def task protected def insert_in_row(header_symbol, row_array, value) - row_array[header_symbols_to_indices[header_symbol]] = value + begin + row_array[header_symbols_to_indices[header_symbol]] = value + rescue => e + logger.info("mangoes yum") + logger.info e.message + logger.info "header symbol" + logger.info header_symbol + logger.info "header symbol nil? #{header_symbol.nil?}" + logger.info "Header Symbols" + logger.info header_symbols + logger.info "Header Symbols to indices" + logger.info header_symbols_to_indices + logger.info "index nil? #{header_symbols_to_indices[header_symbol].nil?}" + logger.info "value: #{value}" + logger.info "value nil? #{value.nil?}" + raise e + end end private @@ -89,7 +107,7 @@ def hash_to_row(hash) # Builds a hash of header symbols to their appropriate indices in the row arrays. def header_symbols_to_indices - @header_symbols_to_indices = header_symbols.each_with_index.to_h + @header_symbols_to_indices ||= header_symbols.each_with_index.to_h end def set_name diff --git a/app/models/enhanced_loan_data_export.rb b/app/models/enhanced_loan_data_export.rb index ec66f6328..b8e4940dd 100644 --- a/app/models/enhanced_loan_data_export.rb +++ b/app/models/enhanced_loan_data_export.rb @@ -11,39 +11,20 @@ def response_hash(loan) result = {} response_sets = ResponseSet.joins(:question_set).where(loan: loan).order("question_sets.kind") response_sets.each do |response_set| - response_set.custom_data.each do |q_id, response_data| - question = questions_by_id[q_id.to_i] - if question.present? - response = Response.new(loan: loan, question: question, - response_set: response_set, data: response_data) - - # Note, this approach will exclude parts of compound data types, such as `range`, - # which can have both a `rating` and a `text` component. - # `url`, `start_cell`, and `end_cell` components from questions with `has_embeddable_media`=true - # are also not included, nor are `business_canvas`, and `breakeven`, which would be way too big - # to put in a CSV cell. - result[q_id.to_i] = - if response.not_applicable? - "" - elsif response.has_rating? - response.rating - elsif response.has_number? - include_numeric_answer_in_export?(response.number) ? response.number : "" - elsif response.has_boolean? - response.boolean - elsif response.has_text? - response.text - end + response_set.answers.each do |a| + if !(header_symbols_to_indices.include?(a.question.id)) + logger.debug("########## #{a.question.id} not found ########") + end + if q_data_types.include?(a.data_type) + result[a.question_id] = a.answer_for_csv + else + logger.info "#{a.question_id} type #{a.data_type} not included" end end end result end - def include_numeric_answer_in_export?(str) - true #include all numeric answers, even if invalid text - end - def q_data_types ["boolean", "text", "number", "percentage", "rating", "currency", "range"] end @@ -85,6 +66,7 @@ def questions_by_id # Returns the list of symbols representing headers in the order they should appear. def header_symbols + puts @header_symbols @header_symbols ||= StandardLoanDataExport::HEADERS + questions.map(&:id) end end diff --git a/app/models/numeric_answer_data_export.rb b/app/models/numeric_answer_data_export.rb index c5a523905..49f873a01 100644 --- a/app/models/numeric_answer_data_export.rb +++ b/app/models/numeric_answer_data_export.rb @@ -2,8 +2,4 @@ class NumericAnswerDataExport < EnhancedLoanDataExport def q_data_types ["number", "percentage", "rating", "currency", "range"] end - - def include_numeric_answer_in_export?(str) - true if Float(str) rescue false - end end diff --git a/spec/models/enhanced_loan_data_export_spec.rb b/spec/models/enhanced_loan_data_export_spec.rb index 7b4927289..b921f374b 100644 --- a/spec/models/enhanced_loan_data_export_spec.rb +++ b/spec/models/enhanced_loan_data_export_spec.rb @@ -38,32 +38,28 @@ # This question is on a subdivision, should still be included. let!(:qc5) do - create(:question, :with_url, qcattrs.merge(division: subdivision, data_type: "rating", label: "QC5")) + create(:question, :with_url, qcattrs.merge(division: subdivision, data_type: "range", label: "QC5")) end let!(:r1_c) do create(:response_set, question_set: criteria, - loan: loan1, - custom_data: { - qc1.id.to_s => {boolean: "yes", not_applicable: "no"}, - qc2.id.to_s => {number: "10", not_applicable: "no"}, - qc3.id.to_s => {text: "foo\nbar", not_applicable: "no"}, - qc4.id.to_s => {rating: "4", text: "text", not_applicable: "no"} - }) + loan: loan1) end + let!(:r1_c_a1) { create(:answer, question: qc1, response_set: r1_c, boolean_data: true, not_applicable: false) } + let!(:r1_c_a2) { create(:answer, question: qc2, response_set: r1_c, numeric_data: 10, not_applicable: false) } + let!(:r1_c_a3) { create(:answer, question: qc3, response_set: r1_c, text_data: "foo\nbar", not_applicable: false) } + let!(:r1_c_a4) { create(:answer, question: qc4, response_set: r1_c, numeric_data: "4", text_data: "text", not_applicable: false) } let!(:r2_c) do create(:response_set, question_set: criteria, - loan: loan2, - custom_data: { - qc1.id.to_s => {boolean: "", not_applicable: "yes"}, - qc2.id.to_s => {number: "invalid number answer should be in export", not_applicable: "no"}, - qc3.id.to_s => {text: "lorp", not_applicable: "no"}, - qc4.id.to_s => {rating: nil, text: nil, not_applicable: "yes"}, - qc5.id.to_s => {rating: "5", url: "https://example.com/1", not_applicable: "no"} - }) + loan: loan2) end + let!(:r2_c_a1) { create(:answer, question: qc1, response_set: r2_c, boolean_data: nil, not_applicable: true) } + let!(:r2_c_a2) { create(:answer, question: qc2, response_set: r2_c, numeric_data: 11, not_applicable: false) } + let!(:r2_c_a3) { create(:answer, question: qc3, response_set: r2_c, text_data: "lorp", not_applicable: false) } + let!(:r2_c_a4) { create(:answer, question: qc4, response_set: r2_c, numeric_data: nil, text_data: nil, not_applicable: true) } + let!(:r2_c_a5) { create(:answer, question: qc5, response_set: r2_c, numeric_data: 5, linked_document_data: {url: "https://example.com/1"}, not_applicable: false) } it "should create correct data attr" do export.process_data @@ -71,7 +67,7 @@ expect(export.data[1]).to eq(["Question ID"] + id_row_nils + [qc1, qc2, qc3, qc4, qc5].map(&:id)) row1 = ["yes", "10", "foo\nbar", "4"] - row2 = ["", "invalid number answer should be in export", "lorp", "", "5"] + row2 = [nil, "11", "lorp", nil, "5"] row3 = [] expect(response_data).to contain_exactly(row1, row2, row3) end @@ -83,19 +79,15 @@ let!(:r1_p) do create(:response_set, question_set: post_analysis, - loan: loan1, - custom_data: { - qp1.id.to_s => {number: "7", not_applicable: "no"} - }) + loan: loan1) end + let!(:r1_p_q1) { create(:answer, question: qp1, response_set: r1_p, numeric_data: 7, not_applicable: false)} let!(:r3_p) do create(:response_set, question_set: post_analysis, - loan: loan3, - custom_data: { - qp1.id.to_s => {number: "99.9", not_applicable: "no"} - }) + loan: loan3) end + let!(:r3_p_q1) { create(:answer, question: qp1, response_set: r3_p, numeric_data: 99.9, not_applicable: false)} it "should create correct data attr" do export.process_data @@ -103,7 +95,7 @@ expect(export.data[1]).to eq(["Question ID"] + id_row_nils + [qc1, qc2, qc3, qc4, qc5, qp1].map(&:id)) row1 = ["yes", "10", "foo\nbar", "4", nil, "7"] - row2 = ["", "invalid number answer should be in export", "lorp", "", "5"] + row2 = [nil, "11", "lorp", nil, "5"] row3 = [nil, nil, nil, nil, nil, "99.9"] expect(response_data).to contain_exactly(row1, row2, row3) end @@ -123,26 +115,20 @@ let!(:r1) do create(:response_set, question_set: setA, - loan: loan1, - custom_data: { - qa1.id.to_s => {number: "5", not_applicable: "no"}, - qa2.id.to_s => {number: "10", not_applicable: "no"} - }) + loan: loan1) end + let!(:r1_a1) { create(:answer, question: qa1, response_set: r1, numeric_data: 5, not_applicable: false) } + let!(:r1_a2) { create(:answer, question: qa2, response_set: r1, numeric_data: 10, not_applicable: false) } let!(:r2) do create(:response_set, question_set: setB, - loan: loan2, - custom_data: { - qb1.id.to_s => {number: "15", not_applicable: "no"} - }) + loan: loan2) end - + let!(:r2_a1) { create(:answer, question: qb1, response_set: r2, numeric_data: 15, not_applicable: false) } it "should include data from both questions sets" do export.process_data expect(export.data[0]).to eq(base_headers + ["QA1", "QA2", "QB1"]) expect(export.data[1]).to eq(["Question ID"] + id_row_nils + [qa1, qa2, qb1].map(&:id)) - row1 = ["5", "10"] row2 = [nil, nil, "15"] row3 = [] From 9a0e6442a7d6379e5c3ce92ea44cc0a63ba40f1a Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Mon, 2 May 2022 11:37:52 -0500 Subject: [PATCH 18/30] 12179: cleaning up debugging from work making data exports work with answers --- .../admin/data_exports_controller.rb | 1 - app/models/answer.rb | 3 +- app/models/data_export.rb | 35 +++++-------------- app/models/enhanced_loan_data_export.rb | 6 ---- 4 files changed, 10 insertions(+), 35 deletions(-) diff --git a/app/controllers/admin/data_exports_controller.rb b/app/controllers/admin/data_exports_controller.rb index 9c8d55a1a..d1da2bbb3 100644 --- a/app/controllers/admin/data_exports_controller.rb +++ b/app/controllers/admin/data_exports_controller.rb @@ -16,7 +16,6 @@ def new end def create - Rails::Debug.logger.info "hello debug " @export_class = data_export_create_params[:type].constantize @data_export = @export_class.new(data_export_create_params) @data_export.division = selected_division_or_root diff --git a/app/models/answer.rb b/app/models/answer.rb index c449f74bc..e3840102c 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -171,8 +171,7 @@ def answer_for_csv numeric_data.to_s when "boolean" boolean_data.nil? ? nil : (boolean_data ? "yes" : "no") - when "breakeven", "business_canvas" - nil + # "breakeven" and "business_canvas" never exported to csv else raise "invalid question data type #{question.data_type}" end diff --git a/app/models/data_export.rb b/app/models/data_export.rb index 4587790e2..9e651e4f1 100644 --- a/app/models/data_export.rb +++ b/app/models/data_export.rb @@ -21,20 +21,19 @@ def self.model_name end def process_data - logger.debug("LOGGING FROM data_export.rb process_data") @child_errors = [] data = [] data.concat(header_rows) scope.find_each do |object| - #begin + begin data << hash_to_row(object_data_as_hash(object)) - # rescue => e - # Rails.logger.error("Error for loan #{object.id} in data export #{self.name}: #{e}") - # - # # TODO generalize object beyond loan here and in task show if non-loans exported - # @child_errors << {loan_id: object.id, message: e.message} - # next - # end + rescue => e + Rails.logger.error("Error for loan #{object.id} in data export #{self.name}: #{e}") + + # TODO generalize object beyond loan here and in task show if non-loans exported + @child_errors << {loan_id: object.id, message: e.message} + next + end end self.update(data: data) @@ -78,23 +77,7 @@ def task protected def insert_in_row(header_symbol, row_array, value) - begin - row_array[header_symbols_to_indices[header_symbol]] = value - rescue => e - logger.info("mangoes yum") - logger.info e.message - logger.info "header symbol" - logger.info header_symbol - logger.info "header symbol nil? #{header_symbol.nil?}" - logger.info "Header Symbols" - logger.info header_symbols - logger.info "Header Symbols to indices" - logger.info header_symbols_to_indices - logger.info "index nil? #{header_symbols_to_indices[header_symbol].nil?}" - logger.info "value: #{value}" - logger.info "value nil? #{value.nil?}" - raise e - end + row_array[header_symbols_to_indices[header_symbol]] = value end private diff --git a/app/models/enhanced_loan_data_export.rb b/app/models/enhanced_loan_data_export.rb index b8e4940dd..d1d40291f 100644 --- a/app/models/enhanced_loan_data_export.rb +++ b/app/models/enhanced_loan_data_export.rb @@ -12,13 +12,8 @@ def response_hash(loan) response_sets = ResponseSet.joins(:question_set).where(loan: loan).order("question_sets.kind") response_sets.each do |response_set| response_set.answers.each do |a| - if !(header_symbols_to_indices.include?(a.question.id)) - logger.debug("########## #{a.question.id} not found ########") - end if q_data_types.include?(a.data_type) result[a.question_id] = a.answer_for_csv - else - logger.info "#{a.question_id} type #{a.data_type} not included" end end end @@ -66,7 +61,6 @@ def questions_by_id # Returns the list of symbols representing headers in the order they should appear. def header_symbols - puts @header_symbols @header_symbols ||= StandardLoanDataExport::HEADERS + questions.map(&:id) end end From 61377bf3fa985a5d9c754d5b9c005bb255ab4aee Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Wed, 4 May 2022 10:36:37 -0500 Subject: [PATCH 19/30] 12179: keep handling string numeric answers in data exports --- app/models/enhanced_loan_data_export.rb | 5 +++++ app/models/numeric_answer_data_export.rb | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/app/models/enhanced_loan_data_export.rb b/app/models/enhanced_loan_data_export.rb index d1d40291f..b6e0e343b 100644 --- a/app/models/enhanced_loan_data_export.rb +++ b/app/models/enhanced_loan_data_export.rb @@ -13,6 +13,7 @@ def response_hash(loan) response_sets.each do |response_set| response_set.answers.each do |a| if q_data_types.include?(a.data_type) + # TODO if keeping text-y numeric answers, need to check that here result[a.question_id] = a.answer_for_csv end end @@ -63,4 +64,8 @@ def questions_by_id def header_symbols @header_symbols ||= StandardLoanDataExport::HEADERS + questions.map(&:id) end + + def include_numeric_answer_in_export?(str) + true #include all numeric answers, even if invalid text + end end diff --git a/app/models/numeric_answer_data_export.rb b/app/models/numeric_answer_data_export.rb index 49f873a01..c5a523905 100644 --- a/app/models/numeric_answer_data_export.rb +++ b/app/models/numeric_answer_data_export.rb @@ -2,4 +2,8 @@ class NumericAnswerDataExport < EnhancedLoanDataExport def q_data_types ["number", "percentage", "rating", "currency", "range"] end + + def include_numeric_answer_in_export?(str) + true if Float(str) rescue false + end end From 7b7b9527ed128d0e4aa0a1547b1eb7ab16fa43bf Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Wed, 4 May 2022 16:38:20 -0500 Subject: [PATCH 20/30] 12179: create shared specs between enhanced and numeric data exports, support flag about whether to include text like numeric answers --- app/models/answer.rb | 8 +- app/models/enhanced_loan_data_export.rb | 5 +- app/models/numeric_answer_data_export.rb | 4 +- spec/models/enhanced_loan_data_export_spec.rb | 134 ---------- .../shared_examples_data_export_spec.rb | 230 ++++++++++++++++++ 5 files changed, 240 insertions(+), 141 deletions(-) create mode 100644 spec/models/shared_examples_data_export_spec.rb diff --git a/app/models/answer.rb b/app/models/answer.rb index e3840102c..0d1beb69f 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -161,14 +161,18 @@ def custom_data_json return {"#{self.question.json_key}": self.raw_value} end - def answer_for_csv + def answer_for_csv(allow_text_like_numeric: false) return nil if not_applicable case question.data_type when "text" text_data when "number", "currency", "percentage", "range" - numeric_data.to_s + if allow_text_like_numeric || (true if Float(numeric_data) rescue false) + numeric_data.to_s + else + nil + end when "boolean" boolean_data.nil? ? nil : (boolean_data ? "yes" : "no") # "breakeven" and "business_canvas" never exported to csv diff --git a/app/models/enhanced_loan_data_export.rb b/app/models/enhanced_loan_data_export.rb index b6e0e343b..c108915c8 100644 --- a/app/models/enhanced_loan_data_export.rb +++ b/app/models/enhanced_loan_data_export.rb @@ -13,8 +13,7 @@ def response_hash(loan) response_sets.each do |response_set| response_set.answers.each do |a| if q_data_types.include?(a.data_type) - # TODO if keeping text-y numeric answers, need to check that here - result[a.question_id] = a.answer_for_csv + result[a.question_id] = a.answer_for_csv(allow_text_like_numeric: allow_text_like_numeric?) end end end @@ -65,7 +64,7 @@ def header_symbols @header_symbols ||= StandardLoanDataExport::HEADERS + questions.map(&:id) end - def include_numeric_answer_in_export?(str) + def allow_text_like_numeric? true #include all numeric answers, even if invalid text end end diff --git a/app/models/numeric_answer_data_export.rb b/app/models/numeric_answer_data_export.rb index c5a523905..7d3b9193d 100644 --- a/app/models/numeric_answer_data_export.rb +++ b/app/models/numeric_answer_data_export.rb @@ -3,7 +3,7 @@ def q_data_types ["number", "percentage", "rating", "currency", "range"] end - def include_numeric_answer_in_export?(str) - true if Float(str) rescue false + def allow_text_like_numeric? + false end end diff --git a/spec/models/enhanced_loan_data_export_spec.rb b/spec/models/enhanced_loan_data_export_spec.rb index b921f374b..35214133c 100644 --- a/spec/models/enhanced_loan_data_export_spec.rb +++ b/spec/models/enhanced_loan_data_export_spec.rb @@ -1,139 +1,5 @@ require "rails_helper" describe EnhancedLoanDataExport, type: :model do - before do - OptionSetCreator.new.create_loan_status - end - describe "process_data" do - let(:division) { create(:division) } - let(:subdivision) { create(:division, parent: division) } - let!(:loan1) { create(:loan, :active, division: division) } - let!(:loan2) { create(:loan, :active, division: subdivision) } - let!(:loan3) { create(:loan, :active, division: division) } - - let(:base_headers) do - StandardLoanDataExport::HEADERS.map { |h| I18n.t("standard_loan_data_exports.headers.#{h}") } - end - let(:id_row_nils) { [nil] * (base_headers.size - 1) } - let(:response_data) { export.data[2..-1].map { |row| row[base_headers.size..-1] } } - - # Decoy question set, should not appear anywhere. - let(:decoy_division) { create(:division) } - let(:decoy_question_set) { create(:question_set, kind: "loan_criteria", division: decoy_division) } - let(:decoy_question_set) { create(:question_set, kind: "loan_criteria", division: decoy_division) } - let(:qdattrs) { {question_set: decoy_question_set, division: decoy_division} } - let!(:qd1) { create(:question, qdattrs.merge(data_type: "number", label: "QD1")) } - - let(:export) { create(:enhanced_loan_data_export, division: division, data: nil) } - - context "with criteria question set" do - let!(:criteria) { create(:question_set, kind: "loan_criteria", division: division) } - let(:qcattrs) { {question_set: criteria, division: division} } - let!(:qc1) { create(:question, qcattrs.merge(data_type: "boolean", label: "QC1")) } - let!(:qc2) { create(:question, qcattrs.merge(data_type: "percentage", label: "QC2")) } - let!(:qc3) { create(:question, qcattrs.merge(data_type: "text", label: "QC3")) } - let!(:qc4) { create(:question, qcattrs.merge(data_type: "range", label: "QC4")) } - - - # This question is on a subdivision, should still be included. - let!(:qc5) do - create(:question, :with_url, qcattrs.merge(division: subdivision, data_type: "range", label: "QC5")) - end - - let!(:r1_c) do - create(:response_set, - question_set: criteria, - loan: loan1) - end - let!(:r1_c_a1) { create(:answer, question: qc1, response_set: r1_c, boolean_data: true, not_applicable: false) } - let!(:r1_c_a2) { create(:answer, question: qc2, response_set: r1_c, numeric_data: 10, not_applicable: false) } - let!(:r1_c_a3) { create(:answer, question: qc3, response_set: r1_c, text_data: "foo\nbar", not_applicable: false) } - let!(:r1_c_a4) { create(:answer, question: qc4, response_set: r1_c, numeric_data: "4", text_data: "text", not_applicable: false) } - let!(:r2_c) do - create(:response_set, - question_set: criteria, - loan: loan2) - end - let!(:r2_c_a1) { create(:answer, question: qc1, response_set: r2_c, boolean_data: nil, not_applicable: true) } - let!(:r2_c_a2) { create(:answer, question: qc2, response_set: r2_c, numeric_data: 11, not_applicable: false) } - let!(:r2_c_a3) { create(:answer, question: qc3, response_set: r2_c, text_data: "lorp", not_applicable: false) } - let!(:r2_c_a4) { create(:answer, question: qc4, response_set: r2_c, numeric_data: nil, text_data: nil, not_applicable: true) } - let!(:r2_c_a5) { create(:answer, question: qc5, response_set: r2_c, numeric_data: 5, linked_document_data: {url: "https://example.com/1"}, not_applicable: false) } - - it "should create correct data attr" do - export.process_data - expect(export.data[0]).to eq(base_headers + ["QC1", "QC2", "QC3", "QC4", "QC5"]) - expect(export.data[1]).to eq(["Question ID"] + id_row_nils + [qc1, qc2, qc3, qc4, qc5].map(&:id)) - - row1 = ["yes", "10", "foo\nbar", "4"] - row2 = [nil, "11", "lorp", nil, "5"] - row3 = [] - expect(response_data).to contain_exactly(row1, row2, row3) - end - - context "with criteria and post_analysis" do - let!(:post_analysis) { create(:question_set, kind: "loan_post_analysis", division: division) } - let(:qpattrs) { {question_set: post_analysis, division: division} } - let!(:qp1) { create(:question, :with_url, qpattrs.merge(data_type: "number", label: "QP1")) } - let!(:r1_p) do - create(:response_set, - question_set: post_analysis, - loan: loan1) - end - let!(:r1_p_q1) { create(:answer, question: qp1, response_set: r1_p, numeric_data: 7, not_applicable: false)} - let!(:r3_p) do - create(:response_set, - question_set: post_analysis, - loan: loan3) - end - let!(:r3_p_q1) { create(:answer, question: qp1, response_set: r3_p, numeric_data: 99.9, not_applicable: false)} - - it "should create correct data attr" do - export.process_data - expect(export.data[0]).to eq(base_headers + ["QC1", "QC2", "QC3", "QC4", "QC5", "QP1"]) - expect(export.data[1]).to eq(["Question ID"] + id_row_nils + [qc1, qc2, qc3, qc4, qc5, qp1].map(&:id)) - - row1 = ["yes", "10", "foo\nbar", "4", nil, "7"] - row2 = [nil, "11", "lorp", nil, "5"] - row3 = [nil, nil, nil, nil, nil, "99.9"] - expect(response_data).to contain_exactly(row1, row2, row3) - end - end - end - - context "when subdivision has own question set" do - let!(:setA) { create(:question_set, kind: "loan_criteria", division: division) } - let(:qaattrs) { {question_set: setA, division: division} } - let!(:qa1) { create(:question, qaattrs.merge(data_type: "number", label: "QA1")) } - let!(:qa2) { create(:question, qaattrs.merge(data_type: "number", label: "QA2")) } - - let!(:setB) { create(:question_set, kind: "loan_criteria", division: subdivision) } - let(:qbattrs) { {question_set: setB, division: subdivision} } - let!(:qb1) { create(:question, qbattrs.merge(data_type: "number", label: "QB1")) } - - let!(:r1) do - create(:response_set, - question_set: setA, - loan: loan1) - end - let!(:r1_a1) { create(:answer, question: qa1, response_set: r1, numeric_data: 5, not_applicable: false) } - let!(:r1_a2) { create(:answer, question: qa2, response_set: r1, numeric_data: 10, not_applicable: false) } - let!(:r2) do - create(:response_set, - question_set: setB, - loan: loan2) - end - let!(:r2_a1) { create(:answer, question: qb1, response_set: r2, numeric_data: 15, not_applicable: false) } - it "should include data from both questions sets" do - export.process_data - expect(export.data[0]).to eq(base_headers + ["QA1", "QA2", "QB1"]) - expect(export.data[1]).to eq(["Question ID"] + id_row_nils + [qa1, qa2, qb1].map(&:id)) - row1 = ["5", "10"] - row2 = [nil, nil, "15"] - row3 = [] - expect(response_data).to contain_exactly(row1, row2, row3) - end - end - end end diff --git a/spec/models/shared_examples_data_export_spec.rb b/spec/models/shared_examples_data_export_spec.rb new file mode 100644 index 000000000..d8053aec4 --- /dev/null +++ b/spec/models/shared_examples_data_export_spec.rb @@ -0,0 +1,230 @@ +require "rails_helper" + +describe "data exports with questionnaire data" do + let(:division) { create(:division) } + let(:subdivision) { create(:division, parent: division) } + let!(:loan1) { create(:loan, :active, division: division) } + let!(:loan2) { create(:loan, :active, division: subdivision) } + let!(:loan3) { create(:loan, :active, division: division) } + + let(:base_headers) do + StandardLoanDataExport::HEADERS.map { |h| I18n.t("standard_loan_data_exports.headers.#{h}") } + end + let(:id_row_nils) { [nil] * (base_headers.size - 1) } + let(:response_data) { export.data[2..-1].map { |row| row[base_headers.size..-1] } } + + # Decoy question set, should not appear anywhere. + let(:decoy_division) { create(:division) } + let(:decoy_question_set) { create(:question_set, kind: "loan_criteria", division: decoy_division) } + let(:decoy_question_set) { create(:question_set, kind: "loan_criteria", division: decoy_division) } + let(:qdattrs) { {question_set: decoy_question_set, division: decoy_division} } + let!(:qd1) { create(:question, qdattrs.merge(data_type: "number", label: "QD1")) } + + shared_examples_for "a data export with questionnaire data" do + before do + OptionSetCreator.new.create_loan_status + end + + describe "process_data" do + + context "with criteria question set" do + let!(:criteria) { create(:question_set, kind: "loan_criteria", division: division) } + let(:qcattrs) { {question_set: criteria, division: division} } + let!(:qc1) { create(:question, qcattrs.merge(data_type: "number", label: "QC1")) } + let!(:qc2) { create(:question, qcattrs.merge(data_type: "percentage", label: "QC2")) } + let!(:qc3) { create(:question, qcattrs.merge(data_type: "currency", label: "QC3")) } + let!(:qc4) { create(:question, qcattrs.merge(data_type: "range", label: "QC4")) } + + + # This question is on a subdivision, should still be included. + let!(:qc5) do + create(:question, :with_url, qcattrs.merge(division: subdivision, data_type: "range", label: "QC5")) + end + + let!(:r1_c) do + create(:response_set, + question_set: criteria, + loan: loan1) + end + let!(:r1_c_a1) { create(:answer, question: qc1, response_set: r1_c, numeric_data: 2, not_applicable: false) } + let!(:r1_c_a2) { create(:answer, question: qc2, response_set: r1_c, numeric_data: 4, not_applicable: false) } + let!(:r1_c_a3) { create(:answer, question: qc3, response_set: r1_c, numeric_data: 6, not_applicable: false) } + let!(:r1_c_a4) { create(:answer, question: qc4, response_set: r1_c, numeric_data: 8, text_data: "text", not_applicable: false) } + let!(:r2_c) do + create(:response_set, + question_set: criteria, + loan: loan2) + end + let!(:r2_c_a1) { create(:answer, question: qc1, response_set: r2_c, numeric_data: 1, not_applicable: true) } + let!(:r2_c_a2) { create(:answer, question: qc2, response_set: r2_c, numeric_data: 3, not_applicable: false) } + let!(:r2_c_a3) { create(:answer, question: qc3, response_set: r2_c, numeric_data: 5, not_applicable: false) } + let!(:r2_c_a4) { create(:answer, question: qc4, response_set: r2_c, numeric_data: nil, text_data: nil, not_applicable: true) } + let!(:r2_c_a5) { create(:answer, question: qc5, response_set: r2_c, numeric_data: 9, linked_document_data: {url: "https://example.com/1"}, not_applicable: false) } + + it "should create correct data attr" do + export.process_data + expect(export.data[0]).to eq(base_headers + ["QC1", "QC2", "QC3", "QC4", "QC5"]) + expect(export.data[1]).to eq(["Question ID"] + id_row_nils + [qc1, qc2, qc3, qc4, qc5].map(&:id)) + + row1 = ["2", "4", "6", "8"] + row2 = [nil, "3", "5", nil, "9"] + row3 = [] + expect(response_data).to contain_exactly(row1, row2, row3) + end + + context "with criteria and post_analysis" do + let!(:post_analysis) { create(:question_set, kind: "loan_post_analysis", division: division) } + let(:qpattrs) { {question_set: post_analysis, division: division} } + let!(:qp1) { create(:question, :with_url, qpattrs.merge(data_type: "number", label: "QP1")) } + let!(:r1_p) do + create(:response_set, + question_set: post_analysis, + loan: loan1) + end + let!(:r1_p_q1) { create(:answer, question: qp1, response_set: r1_p, numeric_data: 10, not_applicable: false)} + let!(:r3_p) do + create(:response_set, + question_set: post_analysis, + loan: loan3) + end + let!(:r3_p_q1) { create(:answer, question: qp1, response_set: r3_p, numeric_data: 99.9, not_applicable: false)} + + it "should create correct data attr" do + export.process_data + expect(export.data[0]).to eq(base_headers + ["QC1", "QC2", "QC3", "QC4", "QC5", "QP1"]) + expect(export.data[1]).to eq(["Question ID"] + id_row_nils + [qc1, qc2, qc3, qc4, qc5, qp1].map(&:id)) + + row1 = ["2", "4", "6", "8", nil, "10"] + row2 = [nil, "3", "5", nil, "9"] + row3 = [nil, nil, nil, nil, nil, "99.9"] + expect(response_data).to contain_exactly(row1, row2, row3) + end + end + end + + context "when subdivision has own question set" do + let!(:setA) { create(:question_set, kind: "loan_criteria", division: division) } + let(:qaattrs) { {question_set: setA, division: division} } + let!(:qa1) { create(:question, qaattrs.merge(data_type: "number", label: "QA1")) } + let!(:qa2) { create(:question, qaattrs.merge(data_type: "number", label: "QA2")) } + + let!(:setB) { create(:question_set, kind: "loan_criteria", division: subdivision) } + let(:qbattrs) { {question_set: setB, division: subdivision} } + let!(:qb1) { create(:question, qbattrs.merge(data_type: "number", label: "QB1")) } + + let!(:r1) do + create(:response_set, + question_set: setA, + loan: loan1) + end + let!(:r1_a1) { create(:answer, question: qa1, response_set: r1, numeric_data: 5, not_applicable: false) } + let!(:r1_a2) { create(:answer, question: qa2, response_set: r1, numeric_data: 10, not_applicable: false) } + let!(:r2) do + create(:response_set, + question_set: setB, + loan: loan2) + end + let!(:r2_a1) { create(:answer, question: qb1, response_set: r2, numeric_data: 15, not_applicable: false) } + it "should include data from both questions sets" do + export.process_data + expect(export.data[0]).to eq(base_headers + ["QA1", "QA2", "QB1"]) + expect(export.data[1]).to eq(["Question ID"] + id_row_nils + [qa1, qa2, qb1].map(&:id)) + row1 = ["5", "10"] + row2 = [nil, nil, "15"] + row3 = [] + expect(response_data).to contain_exactly(row1, row2, row3) + end + end + end + end + + describe EnhancedLoanDataExport do + let(:export) { create(:enhanced_loan_data_export, division: division, data: nil) } + it_behaves_like "a data export with questionnaire data" + + context "there are text and boolean questions, and text-like numeric answers" do + let!(:criteria) { create(:question_set, kind: "loan_criteria", division: division) } + let(:qcattrs) { {question_set: criteria, division: division} } + let!(:qc1) { create(:question, qcattrs.merge(data_type: "boolean", label: "QC1")) } + let!(:qc2) { create(:question, qcattrs.merge(data_type: "text", label: "QC2")) } + let!(:qc3) { create(:question, qcattrs.merge(data_type: "currency", label: "QC3")) } + let!(:qc4) { create(:question, qcattrs.merge(data_type: "range", label: "QC4")) } + + + # This question is on a subdivision, should still be included. + let!(:qc5) do + create(:question, :with_url, qcattrs.merge(division: subdivision, data_type: "text", label: "QC5")) + end + + let!(:r1_c) do + create(:response_set, + question_set: criteria, + loan: loan1) + end + let!(:r1_c_a1) { create(:answer, question: qc1, response_set: r1_c, boolean_data: true, not_applicable: false) } + let!(:r1_c_a2) { create(:answer, question: qc2, response_set: r1_c, text_data: "boo", not_applicable: false) } + let!(:r1_c_a3) { create(:answer, question: qc3, response_set: r1_c, numeric_data: "six", not_applicable: false) } + let!(:r1_c_a4) { create(:answer, question: qc4, response_set: r1_c, numeric_data: 8, text_data: "text", not_applicable: false) } + let!(:r2_c) do + create(:response_set, + question_set: criteria, + loan: loan2) + end + let!(:r2_c_a1) { create(:answer, question: qc1, response_set: r2_c, boolean_data: false, not_applicable: false) } + let!(:r2_c_a2) { create(:answer, question: qc2, response_set: r2_c, text_data: "hey", not_applicable: true) } + let!(:r2_c_a3) { create(:answer, question: qc3, response_set: r2_c, numeric_data: 5, not_applicable: false) } + let!(:r2_c_a4) { create(:answer, question: qc4, response_set: r2_c, numeric_data: 10, text_data: "berry", not_applicable: true) } + let!(:r2_c_a5) { create(:answer, question: qc5, response_set: r2_c, text_data: "apple", linked_document_data: {url: "https://example.com/1"}, not_applicable: false) } + + it "should create correct data attr" do + export.process_data + expect(export.data[0]).to eq(base_headers + ["QC1", "QC2", "QC3", "QC4", "QC5"]) + expect(export.data[1]).to eq(["Question ID"] + id_row_nils + [qc1, qc2, qc3, qc4, qc5].map(&:id)) + + row1 = ["yes", "boo", "six", "8"] + row2 = ["no", nil, "5", nil, "apple"] + row3 = [] + expect(response_data).to contain_exactly(row1, row2, row3) + end + end + end + + describe NumericAnswerDataExport do + let(:export) { create(:numeric_answer_data_export, division: division, data: nil) } + it_behaves_like "a data export with questionnaire data" + + context "there are text-like numeric answers" do + let!(:criteria) { create(:question_set, kind: "loan_criteria", division: division) } + let(:qcattrs) { {question_set: criteria, division: division} } + let!(:qc1) { create(:question, qcattrs.merge(data_type: "number", label: "QC1")) } + let!(:qc2) { create(:question, qcattrs.merge(data_type: "currency", label: "QC2")) } + + let!(:r1_c) do + create(:response_set, + question_set: criteria, + loan: loan1) + end + let!(:r1_c_a1) { create(:answer, question: qc1, response_set: r1_c, numeric_data: 2, not_applicable: false) } + let!(:r1_c_a2) { create(:answer, question: qc2, response_set: r1_c, numeric_data: "four", not_applicable: false) } + + let!(:r2_c) do + create(:response_set, + question_set: criteria, + loan: loan2) + end + let!(:r2_c_a1) { create(:answer, question: qc1, response_set: r2_c, numeric_data: 3, not_applicable: true) } + let!(:r2_c_a2) { create(:answer, question: qc2, response_set: r2_c, numeric_data: 5, not_applicable: false) } + + it "should create correct data attr" do + export.process_data + expect(export.data[0]).to eq(base_headers + ["QC1", "QC2"]) + expect(export.data[1]).to eq(["Question ID"] + id_row_nils + [qc1, qc2].map(&:id)) + + row1 = ["2", nil] + row2 = [nil, "5"] + row3 = [] + expect(response_data).to contain_exactly(row1, row2, row3) + end + end + end +end From b76367e9f6067848e11bfce9bd115ff74a49c48b Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Fri, 6 May 2022 18:38:24 -0500 Subject: [PATCH 21/30] 12179: WIP specs breaking because internal names not unique, which is a problem that needs to be fixed next. pause here to try changing form to use question id. --- .../admin/response_sets_controller.rb | 5 ++++ app/models/answer.rb | 6 +++-- app/models/response_set.rb | 8 ++++++ .../loans/questionnaires/_answer.html.slim | 4 +++ spec/factories/question_sets_factory.rb | 6 ++--- spec/factories/questions_factory.rb | 2 +- spec/system/admin/questionnaire_spec.rb | 27 ++++++++----------- 7 files changed, 35 insertions(+), 23 deletions(-) diff --git a/app/controllers/admin/response_sets_controller.rb b/app/controllers/admin/response_sets_controller.rb index 1f0ff9bb2..6d96eefd2 100644 --- a/app/controllers/admin/response_sets_controller.rb +++ b/app/controllers/admin/response_sets_controller.rb @@ -2,6 +2,8 @@ class Admin::ResponseSetsController < Admin::AdminController include QuestionnaireRenderable def create + puts "in create: " + puts response_set_params @response_set = ResponseSet.new @response_set.assign_attributes(response_set_params.merge(updater_id: current_user.id)) authorize @response_set @@ -10,6 +12,7 @@ def create @conflicting_response_set = ResponseSet.find_by(loan_id: @response_set.loan_id, question_set_id: @response_set.question_set_id) if @conflicting_response_set + puts "conflicting response set" @response_set_from_db = { updater: @conflicting_response_set.updater, updated_at: @conflicting_response_set.updated_at, @@ -17,7 +20,9 @@ def create } handle_conflict else + puts "should save answers next" @response_set.save! + @response_set.save_answers(response_set_params) redirect_to display_path, notice: I18n.t(:notice_created) end end diff --git a/app/models/answer.rb b/app/models/answer.rb index 0d1beb69f..6bd063662 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -21,7 +21,7 @@ def compare_to_custom_data end def to_s - "NA: #{not_applicable}; text: #{text_data}; numeric: #{numeric_data}; boolean: #{boolean_data}; doc: #{linked_document_data}; breakeven: #{breakeven_data}; canvas: #{business_canvas_data}" + "RS: #{response_set.question_set.kind}, Q: #{question.label.to_s} | NA: #{not_applicable}; text: #{text_data}; numeric: #{numeric_data}; boolean: #{boolean_data}; doc: #{linked_document_data}; breakeven: #{breakeven_data}; canvas: #{business_canvas_data}" end def blank? @@ -79,6 +79,8 @@ def self.contains_answer_data?(hash_data) # this method is temporary for spr 2022 overhaul # doesn't save blank answers def self.save_from_form_field_params(question, fields, response_set) + puts "has number data? #{fields.key?("number")}" + puts fields["number"] unless question.group? || !self.contains_answer_data?(fields) not_applicable = fields.key?("not_applicable") ? (fields["not_applicable"] == "yes") : "no" text_data = fields.key?("text") ? fields["text"] : nil @@ -151,7 +153,7 @@ def raw_value json["start_cell"] = self.linked_document_data["start_cell"] json["end_cell"] = self.linked_document_data["end_cell"] end - #puts json + puts json json end diff --git a/app/models/response_set.rb b/app/models/response_set.rb index 4de028042..745cdd726 100644 --- a/app/models/response_set.rb +++ b/app/models/response_set.rb @@ -34,9 +34,15 @@ def root_response # Fetches a custom value from the json field. # Ensures `question` is decorated before passing to Response. def response(question) + puts "QUestion in response(question) in response set" + puts question + puts "#" question = ensure_decorated(question) answer = Answer.find_by(response_set: self, question: question) + puts answer.nil? + Answer.all.each{|a| puts a.to_s} raw_value = answer.present? ? answer.raw_value : nil + puts "raw value for response made from question:" Response.new(loan: loan, question: question, response_set: self, data: raw_value) end @@ -56,6 +62,8 @@ def ensure_all_answers_copied end def save_answers(form_hash) + puts "save answers in response set model with:" + puts form_hash form_hash.each do |key, value| if key.include?("field") # key is an internal_name of a question question = Question.find_by(internal_name: key) diff --git a/app/views/admin/loans/questionnaires/_answer.html.slim b/app/views/admin/loans/questionnaires/_answer.html.slim index 6132add1c..d661999f4 100644 --- a/app/views/admin/loans/questionnaires/_answer.html.slim +++ b/app/views/admin/loans/questionnaires/_answer.html.slim @@ -26,7 +26,11 @@ data: {rating: true} - if response.has_number? + - puts "response has number" + - puts response.number - if response.number.present? && !response.not_applicable? + - puts "response has number and is applicable" + - puts display_value_for_number(response) .view-element.answer.hidden-print = display_value_for_number(response) .view-element.answer.numerical-answer.visible-print-block table.table class=(@previous_number_table ? 'sibling-table' : '') diff --git a/spec/factories/question_sets_factory.rb b/spec/factories/question_sets_factory.rb index b942dae26..c70acb647 100644 --- a/spec/factories/question_sets_factory.rb +++ b/spec/factories/question_sets_factory.rb @@ -6,10 +6,8 @@ trait :with_questions do kind { 'loan_criteria' } after(:create) do |model| - create(:question, division: model.division, parent: model.root_group, question_set: model, - internal_name: 'summary', data_type: 'number') - create(:question, division: model.division, parent: model.root_group, question_set: model, - internal_name: 'workers', data_type: 'text') + create(:question, division: model.division, parent: model.root_group, question_set: model, data_type: 'number') + create(:question, division: model.division, parent: model.root_group, question_set: model, data_type: 'text') end end end diff --git a/spec/factories/questions_factory.rb b/spec/factories/questions_factory.rb index 81666e7bf..82892f023 100644 --- a/spec/factories/questions_factory.rb +++ b/spec/factories/questions_factory.rb @@ -6,7 +6,7 @@ division { root_division } question_set - internal_name { Faker::Lorem.words(2).join('_').downcase } + internal_name { "field_#{id}" } data_type { Question::DATA_TYPES.sample } active { true } diff --git a/spec/system/admin/questionnaire_spec.rb b/spec/system/admin/questionnaire_spec.rb index bdf897a4a..460fad1ae 100644 --- a/spec/system/admin/questionnaire_spec.rb +++ b/spec/system/admin/questionnaire_spec.rb @@ -37,6 +37,13 @@ # save changes button is visible in edit mode fill_and_save("1337") + puts "internal_names" + Question.all.each{|q| puts q.internal_name} + puts loan.response_sets.count + puts loan.response_sets.first.answers.count + expect(loan.response_sets.first.reload.answers.first.numeric_data).to eq "1337" + expect(page).not_to have_content("Now editing") + expect(page).to have_content("1,337", wait: 20) expect(page).to have_css(".view-element", text: "1,337") # test outline expansion @@ -55,7 +62,10 @@ field.set("31337") find("body").click click_button("Save Changes") - expect(page).to have_css(".view-element", text: "31,337") + save_and_open_page + click_on("Post-Analysis") + expect(loan.response_sets.last.answers.first.reload.numeric_data).to eq "31337" + expect(page).to have_css(".view-element", text: "31,337", wait: 20) end end @@ -77,21 +87,6 @@ end end - context "refactoring to answers table" do - let!(:question_set) { create(:question_set, kind: "loan_criteria", division: division)} - let!(:root_q) {create(:question, data_type: "group", question_set: question_set, division: division)} - let!(:questions) { {} } - before do - Question::DATA_TYPES.each do |t| - questions[t] = Question.create(data_type: t, question_set: question_set, parent: root_q, division: division) - end - end - end - - def fill_qtype_with_value(type, value) - find("input[type=#{type}]").set(value) - end - def fill_and_save(value) first(".edit-all").click expect(page).to have_content("Now editing") From ab44b97c2437a61d803aa9c4e34c2060f63a36c2 Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Mon, 9 May 2022 14:10:03 -0500 Subject: [PATCH 22/30] 12179: specs passing after addressing problem with question factory creating "field_" internal names before id is available --- app/models/response_set.rb | 1 + spec/factories/questions_factory.rb | 1 - spec/models/enhanced_loan_data_export_spec.rb | 5 - .../models/numeric_answer_data_export_spec.rb | 113 ------------------ 4 files changed, 1 insertion(+), 119 deletions(-) delete mode 100644 spec/models/enhanced_loan_data_export_spec.rb delete mode 100644 spec/models/numeric_answer_data_export_spec.rb diff --git a/app/models/response_set.rb b/app/models/response_set.rb index 745cdd726..bcca0e01f 100644 --- a/app/models/response_set.rb +++ b/app/models/response_set.rb @@ -67,6 +67,7 @@ def save_answers(form_hash) form_hash.each do |key, value| if key.include?("field") # key is an internal_name of a question question = Question.find_by(internal_name: key) + raise "Unrecognized question identifier: #{key} in #{form_hash}" if question.nil? Answer.save_from_form_field_params(question, value, self) end end diff --git a/spec/factories/questions_factory.rb b/spec/factories/questions_factory.rb index 82892f023..cfbdccf72 100644 --- a/spec/factories/questions_factory.rb +++ b/spec/factories/questions_factory.rb @@ -6,7 +6,6 @@ division { root_division } question_set - internal_name { "field_#{id}" } data_type { Question::DATA_TYPES.sample } active { true } diff --git a/spec/models/enhanced_loan_data_export_spec.rb b/spec/models/enhanced_loan_data_export_spec.rb deleted file mode 100644 index 35214133c..000000000 --- a/spec/models/enhanced_loan_data_export_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "rails_helper" - -describe EnhancedLoanDataExport, type: :model do - -end diff --git a/spec/models/numeric_answer_data_export_spec.rb b/spec/models/numeric_answer_data_export_spec.rb deleted file mode 100644 index 1d421d904..000000000 --- a/spec/models/numeric_answer_data_export_spec.rb +++ /dev/null @@ -1,113 +0,0 @@ -require "rails_helper" - -describe NumericAnswerDataExport, type: :model do - before do - OptionSetCreator.new.create_loan_status - end - - describe "process_data" do - let(:division) { create(:division) } - let(:subdivision) { create(:division, parent: division) } - let!(:loan1) { create(:loan, :active, division: division) } - let!(:loan2) { create(:loan, :active, division: subdivision) } - let!(:loan3) { create(:loan, :active, division: division) } - - let(:base_headers) do - StandardLoanDataExport::HEADERS.map { |h| I18n.t("standard_loan_data_exports.headers.#{h}") } - end - let(:id_row_nils) { [nil] * (base_headers.size - 1) } - let(:response_data) { export.data[2..-1].map { |row| row[base_headers.size..-1] } } - - # Decoy question set, should not appear anywhere. - let(:decoy_division) { create(:division) } - let(:decoy_question_set) { create(:question_set, kind: "loan_criteria", division: decoy_division) } - let(:decoy_question_set) { create(:question_set, kind: "loan_criteria", division: decoy_division) } - let(:qdattrs) { {question_set: decoy_question_set, division: decoy_division} } - let!(:qd1) { create(:question, qdattrs.merge(data_type: "number", label: "QD1")) } - - let(:export) { create(:numeric_answer_data_export, division: division, data: nil) } - - context "with criteria question set" do - let!(:criteria) { create(:question_set, kind: "loan_criteria", division: division) } - let(:qcattrs) { {question_set: criteria, division: division} } - let!(:qc1) { create(:question, qcattrs.merge(data_type: "boolean", label: "QC1")) } - let!(:qc2) { create(:question, qcattrs.merge(data_type: "percentage", label: "QC2")) } - let!(:qc3) { create(:question, qcattrs.merge(data_type: "text", label: "QC3")) } - let!(:qc4) { create(:question, qcattrs.merge(data_type: "range", label: "QC4")) } - - - # This question is on a subdivision, should still be included. - let!(:qc5) do - create(:question, :with_url, qcattrs.merge(division: subdivision, data_type: "rating", label: "QC5")) - end - - let!(:r1_c) do - create(:response_set, - question_set: criteria, - loan: loan1, - custom_data: { - qc1.id.to_s => {boolean: "yes", not_applicable: "no"}, - qc2.id.to_s => {number: "10", not_applicable: "no"}, - qc3.id.to_s => {text: "foo\nbar", not_applicable: "no"}, - qc4.id.to_s => {rating: "4", text: "text", not_applicable: "no"} - }) - end - let!(:r2_c) do - create(:response_set, - question_set: criteria, - loan: loan2, - custom_data: { - qc1.id.to_s => {boolean: "", not_applicable: "yes"}, - qc2.id.to_s => {number: "invalid text in number answer", not_applicable: "no"}, - qc3.id.to_s => {text: "lorp", not_applicable: "no"}, - qc4.id.to_s => {rating: nil, text: nil, not_applicable: "yes"}, - qc5.id.to_s => {rating: "5", url: "https://example.com/1", not_applicable: "no"} - }) - end - - it "should create correct data attr" do - export.process_data - expect(export.data[0]).to eq(base_headers + ["QC2", "QC4", "QC5"]) - expect(export.data[1]).to eq(["Question ID"] + id_row_nils + [qc2, qc4, qc5].map(&:id)) - - row1 = ["10", "4"] - row2 = [ "", "", "5"] - row3 = [] - expect(response_data).to contain_exactly(row1, row2, row3) - end - - context "with criteria and post_analysis" do - let!(:post_analysis) { create(:question_set, kind: "loan_post_analysis", division: division) } - let(:qpattrs) { {question_set: post_analysis, division: division} } - let!(:qp1) { create(:question, :with_url, qpattrs.merge(data_type: "number", label: "QP1")) } - let!(:r1_p) do - create(:response_set, - question_set: post_analysis, - loan: loan1, - custom_data: { - qp1.id.to_s => {number: "7", not_applicable: "no"} - }) - end - let!(:r3_p) do - create(:response_set, - question_set: post_analysis, - loan: loan3, - custom_data: { - qp1.id.to_s => {number: "99.9", not_applicable: "no"} - }) - end - - it "should create correct data attr" do - export.process_data - expect(export.data[0]).to eq(base_headers + [ "QC2", "QC4", "QC5", "QP1"]) - expect(export.data[1]).to eq(["Question ID"] + id_row_nils + [ qc2, qc4, qc5, qp1].map(&:id)) - - row1 = ["10", "4", nil, "7"] - row2 = ["", "", "5"] - row3 = [nil, nil, nil, "99.9"] - expect(response_data).to contain_exactly(row1, row2, row3) - end - end - end - end -end From c8bdab4bdd595d1c276eea84abd7d4534808e9df Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Mon, 9 May 2022 17:06:09 -0500 Subject: [PATCH 23/30] 12179: clean up debug code, specs passing and manual tests passing --- app/controllers/admin/response_sets_controller.rb | 4 ---- app/models/answer.rb | 6 +----- app/models/option.rb | 2 -- app/models/response_set.rb | 12 ------------ .../admin/loans/questionnaires/_answer.html.slim | 4 ---- spec/models/loan_spec.rb | 1 - spec/system/admin/questionnaire_spec.rb | 5 ----- 7 files changed, 1 insertion(+), 33 deletions(-) diff --git a/app/controllers/admin/response_sets_controller.rb b/app/controllers/admin/response_sets_controller.rb index 6d96eefd2..1c418f1c4 100644 --- a/app/controllers/admin/response_sets_controller.rb +++ b/app/controllers/admin/response_sets_controller.rb @@ -2,8 +2,6 @@ class Admin::ResponseSetsController < Admin::AdminController include QuestionnaireRenderable def create - puts "in create: " - puts response_set_params @response_set = ResponseSet.new @response_set.assign_attributes(response_set_params.merge(updater_id: current_user.id)) authorize @response_set @@ -12,7 +10,6 @@ def create @conflicting_response_set = ResponseSet.find_by(loan_id: @response_set.loan_id, question_set_id: @response_set.question_set_id) if @conflicting_response_set - puts "conflicting response set" @response_set_from_db = { updater: @conflicting_response_set.updater, updated_at: @conflicting_response_set.updated_at, @@ -20,7 +17,6 @@ def create } handle_conflict else - puts "should save answers next" @response_set.save! @response_set.save_answers(response_set_params) redirect_to display_path, notice: I18n.t(:notice_created) diff --git a/app/models/answer.rb b/app/models/answer.rb index 6bd063662..fdf009eeb 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -25,8 +25,7 @@ def to_s end def blank? - puts self.to_s - #!not_applicable? && + !not_applicable? && text_data.blank? && numeric_data.blank? && boolean_data.blank? && @@ -79,8 +78,6 @@ def self.contains_answer_data?(hash_data) # this method is temporary for spr 2022 overhaul # doesn't save blank answers def self.save_from_form_field_params(question, fields, response_set) - puts "has number data? #{fields.key?("number")}" - puts fields["number"] unless question.group? || !self.contains_answer_data?(fields) not_applicable = fields.key?("not_applicable") ? (fields["not_applicable"] == "yes") : "no" text_data = fields.key?("text") ? fields["text"] : nil @@ -153,7 +150,6 @@ def raw_value json["start_cell"] = self.linked_document_data["start_cell"] json["end_cell"] = self.linked_document_data["end_cell"] end - puts json json end diff --git a/app/models/option.rb b/app/models/option.rb index 5e49367d0..bab174d61 100644 --- a/app/models/option.rb +++ b/app/models/option.rb @@ -17,8 +17,6 @@ class Option < ApplicationRecord def ensure_value_assigned unless value - # puts "defaulting value to id: #{self.id}" - # make sure we don't have any recursive callbacks self.update_column(:value, self.id) end end diff --git a/app/models/response_set.rb b/app/models/response_set.rb index bcca0e01f..6b748c244 100644 --- a/app/models/response_set.rb +++ b/app/models/response_set.rb @@ -34,15 +34,9 @@ def root_response # Fetches a custom value from the json field. # Ensures `question` is decorated before passing to Response. def response(question) - puts "QUestion in response(question) in response set" - puts question - puts "#" question = ensure_decorated(question) answer = Answer.find_by(response_set: self, question: question) - puts answer.nil? - Answer.all.each{|a| puts a.to_s} raw_value = answer.present? ? answer.raw_value : nil - puts "raw value for response made from question:" Response.new(loan: loan, question: question, response_set: self, data: raw_value) end @@ -62,8 +56,6 @@ def ensure_all_answers_copied end def save_answers(form_hash) - puts "save answers in response set model with:" - puts form_hash form_hash.each do |key, value| if key.include?("field") # key is an internal_name of a question question = Question.find_by(internal_name: key) @@ -150,11 +142,7 @@ def method_missing(method_sym, *arguments, &block) # we then convert from internal name to the question id in set_response. q = question(attribute_name) case action - when :get - puts "GET in method missing with #{q}" - #return response(q) when :set - puts "SET in method missing with #{q}" return set_response(q, arguments.first) end end diff --git a/app/views/admin/loans/questionnaires/_answer.html.slim b/app/views/admin/loans/questionnaires/_answer.html.slim index d661999f4..6132add1c 100644 --- a/app/views/admin/loans/questionnaires/_answer.html.slim +++ b/app/views/admin/loans/questionnaires/_answer.html.slim @@ -26,11 +26,7 @@ data: {rating: true} - if response.has_number? - - puts "response has number" - - puts response.number - if response.number.present? && !response.not_applicable? - - puts "response has number and is applicable" - - puts display_value_for_number(response) .view-element.answer.hidden-print = display_value_for_number(response) .view-element.answer.numerical-answer.visible-print-block table.table class=(@previous_number_table ? 'sibling-table' : '') diff --git a/spec/models/loan_spec.rb b/spec/models/loan_spec.rb index 16cb67aed..2cc144b98 100644 --- a/spec/models/loan_spec.rb +++ b/spec/models/loan_spec.rb @@ -153,7 +153,6 @@ end xit 'it should return all future events and past events if they are complete or have logs' do - # puts project_steps.awesome_inspect events = loan.project_steps expect(events.size).to eq 8 events.each do |event| diff --git a/spec/system/admin/questionnaire_spec.rb b/spec/system/admin/questionnaire_spec.rb index 460fad1ae..64942a554 100644 --- a/spec/system/admin/questionnaire_spec.rb +++ b/spec/system/admin/questionnaire_spec.rb @@ -37,10 +37,6 @@ # save changes button is visible in edit mode fill_and_save("1337") - puts "internal_names" - Question.all.each{|q| puts q.internal_name} - puts loan.response_sets.count - puts loan.response_sets.first.answers.count expect(loan.response_sets.first.reload.answers.first.numeric_data).to eq "1337" expect(page).not_to have_content("Now editing") expect(page).to have_content("1,337", wait: 20) @@ -62,7 +58,6 @@ field.set("31337") find("body").click click_button("Save Changes") - save_and_open_page click_on("Post-Analysis") expect(loan.response_sets.last.answers.first.reload.numeric_data).to eq "31337" expect(page).to have_css(".view-element", text: "31,337", wait: 20) From c6d7571fef26fb71f1c5cb9cd70bd65e1776444c Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Tue, 10 May 2022 15:24:40 -0500 Subject: [PATCH 24/30] 12179: fix problem where data exports did not support exporting from divisions with inherited question sets --- app/models/data_export.rb | 7 +++- app/models/enhanced_loan_data_export.rb | 7 ++-- .../shared_examples_data_export_spec.rb | 40 +++++++++++++++++++ 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/app/models/data_export.rb b/app/models/data_export.rb index 9e651e4f1..f21e1e5aa 100644 --- a/app/models/data_export.rb +++ b/app/models/data_export.rb @@ -77,7 +77,12 @@ def task protected def insert_in_row(header_symbol, row_array, value) - row_array[header_symbols_to_indices[header_symbol]] = value + index = header_symbols_to_indices[header_symbol] + if index.nil? + raise "Header symbol #{header_symbol} not found in #{header_symbols_to_indices}. Failed to add #{value}" + else + row_array[index] = value + end end private diff --git a/app/models/enhanced_loan_data_export.rb b/app/models/enhanced_loan_data_export.rb index c108915c8..1e8211b2c 100644 --- a/app/models/enhanced_loan_data_export.rb +++ b/app/models/enhanced_loan_data_export.rb @@ -50,9 +50,10 @@ def questions end def question_sets - # We want self to come first for deterministic behavior in specs. After that it doesn't really matter. - # self_and_descendants orders by depth so we are good. - division.self_and_descendants.flat_map { |d| QuestionSet.where(division: d).order(:kind).to_a } + # get all question sets for all response sets of loans in this division and descendants + loans = division.self_and_descendants.flat_map { |d| Loan.where(division: d) } + question_sets = loans.flat_map{|l| l.response_sets.map{|rs| rs.question_set}}.uniq + question_sets.sort_by(&:kind) end def questions_by_id diff --git a/spec/models/shared_examples_data_export_spec.rb b/spec/models/shared_examples_data_export_spec.rb index d8053aec4..25f2decf1 100644 --- a/spec/models/shared_examples_data_export_spec.rb +++ b/spec/models/shared_examples_data_export_spec.rb @@ -135,6 +135,46 @@ expect(response_data).to contain_exactly(row1, row2, row3) end end + + context "when loan belonging to subdivision uses a question set of ancestor division and a qs in its own division" do + let(:export) { create(:enhanced_loan_data_export, division: subdivision, data: nil) } + let!(:setA) { create(:question_set, kind: "loan_criteria", division: division) } + let(:qaattrs) { {question_set: setA, division: division} } + let!(:qa1) { create(:question, qaattrs.merge(data_type: "number", label: "QA1")) } + let!(:qa2) { create(:question, qaattrs.merge(data_type: "number", label: "QA2")) } + + let!(:setB) { create(:question_set, kind: "loan_data", division: subdivision) } + let(:qbattrs) { {question_set: setB, division: subdivision} } + let!(:qb1) { create(:question, qbattrs.merge(data_type: "number", label: "QB1")) } + + let!(:r1) do + create(:response_set, + question_set: setA, + loan: loan1) + end + let!(:r1_a1) { create(:answer, question: qa1, response_set: r1, numeric_data: 5, not_applicable: false) } + let!(:r1_a2) { create(:answer, question: qa2, response_set: r1, numeric_data: 10, not_applicable: false) } + let!(:r2) do + create(:response_set, + question_set: setB, + loan: loan2) + end + let!(:r2_a1) { create(:answer, question: qb1, response_set: r2, numeric_data: 15, not_applicable: false) } + let!(:r3) do + create(:response_set, + question_set: setA, + loan: loan2) + end + let!(:r3_a1) { create(:answer, question: qa1, response_set: r3, numeric_data: 3, not_applicable: false) } + let!(:r3_a2) { create(:answer, question: qa2, response_set: r3, numeric_data:7, not_applicable: false) } + it "has questions from both question sets" do + export.process_data + expect(export.data[0]).to eq(base_headers + ["QA1", "QA2", "QB1"]) + expect(export.data[1]).to eq(["Question ID"] + id_row_nils + [qa1, qa2, qb1].map(&:id)) + row1 = ["3", "7", "15"] # only loan 2 should be exported, since only loan in subdivision + expect(response_data).to contain_exactly(row1) + end + end end end From 5a3a089e180651ba66d73e77109c2592fd770801 Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Wed, 11 May 2022 10:27:45 -0500 Subject: [PATCH 25/30] 12179: add boolean flag to filtered question serializer so inheritance info only calculating in question editing, and not in questionnaire loading --- app/controllers/admin/questions_controller.rb | 2 +- app/models/filtered_question.rb | 1 + .../filtered_question_serializer.rb | 24 ++++++++++++------- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/app/controllers/admin/questions_controller.rb b/app/controllers/admin/questions_controller.rb index 3dd50fd95..dea1f38df 100644 --- a/app/controllers/admin/questions_controller.rb +++ b/app/controllers/admin/questions_controller.rb @@ -11,7 +11,7 @@ def index @question_sets = QuestionSet.find_for_division(selected_division_or_root) if @question_sets.any? @question_set = params.key?(:qset) ? QuestionSet.find(params[:qset]) : @question_sets.first - @questions = ActiveModel::Serializer::CollectionSerializer.new(top_level_questions(@question_set)) + @questions = ActiveModel::Serializer::CollectionSerializer.new(top_level_questions(@question_set), include_inheritance: true) @selected_division_depth = selected_division.depth end end diff --git a/app/models/filtered_question.rb b/app/models/filtered_question.rb index 86de7092d..b474e83f7 100644 --- a/app/models/filtered_question.rb +++ b/app/models/filtered_question.rb @@ -37,6 +37,7 @@ def parent end end + def visible? selected_division.self_or_descendant_of?(question.division) || include_descendant_divisions && selected_division.self_or_ancestor_of?(question.division) diff --git a/app/serializers/filtered_question_serializer.rb b/app/serializers/filtered_question_serializer.rb index 3c6f32922..569901b38 100644 --- a/app/serializers/filtered_question_serializer.rb +++ b/app/serializers/filtered_question_serializer.rb @@ -1,11 +1,25 @@ class FilteredQuestionSerializer < ApplicationSerializer attributes :id, :name, :children, :parent_id, :division_id, :division_depth, :fieldset, :optional, - :data_type, :required_loan_types, :inheritance_info, :active, :can_edit + :data_type, :required_loan_types, :active, :can_edit + # inheritance info is only needed in questions editing view, but this serializer is also used in the questionnaire view + # it adds significant db queries. this flag makes it so we can limit running those queries to where they are needed + attribute :inheritance_info, if: -> { self.include_inheritance } + attr_accessor :include_inheritance def initialize(*args, **options) + self.include_inheritance = options[:include_inheritance] super(*args, options) end + + def inheritance_info + cmp = object.division.depth <=> object.selected_division.depth + return nil if cmp == 0 + + direction = cmp == -1 ? "ancestor" : "descendant" + I18n.t("questions.inheritance_tag.#{direction}", division_name: object.division.name) + end + def name object.full_number_and_label end @@ -30,14 +44,6 @@ def required_loan_types object.loan_question_requirements.map { |i| i.loan_type.label.to_s } end - def inheritance_info - cmp = object.division.depth <=> object.selected_division.depth - return nil if cmp == 0 - - direction = cmp == -1 ? "ancestor" : "descendant" - I18n.t("questions.inheritance_tag.#{direction}", division_name: object.division.name) - end - def can_edit # This is not a policy-level matter, it's a view thing. Only questions for the currently selected division # can be edited as a matter of good UX. A user may be allowed to edit more questions than that From 01ff40c040331e1b1cb11f6045128fe7c238fec6 Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Wed, 11 May 2022 10:47:52 -0500 Subject: [PATCH 26/30] 12179: cleaning up comments etc --- app/models/data_export.rb | 1 - app/models/response.rb | 28 ------------------- app/models/response_set.rb | 17 ++++------- .../filtered_question_serializer.rb | 2 +- 4 files changed, 7 insertions(+), 41 deletions(-) diff --git a/app/models/data_export.rb b/app/models/data_export.rb index f21e1e5aa..4b79704d3 100644 --- a/app/models/data_export.rb +++ b/app/models/data_export.rb @@ -43,7 +43,6 @@ def process_data end def to_csv! - logger.debug("LOGGING FROM data_export.rb") raise ArgumentError, "No data found" if data.blank? raise TypeError, "Data should be a 2D Array" unless (data.is_a?(Array) && data.first.is_a?(Array)) diff --git a/app/models/response.rb b/app/models/response.rb index 99bc30eaf..62fb255f5 100644 --- a/app/models/response.rb +++ b/app/models/response.rb @@ -36,34 +36,6 @@ def linked_document end end - #===== START temp methods for 2022 overhaul ===# - - # def boolean - # @answer.boolean_data ? "yes" : "no" if @answer.present? - # end - # - # def text - # @answer.text_data if @answer.present? - # end - # - # def number - # @answer.numeric_data if @answer.present? - # end - # - # def not_applicable - # @answer.not_applicable ? "yes" : "no" if @answer.present? - # end - # - # def url - # @answer.linked_document_data["url"] if @answer.present? - # end - # - # def group? - # @question.present? && @question.group? - # end - - #===== START temp methods for 2022 overhaul ===# - def breakeven_table @breakeven_table ||= BreakevenTableQuestion.new(breakeven) end diff --git a/app/models/response_set.rb b/app/models/response_set.rb index 6b748c244..66516f878 100644 --- a/app/models/response_set.rb +++ b/app/models/response_set.rb @@ -40,7 +40,7 @@ def response(question) Response.new(loan: loan, question: question, response_set: self, data: raw_value) end - # for migration + # # this method is temporary for spr 2022 overhaul, for migration def ensure_all_answers_copied answer_q_ids = answers.pluck(:question_id).sort # select q ids where the response is not blank @@ -71,15 +71,9 @@ def set_answer_from_custom_data_style_json(question, value) end # Change/assign custom field value, but don't save. - # WHY do we not save here? probably just to save db writes - # THIS is where the question internal_name (e.g. field_110) coming form jqtree gets converted back to + # This is where the question internal_name (e.g. field_110) coming form jqtree gets converted back to # the question id that is the key in the response set's custom_data. - # so we can use the question.id and self.id to find an answer record. def set_response(question, value) - #TODO: find or create answer record by question id and self.id - # call a new method on answer that takes value and saves the fields - # i don't THINK we actually need to return custom data, but if we - # have to, we'll call a method on answer model that composes custom_data equivalent. self.custom_data ||= {} custom_data[question.json_key] = value custom_data @@ -131,9 +125,8 @@ def make_answers # back to the server with params that are internal names of questions e.g. "field_110=" # Rails calls method_missing since these aren't attrs of a response set, # and this method then calls response(q) and set_response(q) instead of erroring. - # it basically uses Rail's under the hood iteration over params from the request - # in lieu of writing our own. - # so far I am unclear where the get version happens . . . + # it uses Rail's under the hood iteration over params from the request + # As of May 2022 'get' action not used anywhere. def method_missing(method_sym, *arguments, &block) attribute_name, action, field = match_dynamic_method(method_sym) if action @@ -144,6 +137,8 @@ def method_missing(method_sym, *arguments, &block) case action when :set return set_response(q, arguments.first) + else + raise "unknown action #{action} in method_missing override in response_set.rb" end end super diff --git a/app/serializers/filtered_question_serializer.rb b/app/serializers/filtered_question_serializer.rb index 569901b38..db595aabb 100644 --- a/app/serializers/filtered_question_serializer.rb +++ b/app/serializers/filtered_question_serializer.rb @@ -2,7 +2,7 @@ class FilteredQuestionSerializer < ApplicationSerializer attributes :id, :name, :children, :parent_id, :division_id, :division_depth, :fieldset, :optional, :data_type, :required_loan_types, :active, :can_edit # inheritance info is only needed in questions editing view, but this serializer is also used in the questionnaire view - # it adds significant db queries. this flag makes it so we can limit running those queries to where they are needed + # calculating inheritance info adds significant db queries. this flag makes it so we can limit running those queries to where they are needed attribute :inheritance_info, if: -> { self.include_inheritance } attr_accessor :include_inheritance From 88e241c9b4420157a239b4bd2ba03e0950670ed1 Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Mon, 16 May 2022 13:10:06 -0500 Subject: [PATCH 27/30] 12179: fix handling of boolean answers, which were saving unanswered as 'no' because '' != "yes" --- app/models/answer.rb | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/app/models/answer.rb b/app/models/answer.rb index fdf009eeb..ff2681b17 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -59,7 +59,7 @@ def self.contains_answer_data?(hash_data) elsif key == "not_applicable" return true if value == "yes" elsif key == "boolean" - return true unless value.nil? + return true unless (value.nil? || value.empty?) elsif %w(business_canvas).include?(key) return true unless self.json_answer_blank?(value) elsif %w(breakeven).include?(key) @@ -88,7 +88,16 @@ def self.save_from_form_field_params(question, fields, response_set) else nil end - boolean_data = fields.key?("boolean") ? (fields["boolean"] == "yes") : nil + boolean_data = if fields.key?("boolean") + case fields["boolean"] + when "yes" + true + when "no" + false + else + nil + end + end breakeven_data = fields.key?("breakeven") ? fields["breakeven"] : nil business_canvas_data = fields.key?("business_canvas") ? fields["business_canvas"] : nil linked_document_data = fields.key?("url") ? {"url": fields["url"] } : {"url": ""} From a76c04e6aebb1b2ccc5fc604d0dc5fcef8f9ef8f Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Mon, 18 Jul 2022 17:03:16 -0400 Subject: [PATCH 28/30] 12197: ensure internal names unique --- app/models/answer.rb | 8 +++++++ ...0718191508_ensure_internal_names_unique.rb | 24 +++++++++++++++++++ db/schema.rb | 7 +++--- 3 files changed, 36 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20220718191508_ensure_internal_names_unique.rb diff --git a/app/models/answer.rb b/app/models/answer.rb index ff2681b17..95b3d4331 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -4,6 +4,7 @@ class Answer < ApplicationRecord delegate :data_type, to: :question validate :has_data validate :question_is_not_group + validate :question_set_matches # this method is temporary for spr 2022 overhaul def compare_to_custom_data @@ -18,6 +19,9 @@ def compare_to_custom_data raise "ERROR for answer #{id}: for RS #{response_set.id} custom data value for #{m} is #{custom_data_value} but is #{answer_value} for answer #{id}" end end + unless question.question_set_id == response_set.question_set_id + raise "ERROR for answer #{id}: question set has id #{question.question_set_id}, response set has qset id #{response_set.question_set_id}" + end end def to_s @@ -131,6 +135,10 @@ def has_data breakeven_data.present? end + def question_set_matches + question.question_set_id == response_set.question_set_id + end + # temp method for spr 2022 overhaul def raw_value json = {} diff --git a/db/migrate/20220718191508_ensure_internal_names_unique.rb b/db/migrate/20220718191508_ensure_internal_names_unique.rb new file mode 100644 index 000000000..8e0712ab9 --- /dev/null +++ b/db/migrate/20220718191508_ensure_internal_names_unique.rb @@ -0,0 +1,24 @@ +class EnsureInternalNamesUnique < ActiveRecord::Migration[6.1] + def up + special_internal_names = ["poc_ownership_percent", "cooperative_members", "women_ownership_percent", "environmental_impact_score"] + Question.find_each do |q| + if q.internal_name.include?("field") + q.update(internal_name: "#{q.internal_name}_#{q.id}") + end + if special_internal_names.include?(q.internal_name) + q.update(internal_name: "#{q.internal_name}_#{q.id}") + end + end + add_index :questions, :internal_name, unique: true + end + + def down + special_internal_names = ["poc_ownership_percent", "cooperative_members", "women_ownership_percent", "environmental_impact_score"] + Question.find_each do |q| + if q.internal_name.include?("field") || special_internal_names.include?(q.internal_name) + q.update(internal_name: q.internal_name.gsub("_#{q.id}", "")) + end + end + remove_index :questions, column: [:internal_name] + end +end diff --git a/db/schema.rb b/db/schema.rb index cf2aeffe6..edc654f03 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2022_04_22_165601) do +ActiveRecord::Schema.define(version: 2022_07_18_191508) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -311,7 +311,7 @@ t.date "date_established" t.integer "division_id" t.string "email" - t.string "entity_structure", default: "not_selected", null: false + t.string "entity_structure", default: "llc", null: false t.string "fax" t.string "inception_value" t.string "industry" @@ -439,6 +439,7 @@ t.integer "question_set_id" t.datetime "updated_at", null: false t.index "question_set_id, ((parent_id IS NULL))", name: "index_questions_on_question_set_id_parent_id_IS_NULL", unique: true, where: "(parent_id IS NULL)", comment: "Ensures max one root per question set" + t.index ["internal_name"], name: "index_questions_on_internal_name", unique: true t.index ["question_set_id"], name: "index_questions_on_question_set_id" end @@ -451,7 +452,7 @@ t.bigint "question_set_id", null: false t.datetime "updated_at", null: false t.integer "updater_id" - t.index ["loan_id", "question_set_id"], name: "index_response_sets_on_loan_id_and_question_set_id" + t.index ["loan_id", "question_set_id"], name: "index_response_sets_on_loan_id_and_question_set_id", unique: true t.index ["question_set_id"], name: "index_response_sets_on_question_set_id" end From 76b93c5c073692b8773162a9bad89e4e90a4e248 Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Fri, 24 Mar 2023 17:20:37 -0500 Subject: [PATCH 29/30] 12179: specs passing, added workaround where internal names for non-group questions must contain string 'field' to identify form fields that need to be saved as answers. --- app/models/question.rb | 11 +++++++++++ config/locales/en/activerecord.en.yml | 1 + ...139_ensure_internal_names_contain_field.rb | 19 +++++++++++++++++++ db/schema.rb | 3 +-- lib/tasks/one_time_changes.rake | 9 ++++++--- spec/factories/questions_factory.rb | 1 + spec/support/helpers/question_spec_helpers.rb | 1 - spec/system/admin/questions_flow_spec.rb | 4 ++-- 8 files changed, 41 insertions(+), 8 deletions(-) create mode 100644 db/migrate/20230323205139_ensure_internal_names_contain_field.rb diff --git a/app/models/question.rb b/app/models/question.rb index c07f6a736..1bc7241b2 100644 --- a/app/models/question.rb +++ b/app/models/question.rb @@ -41,6 +41,7 @@ class Question < ApplicationRecord validates :data_type, presence: true validate :parent_division_depth_must_be_less_than_or_equal_to_ours + validate :internal_name_contains_field after_save :ensure_internal_name @@ -198,12 +199,22 @@ def prepare_numbers true end + # temporary for biz dev data migration def ensure_internal_name if !internal_name self.update! internal_name: "field_#{id}" end end + # temporary for biz dev data migration + # all non-group qs need to have "field" for their Answers to save + def internal_name_contains_field + return if internal_name.nil? # will then go to ensure_internal_name + return if data_type == "group" + return if internal_name.include?("field") + errors.add(:base, :invalid_internal_name) + end + def parent_division_depth_must_be_less_than_or_equal_to_ours return if parent.nil? diff --git a/config/locales/en/activerecord.en.yml b/config/locales/en/activerecord.en.yml index b71597c94..7377dfdf2 100644 --- a/config/locales/en/activerecord.en.yml +++ b/config/locales/en/activerecord.en.yml @@ -219,6 +219,7 @@ en: no_summary: "Summary is required." question: invalid_parent_division_depth: "Parent must be in same or ancestor division." + invalid_internal_name: "Internal name must include the string 'field'" organization: attributes: primary_contact: diff --git a/db/migrate/20230323205139_ensure_internal_names_contain_field.rb b/db/migrate/20230323205139_ensure_internal_names_contain_field.rb new file mode 100644 index 000000000..a3ab69c4e --- /dev/null +++ b/db/migrate/20230323205139_ensure_internal_names_contain_field.rb @@ -0,0 +1,19 @@ +class EnsureInternalNamesContainField < ActiveRecord::Migration[6.1] + + # In the move from json blob to Answer table, need a way + # to distinguish internal_name form keys from other form keys + # when saving Answers data. This supports that need, and + # internal_name will be removed entirely in a following release. + + def up + Question.find_each do |q| + if !q.internal_name.include?("field") + q.update(internal_name: "#{q.internal_name}_field") + end + end + end + + def down + # do nothing, not a problem if they do contain field + end +end diff --git a/db/schema.rb b/db/schema.rb index c1acb1e66..1f7198857 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,8 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2023_01_30_223046) do - +ActiveRecord::Schema.define(version: 2023_03_23_205139) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/lib/tasks/one_time_changes.rake b/lib/tasks/one_time_changes.rake index 087b31009..102935ded 100644 --- a/lib/tasks/one_time_changes.rake +++ b/lib/tasks/one_time_changes.rake @@ -1,5 +1,5 @@ namespace :one_time_changes do -<<<<<<< HEAD + desc "A one time task to create Answers for all existing response sets" task migrate_from_response_custom_data_to_answers: :environment do ResponseSet.find_each do |rs| @@ -13,7 +13,10 @@ namespace :one_time_changes do puts e.message Rails.logger.info e.message # don't re-raise, keep going -======= + end + end + end + desc "A one time task responding to Oct 2022 request to update stautuses." task update_loan_statuses_oct_22: :environment do @@ -75,11 +78,11 @@ namespace :one_time_changes do loans_to_update.each do |l| l.update(status_value: "possible") ->>>>>>> develop end end end + desc "A one time task responding to Nov 2019 request to update loan date fields." task adjust_loan_dates: :environment do Loan.find_each do |l| diff --git a/spec/factories/questions_factory.rb b/spec/factories/questions_factory.rb index cfbdccf72..1ace60f7f 100644 --- a/spec/factories/questions_factory.rb +++ b/spec/factories/questions_factory.rb @@ -8,6 +8,7 @@ question_set data_type { Question::DATA_TYPES.sample } active { true } + internal_name { "#{Faker::Name.unique.name}_field" } after(:build) do |model| model.parent ||= model.question_set.root_group diff --git a/spec/support/helpers/question_spec_helpers.rb b/spec/support/helpers/question_spec_helpers.rb index 85f42441b..e55eaa5b5 100644 --- a/spec/support/helpers/question_spec_helpers.rb +++ b/spec/support/helpers/question_spec_helpers.rb @@ -71,7 +71,6 @@ def create_question(set: qset, active: true, name: "", type:, override: true, re create(:question, question_set: set, active: active, - internal_name: name, data_type: type, override_associations: override, loan_types: loan_types || (required ? [lt1] : []), diff --git a/spec/system/admin/questions_flow_spec.rb b/spec/system/admin/questions_flow_spec.rb index 62481bc48..634d816c2 100644 --- a/spec/system/admin/questions_flow_spec.rb +++ b/spec/system/admin/questions_flow_spec.rb @@ -107,7 +107,7 @@ fill_in("Title", with: "Stuff") click_button("Save") - expect(page).to have_content("Errors prevented the record from being saved.", wait: 40) + expect(page).to have_content("Errors prevented the record from being saved.", wait: 80) select("Number", from: "Data Type") click_button("Save") @@ -117,7 +117,7 @@ find(%([data-id="#{q113.id}"] > .jqtree-element .edit-action)).click fill_in("Title", with: "Stonks") click_button("Save") - + expect(page).to have_css(".jqtree-title", text: "Stonks", wait: 80) expect(page).not_to have_css(".jqtree-title", text: "q113") From 99daf104579423c7320d03ef6c2aee605a87e378 Mon Sep 17 00:00:00 2001 From: DevneyHamilton Date: Wed, 29 Mar 2023 18:03:19 -0500 Subject: [PATCH 30/30] 12179: greatly simplify database migrations and move them to run after migrations already deployed to staging and prod --- ...0220418205710_add_uniq_index_to_answers.rb | 5 ----- ...ire_question_and_response_set_on_answer.rb | 10 ---------- ..._answers_for_response_set_and_questions.rb | 5 ----- ...rs.rb => 20230301194344_create_answers.rb} | 5 +++-- ...139_ensure_internal_names_contain_field.rb | 19 ------------------- ...nternal_names_unique_and_contain_field.rb} | 7 +++---- db/schema.rb | 2 +- 7 files changed, 7 insertions(+), 46 deletions(-) delete mode 100644 db/migrate/20220418205710_add_uniq_index_to_answers.rb delete mode 100644 db/migrate/20220421220934_require_question_and_response_set_on_answer.rb delete mode 100644 db/migrate/20220422165601_re_add_uniq_index_on_answers_for_response_set_and_questions.rb rename db/migrate/{20220305194344_create_answers.rb => 20230301194344_create_answers.rb} (73%) delete mode 100644 db/migrate/20230323205139_ensure_internal_names_contain_field.rb rename db/migrate/{20220718191508_ensure_internal_names_unique.rb => 20230329191508_ensure_internal_names_unique_and_contain_field.rb} (80%) diff --git a/db/migrate/20220418205710_add_uniq_index_to_answers.rb b/db/migrate/20220418205710_add_uniq_index_to_answers.rb deleted file mode 100644 index bdc8e7386..000000000 --- a/db/migrate/20220418205710_add_uniq_index_to_answers.rb +++ /dev/null @@ -1,5 +0,0 @@ -class AddUniqIndexToAnswers < ActiveRecord::Migration[6.1] - def change - add_index :answers, %i[response_set_id question_id], unique: true - end -end diff --git a/db/migrate/20220421220934_require_question_and_response_set_on_answer.rb b/db/migrate/20220421220934_require_question_and_response_set_on_answer.rb deleted file mode 100644 index c0854a0f3..000000000 --- a/db/migrate/20220421220934_require_question_and_response_set_on_answer.rb +++ /dev/null @@ -1,10 +0,0 @@ -class RequireQuestionAndResponseSetOnAnswer < ActiveRecord::Migration[6.1] - def change - Answer.where(response_set_id: nil).delete_all - Answer.where(question_id: nil).delete_all - remove_reference :answers, :response_set, index: true, foreign_key: true - add_reference :answers, :response_set, index: true, foreign_key: true, null: false - remove_reference :answers, :question, index: true, foreign_key: true - add_reference :answers, :question, index: true, foreign_key: true, null: false - end -end diff --git a/db/migrate/20220422165601_re_add_uniq_index_on_answers_for_response_set_and_questions.rb b/db/migrate/20220422165601_re_add_uniq_index_on_answers_for_response_set_and_questions.rb deleted file mode 100644 index 66be87f2b..000000000 --- a/db/migrate/20220422165601_re_add_uniq_index_on_answers_for_response_set_and_questions.rb +++ /dev/null @@ -1,5 +0,0 @@ -class ReAddUniqIndexOnAnswersForResponseSetAndQuestions < ActiveRecord::Migration[6.1] - def change - add_index :answers, %i[response_set_id question_id], unique: true - end -end diff --git a/db/migrate/20220305194344_create_answers.rb b/db/migrate/20230301194344_create_answers.rb similarity index 73% rename from db/migrate/20220305194344_create_answers.rb rename to db/migrate/20230301194344_create_answers.rb index 8da1a7a52..b1128a313 100644 --- a/db/migrate/20220305194344_create_answers.rb +++ b/db/migrate/20230301194344_create_answers.rb @@ -1,8 +1,8 @@ class CreateAnswers < ActiveRecord::Migration[6.1] def change create_table :answers do |t| - t.references :response_set, index: true, foreign_key: true - t.references :question, index: true, foreign_key: true + t.references :response_set, index: true, foreign_key: true, null: false + t.references :question, index: true, foreign_key: true, null: false t.boolean :not_applicable, null: false, default: false t.string :text_data t.string :numeric_data @@ -12,5 +12,6 @@ def change t.json :breakeven_data t.timestamps end + add_index :answers, %i[response_set_id question_id], unique: true end end diff --git a/db/migrate/20230323205139_ensure_internal_names_contain_field.rb b/db/migrate/20230323205139_ensure_internal_names_contain_field.rb deleted file mode 100644 index a3ab69c4e..000000000 --- a/db/migrate/20230323205139_ensure_internal_names_contain_field.rb +++ /dev/null @@ -1,19 +0,0 @@ -class EnsureInternalNamesContainField < ActiveRecord::Migration[6.1] - - # In the move from json blob to Answer table, need a way - # to distinguish internal_name form keys from other form keys - # when saving Answers data. This supports that need, and - # internal_name will be removed entirely in a following release. - - def up - Question.find_each do |q| - if !q.internal_name.include?("field") - q.update(internal_name: "#{q.internal_name}_field") - end - end - end - - def down - # do nothing, not a problem if they do contain field - end -end diff --git a/db/migrate/20220718191508_ensure_internal_names_unique.rb b/db/migrate/20230329191508_ensure_internal_names_unique_and_contain_field.rb similarity index 80% rename from db/migrate/20220718191508_ensure_internal_names_unique.rb rename to db/migrate/20230329191508_ensure_internal_names_unique_and_contain_field.rb index 8e0712ab9..9ba912f44 100644 --- a/db/migrate/20220718191508_ensure_internal_names_unique.rb +++ b/db/migrate/20230329191508_ensure_internal_names_unique_and_contain_field.rb @@ -1,12 +1,11 @@ -class EnsureInternalNamesUnique < ActiveRecord::Migration[6.1] +class EnsureInternalNamesUniqueAndContainField < ActiveRecord::Migration[6.1] def up special_internal_names = ["poc_ownership_percent", "cooperative_members", "women_ownership_percent", "environmental_impact_score"] Question.find_each do |q| if q.internal_name.include?("field") q.update(internal_name: "#{q.internal_name}_#{q.id}") - end - if special_internal_names.include?(q.internal_name) - q.update(internal_name: "#{q.internal_name}_#{q.id}") + else + q.update(internal_name: "#{q.internal_name}_#{q.id}_field") end end add_index :questions, :internal_name, unique: true diff --git a/db/schema.rb b/db/schema.rb index 1f7198857..6ec794db2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2023_03_23_205139) do +ActiveRecord::Schema.define(version: 2023_03_29_191508) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql"