Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

12193 remove json blob for Answers, remove internal_name etc, and use nested forms to save Answers #968

Draft
wants to merge 87 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
87 commits
Select commit Hold shift + click to select a range
472b045
12132: first pass at migration code, make_answers method working for …
DevneyHamilton Mar 9, 2022
8d055ca
12132: notes on how things currently work and changes to make
DevneyHamilton Apr 18, 2022
2f409ed
12132: adds answer table in schema.rb
DevneyHamilton Apr 18, 2022
5a0c013
12132: make answers method on response set
DevneyHamilton Apr 18, 2022
a5c831a
Merge branch 'develop' into 12132_response_refactor
DevneyHamilton Apr 18, 2022
27d45f8
12132: remove duplicate field in schema from merge, and add wait time…
DevneyHamilton Apr 18, 2022
92de0e3
WIP 12132: attempting specs on answers table to custom data and back …
DevneyHamilton Apr 21, 2022
737a5dc
12132: saving form data to answers table, needs auto tests
DevneyHamilton Apr 21, 2022
cccc5d2
12132: adding answers and saving, reloading questionnaire for that lo…
DevneyHamilton Apr 22, 2022
f779c1d
12179: version with answer row for every question on a response set
DevneyHamilton Apr 25, 2022
0db9d79
12179: one time changes rake task to make answers
DevneyHamilton Apr 25, 2022
f8de5b5
WIP failing specs on 12179
DevneyHamilton Apr 26, 2022
506add7
WIP 12197: more specs passing
DevneyHamilton Apr 26, 2022
7cba41a
12197: remove response blank specs, and fix some of the boolean handl…
DevneyHamilton Apr 27, 2022
7119f49
12179: fixing not applicable boolean madness
DevneyHamilton Apr 27, 2022
d2348c7
12179: fix displaying linked document
DevneyHamilton Apr 27, 2022
3442136
12179: make validations on answer more expansive; just check there is…
DevneyHamilton Apr 27, 2022
5a21afe
12197: WIP enhanced data export working
DevneyHamilton May 2, 2022
9a0e644
12179: cleaning up debugging from work making data exports work with…
DevneyHamilton May 2, 2022
61377bf
12179: keep handling string numeric answers in data exports
DevneyHamilton May 4, 2022
7b7b952
12179: create shared specs between enhanced and numeric data exports,…
DevneyHamilton May 4, 2022
b76367e
12179: WIP specs breaking because internal names not unique, which is…
DevneyHamilton May 6, 2022
ab44b97
12179: specs passing after addressing problem with question factory c…
DevneyHamilton May 9, 2022
c8bdab4
12179: clean up debug code, specs passing and manual tests passing
DevneyHamilton May 9, 2022
c6d7571
12179: fix problem where data exports did not support exporting from …
DevneyHamilton May 10, 2022
5a3a089
12179: add boolean flag to filtered question serializer so inheritanc…
DevneyHamilton May 11, 2022
01ff40c
12179: cleaning up comments etc
DevneyHamilton May 11, 2022
88e241c
12179: fix handling of boolean answers, which were saving unanswered …
DevneyHamilton May 16, 2022
18634e4
12193: WIP mostly can display form without breaking, need to rip out …
DevneyHamilton May 11, 2022
88280a7
12193: WIP saving with nested attrs for number and text answers
DevneyHamilton May 17, 2022
87f1ef8
12193: WIP sends params that look good for existing answers, doesn't …
DevneyHamilton May 18, 2022
5ac34a7
12193: WIP attempting to make group logic work better with answer ins…
DevneyHamilton May 18, 2022
ece0edf
12193: WIP fixed weird repeat thing with fields_for by making a new a…
DevneyHamilton May 18, 2022
a8afa31
12193: WIP saving text & numeric answers and handling non-existant a…
DevneyHamilton May 18, 2022
da5a125
12193: not applicable workign in questionnaire w/out internal name
DevneyHamilton May 25, 2022
e0e936c
12193: boolean qs working in form without internal_name
DevneyHamilton May 25, 2022
b5affab
12193: ratings working without internal_name
DevneyHamilton May 25, 2022
8b3d351
12193: WIP biz canvas saves most recent value but not displaying
DevneyHamilton May 27, 2022
242ca47
12193: WIP biz canvas answers displaying
DevneyHamilton Jun 7, 2022
6e4d8a8
12193: breakeven table saving & displaying
DevneyHamilton Jun 7, 2022
a03261b
12193: linked document working without internal name
DevneyHamilton Jun 8, 2022
8ca8d68
12193: remove internal name from build_node_lookup_table_for, all spe…
DevneyHamilton Jun 8, 2022
af7e6f2
12193: WIP internal name removed, all specs except progress numerator…
DevneyHamilton Jun 8, 2022
a990591
12193: some minor improvements in performance
DevneyHamilton Jun 10, 2022
123251d
12193: saving ok with having added answers to loanfilteredquestions
DevneyHamilton Jun 13, 2022
264edd9
12193: remove response model
DevneyHamilton Jun 13, 2022
adb025b
12193: remove temporary code from migration in phase 1
DevneyHamilton Jun 13, 2022
404a17e
12193: remove addl temporary code from migration
DevneyHamilton Jun 13, 2022
02d33a0
12193: print view of questionnaire working without response model
DevneyHamilton Jun 13, 2022
7d63c6a
12193: drastically reduce db queries on questionnaire page load
DevneyHamilton Jun 13, 2022
71c9129
12193: add active record query trace as optional gem
DevneyHamilton Jun 14, 2022
c3f209d
12193: fix problem with a q group with no active children being a lea…
DevneyHamilton Jun 17, 2022
54bc71d
12193: add comment with key search term 'leaf' where q group treated …
DevneyHamilton Jun 17, 2022
7872704
12193: remove response model spec
DevneyHamilton Jun 17, 2022
982272d
12193: remove custom data column from response sets table
DevneyHamilton Jun 17, 2022
1524ebe
12193: remove custom data references in response set and answer models
DevneyHamilton Jun 17, 2022
e5431ed
12193: remove addl code associated w custom data to answer migration
DevneyHamilton Jun 17, 2022
d74a370
12193: loan check specs passing
DevneyHamilton Jun 17, 2022
36ad02c
12193: loan health check job specs pass
DevneyHamilton Jun 17, 2022
e391d63
12193: project duplication spec passing
DevneyHamilton Jun 17, 2022
5c48966
12193: remove very hard-to-trace **args from filteredquestion and loa…
DevneyHamilton Jun 22, 2022
37f42a0
12193: adding some guiding comments
DevneyHamilton Jun 22, 2022
f3938f3
12193: remove unused methods in response_set.rb
DevneyHamilton Jun 22, 2022
b2b8722
12193: WIP overhaul of lon filtered question initializer to handle re…
DevneyHamilton Jun 22, 2022
ec373f8
12193: got first progress spec passing with new approach
DevneyHamilton Jun 29, 2022
972f243
12193: all progress specs passing
DevneyHamilton Jun 30, 2022
7cb057b
12193: loan health check specs passing
DevneyHamilton Jul 1, 2022
72b8a52
12193: loan filtered q specs all passing
DevneyHamilton Jul 5, 2022
51521bc
12193: all specs passing
DevneyHamilton Jul 11, 2022
89b4f25
12193: cache division depth on questions to speed up page loading
DevneyHamilton Jul 11, 2022
78137bd
12193: add waits to reduce flapping in specs
DevneyHamilton Jul 11, 2022
fdf5e01
12193: breakeven table ok
DevneyHamilton Jul 13, 2022
5764013
12193: fixed [no answer] display when editing
DevneyHamilton Jul 13, 2022
a76c04e
12197: ensure internal names unique
DevneyHamilton Jul 18, 2022
09a1ed1
12193: cherrypick uniq internal name changes onto this branch from ph…
DevneyHamilton Jul 18, 2022
f55e1dc
12193: migration changes to schema.rb removing internal name
DevneyHamilton Jul 27, 2022
3de0335
12193: fix bug where marking breakeven table not applicable erases data
DevneyHamilton Jul 28, 2022
14db237
12193: fix progress bug
DevneyHamilton Jul 28, 2022
2f393d6
12193: fix progress pct specs and remove more unused code from phase 1
DevneyHamilton Jul 28, 2022
2966a21
Merge branch 'develop' into 12179_answers_migration
DevneyHamilton Feb 22, 2023
fde47ed
Merge branch '12179_answers_migration' into 12193_remove_internal_name
DevneyHamilton Feb 27, 2023
76b93c5
12179: specs passing, added workaround where internal names for non-g…
DevneyHamilton Mar 24, 2023
e1aa80a
Merge branch '12179_answers_migration' into 12193_remove_internal_name
DevneyHamilton Mar 29, 2023
58e158b
12193: remove instances of internal_name from answers_migration merge…
DevneyHamilton Mar 29, 2023
99daf10
12179: greatly simplify database migrations and move them to run afte…
DevneyHamilton Mar 29, 2023
9429564
Merge branch '12179_answers_migration' into 12193_remove_internal_name
DevneyHamilton Mar 30, 2023
ead4803
12193: specs passing after untangling merges and migrations
DevneyHamilton Mar 30, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ group :development, :test do
# Load environment variables from .env file in development
gem "dotenv-rails"

# gem 'active_record_query_trace'

# Call 'byebug' anywhere in the code to stop execution and get a debugger console
gem "byebug"
gem "pry"
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/admin/questions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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), for_questions_view: true)
@selected_division_depth = selected_division.depth
end
end
Expand Down
32 changes: 31 additions & 1 deletion app/controllers/admin/response_sets_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ def update

# Add updater id to params
adjusted_params = response_set_params.merge(updater_id: current_user.id)
#adjusted_params = handle_json_attr_values(adjusted_params, params)
# If there was a conflict and "Overwrite" was clicked, update the lock version to the one pulled
# from the database when the warning was displayed. We do this instead of just ignoring the
# lock_version in case someone made further changes since the warning was displayed. This way,
Expand All @@ -60,6 +61,10 @@ def destroy

private

def handle_json_attr_values(adjusted_params, params)

end

def handle_conflict
@conflict = true
@tab = 'questions'
Expand All @@ -74,7 +79,32 @@ def resolve_polymorphic(type, id)
end

def response_set_params
params.require(:response_set).permit!
params.require(:response_set).permit(:id, :loan_id, :question_set_id, :lock_version,
answers_attributes: [:id,
:question_id,
:text_data,
:numeric_data,
:not_applicable,
:boolean_data,
linked_document_data: [:url, :start_cell, :end_cell],
business_canvas_data: [
:key_partners,
:key_activities,
:key_resources,
:value_propositions,
:customer_relationships,
:channels,
:customer_segments,
:cost_structure,
:revenue_streams
],
breakeven_data: [
:periods,
:units,
fixed_costs: [:name, :amount],
products: [:name, :description, :unit, :price, :percentage_of_sales, :quantity, :cost]
]
])
end

def display_path
Expand Down
5 changes: 3 additions & 2 deletions app/controllers/concerns/questionnaire_renderable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ def prep_questionnaire(json: true)
@question_sets = QuestionSet.find_for_division(@loan_division)
unless @question_sets.empty?
@question_set ||= params.key?(:qset) ? QuestionSet.find(params[:qset]) : @question_sets.first
@response_set ||= ResponseSet.find_or_initialize_by(loan: @loan, question_set: @question_set)
@root = LoanFilteredQuestion.new(@question_set.root_group_preloaded, loan: @loan)
@response_set ||= ResponseSet.includes(:answers).find_or_initialize_by(loan: @loan, question_set: @question_set)
#@root = LoanFilteredQuestion.new(@question_set.root_group_preloaded, loan: @loan)
@root = LoanFilteredQuestion.new(@question_set.root_group_preloaded, loan: @loan, response_set: @response_set)
@questions_json = @root.children.map { |q| FilteredQuestionSerializer.new(q) }.to_json if json
end
end
Expand Down
28 changes: 14 additions & 14 deletions app/helpers/responses_helper.rb
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
module ResponsesHelper
def display_value_for_number(response)
return if !response.has_number?
if response.has_currency?
"#{prefix(response)}#{number_with_delimiter(response.number)} #{postfix(response)}"
elsif response.has_percentage?
"#{number_with_delimiter(response.number)}%"
def display_value_for_number(question, answer: nil, response_set: nil)
return if !question.has_number? || answer.nil?
if question.has_currency?
"#{prefix(question, response_set)}#{number_with_delimiter(answer.number)} #{postfix(question, response_set)}"
elsif question.has_percentage?
"#{number_with_delimiter(answer.number)}%"
else
number_with_delimiter(response.number)
number_with_delimiter(answer.number)
end
end

def prefix(response)
if response.has_currency?
response.loan.currency.try(:short_symbol)
def prefix(question, response_set)
if question.has_currency?
response_set.loan.currency.try(:short_symbol)
end
end

def postfix(response)
if response.has_currency?
response.loan.currency.try(:code)
elsif response.has_percentage?
def postfix(question, response_set)
if question.has_currency?
response_set.loan.currency.try(:code)
elsif question.has_percentage?
"%"
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/helpers/translations_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def locale_options
end

def translate_boolean(bool)
t(bool ? "reply_yes" : "reply_no")
t(bool ? "reply_true" : "reply_false")
end
end

Expand Down
179 changes: 179 additions & 0 deletions app/models/answer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
class Answer < ApplicationRecord
belongs_to :response_set, optional: false
belongs_to :question, optional: false
delegate :data_type, to: :question
validate :question_is_not_group
validates_presence_of :question_id
validate :question_set_matches

before_save :ensure_json_format
before_save :clean_breakeven


def ensure_json_format
business_canvas_data = business_canvas_data.to_json
breakeven_data = breakeven_data.to_json
end

def clean_breakeven
return unless breakeven_data
clean_breakeven_products = []
if breakeven_data["products"]
breakeven_data["products"].each do |p|
clean_breakeven_products << p unless p.values.reject(&:blank?).empty?
end
breakeven_data["products"] = clean_breakeven_products
end
if breakeven_data["fixed_costs"]
clean_fixed_costs = []
breakeven_data["fixed_costs"].each do |fc|
clean_fixed_costs << fc unless fc.values.reject(&:blank?).empty?
end
breakeven_data["fixed_costs"] = clean_fixed_costs
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 self.json_answer_blank?(answer_json)
answer_json.values.all?{|v| v.blank?}
end

def json_answer_blank?(answer_json)
answer_json.values.all?{|v| v.blank?}
end

def to_s
"RS: #{response_set.question_set.kind}, Q id: #{question.id}, 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?
!not_applicable &&
text_data.blank? &&
numeric_data.blank? &&
boolean_data.nil? &&
linked_document_data_blank? &&
business_canvas_blank? &&
breakeven_data_blank?
end

def applicable?
!not_applicable
end

def text
text_data
end

def boolean
boolean_data
end

def number
numeric_data
end
# can we get rid of rating as a concept? its very confusing. no it is limited to 1-5.
def rating
numeric_data
end

def breakeven_table
@breakeven_table ||= BreakevenTableQuestion.new(breakeven_data)
end

def breakeven_hash
@breakeven_hash ||= breakeven_table.data_hash
end

def business_canvas
business_canvas_data.symbolize_keys if business_canvas_data
end

def breakeven_report
@breakeven_report ||= breakeven_table.report
end

def linked_document_data_blank?
linked_document_data.blank? || linked_document_data.values.all?{ |v| v.blank? }
end

def business_canvas_blank?
business_canvas_data.blank? || self.json_answer_blank?(business_canvas_data)
end

def breakeven_data_blank?
breakeven_data.blank? || self.json_answer_blank?(breakeven_data)
end

def linked_document
if linked_document_data.present?
LinkedDocument.new(linked_document_data.symbolize_keys)
else
nil
end
end

#TODO: can remove after answers migration?
def question_set_matches
question.question_set_id == response_set.question_set_id
end

# temp method for spr 2022 overhaul
def raw_value
json = {}
json["not_applicable"] = self.not_applicable ? "yes" : "no"
if self.text_data.present?
json["text"] = self.text_data
end
unless self.boolean_data.nil?
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?
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["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
end

def question_is_not_group
question.data_type != "group"
end

def question_set_matches
question.question_set_id == response_set.question_set_id
end

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"
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
else
raise "invalid question data type #{question.data_type}"
end
end
end
1 change: 0 additions & 1 deletion app/models/calendar_event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ class CalendarEvent
alias_method :has_precedent?, :has_precedent

def self.build_for(item)
puts "item: #{item.inspect}"
case item
when BasicProject
[new_project_start(item), new_project_end(item)]
Expand Down
42 changes: 0 additions & 42 deletions app/models/concerns/progress_calculable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,46 +18,4 @@ def progress_pct
def progress_type
required? ? "normal" : "optional"
end

protected

# If this is a required node, the numerator is the number of answered, required child questions,
# plus the numerators of any required child groups.
# If this is an optional node, the numerator is just the number of answered child questions,
# plus the numerators of any child groups.
def progress_numerator
return @progress_numerator if @progress_numerator
return @progress_numerator = 0 unless active?

@progress_numerator = progress_applicable_children.sum do |c|
(c.answered? && !c.group? ? 1 : 0) + c.progress_numerator
end
end

# If this is a required node, the denominator is the number of required child questions,
# plus the denominators of any required child groups.
# If this is an optional node, the denominator is just the total number of child questions,
# plus the denominators of any child groups.
def progress_denominator
return @progress_denominator if @progress_denominator
return @progress_denominator = 0 unless active?

@progress_denominator = progress_applicable_children.sum do |c|
(!c.group? ? 1 : 0) + c.progress_denominator
end
end

# Inactive questions should be ignored. Inactive questions only show when they are
# answered, and they are never required, so progress makes no sense. Retired questions should
# never show, so they should be excluded as well.
# If the current response is required, only count children that are also required.
def progress_applicable_children
children.select do |c|
if self.required?
c.active? && c.required?
else
c.active?
end
end
end
end
9 changes: 7 additions & 2 deletions app/models/data_export.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,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
Expand All @@ -89,7 +94,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
Expand Down
Loading