Skip to content

Commit

Permalink
🚧 WIP Refactoring accommodate additional download
Browse files Browse the repository at this point in the history
This commit:
- Adds `BaseFormatterService` to handle common formatting logic for all
download formats
- Adds `PlainTextFormatterService` to handle formatting specific for
plain text
- Adds a model_exporter class method to each question type model with
the name of each question type
- Updates specs to reflect changes to PlainTextFormatterService

Ref:
- #348
  • Loading branch information
sjproctor committed Feb 25, 2025
1 parent d80412b commit 64b697d
Show file tree
Hide file tree
Showing 16 changed files with 121 additions and 92 deletions.
2 changes: 1 addition & 1 deletion app/controllers/search_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def index

def text_download
questions = Question.where(id: Bookmark.select(:question_id))
content = questions.map { |question| QuestionTextFormatterService.new(question).format }.join('')
content = questions.map { |question| PlainTextFormatterService.new(question).format }.join('')
send_data content, filename: 'questions.txt', type: 'text/plain'
end

Expand Down
1 change: 1 addition & 0 deletions app/models/question.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class Question < ApplicationRecord
class_attribute :required_csv_headers, default: %w[IMPORT_ID TEXT TYPE].freeze
class_attribute :type_label, default: "Question", instance_writer: false
class_attribute :type_name, default: "Question", instance_writer: false
class_attribute :model_exporter, default: "Question", instance_writer: false

##
# @!attribute qti_max_value [r|w]
Expand Down
1 change: 1 addition & 0 deletions app/models/question/bow_tie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
# => true
class Question::BowTie < Question
self.type_name = "Bow Tie"
self.model_exporter = 'bow_tie'

This comment has been minimized.

Copy link
@laritakr

laritakr Feb 25, 2025

Contributor

I anticipate the model_exporter to be the name of the method you used in format_by_type. So both essay and upload types would use essay_type, traditional, select_all_that_apply, and drag_and_drop would use traditional_type, etc. Basically it's a way of dynamically determining which method of formatting you use. If you prefer to rename them to something more indicative of what they do, that would be fine. But each question type does not need one named on their own class name.


# NOTE: We're not storing this in a JSONB data type, but instead favoring a text field. The need
# for the data to be used in the application, beyond export of data, is minimal.
Expand Down
1 change: 1 addition & 0 deletions app/models/question/categorization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class Question::Categorization < Question
include MatchingQuestionBehavior

self.type_name = "Categorization"
self.model_exporter = 'categorization'
self.export_as_xml = true
self.choice_cardinality_is_multiple = true

Expand Down
1 change: 1 addition & 0 deletions app/models/question/drag_and_drop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
# data: [{ answer: "Aardvark", correct: true }, { answer: "Blue", correct: false }, { answer: "Yellow", correct:false }, { answer: "Cat", correct: true }])
class Question::DragAndDrop < Question
self.type_name = "Drag and Drop"
self.model_exporter = 'drag_and_drop'

##
# Represents the mapping process of a CSV Row to the underlying {Question::DragAndDrop}.
Expand Down
1 change: 1 addition & 0 deletions app/models/question/essay.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class Question::Essay < Question
include MarkdownQuestionBehavior

self.type_name = "Essay"
self.model_exporter = 'essay'
self.export_as_xml = true

class ImportCsvRow < MarkdownQuestionBehavior::ImportCsvRow
Expand Down
1 change: 1 addition & 0 deletions app/models/question/matching.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class Question::Matching < Question
include MatchingQuestionBehavior

self.type_name = "Matching"
self.model_exporter = 'matching'
self.export_as_xml = true
self.choice_cardinality_is_multiple = false

Expand Down
1 change: 1 addition & 0 deletions app/models/question/scenario.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
class Question::Scenario < Question
self.type_label = "Scenario"
self.type_name = "Scenario"
self.model_exporter = 'scenario'
self.included_in_filterable_type = false

class ImportCsvRow < Question::ImportCsvRow
Expand Down
1 change: 1 addition & 0 deletions app/models/question/select_all_that_apply.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
# duplication than more complicated inheritance.
class Question::SelectAllThatApply < Question
self.type_name = "Select All That Apply"
self.model_exporter = 'select_all_that_apply'

##
# Represents the mapping process of a CSV Row to the underlying {Question::SelectAllThatApply}.
Expand Down
1 change: 1 addition & 0 deletions app/models/question/stimulus_case_study.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
class Question::StimulusCaseStudy < Question
self.type_label = "Case Study"
self.type_name = "Stimulus Case Study"
self.model_exporter = 'stimulus_case_study'
self.has_parts = true

has_many :as_parent_question_aggregations,
Expand Down
1 change: 1 addition & 0 deletions app/models/question/traditional.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ class Question::Traditional < Question
#
# @see https://github.com/notch8/viva/issues/261
self.type_name = "Multiple Choice"
self.model_exporter = 'multiple_choice'

##
# Represents the mapping process of a CSV Row to the underlying {Question::Traditional}.
Expand Down
1 change: 1 addition & 0 deletions app/models/question/upload.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ class Question::Upload < Question
include MarkdownQuestionBehavior

self.type_name = "Upload"
self.model_exporter = 'upload'
self.export_as_xml = true

class ImportCsvRow < MarkdownQuestionBehavior::ImportCsvRow
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,68 +2,84 @@
require 'nokogiri'

##
# Service to handle formatting questions into plain text
# rubocop:disable Metrics/ClassLength
class QuestionTextFormatterService
def initialize(question)
# Service to handle formatting questions for downloads

class BaseFormatterService
def initialize(question, subq = false)
@question = question
@subq = subq
end

def format
content = format_by_type
content + "\n**********\n\n"
format_by_type + content_divider
end

# These methods are protected instead of private so they can be called by other instances
protected

def essay_type
def content_divider
raise NotImplementedError, "Subclasses must implement content_divider"
end

def format_by_type
method = @question.class.model_exporter
send(method)
end

def format_question_header
raise NotImplementedError, "Subclasses must implement format_question_header"
end

def essay
format_question_header + format_essay_content
end

def traditional_type
def upload
format_question_header + format_essay_content
end

def multiple_choice
format_question_header + format_answers(@question.data) { |answer, index| format_traditional_answer(answer, index) }
end

def matching_type
def select_all_that_apply
format_question_header + format_answers(@question.data) { |answer, index| format_traditional_answer(answer, index) }
end

def drag_and_drop
format_question_header + format_answers(@question.data) { |answer, index| format_traditional_answer(answer, index) }
end

def matching
format_question_header + format_answers(@question.data) { |answer, index| format_matching_answer(answer, index) }
end

def categorization_type
def categorization
format_question_header + format_categories(@question.data)
end

def bowtie_type
format_question_header + format_bowtie_sections
def bow_tie
format_question_header + format_bow_tie_sections
end

def stimulus_type
def stimulus_case_study
output = @question.child_questions.map { |sub_question| format_sub_question(sub_question) }
# removes additional line breaks from the sub-questions
output[-1] = output[-1].chomp if output.any?
"Question Type: #{question_type}\nQuestion: #{@question.text}\n\n#{output.join('')}"
"#{format_question_header}#{output.join('')}"
end

def format_sub_question(sub_question)
case sub_question.type
when "Question::Scenario"
if sub_question.type == "Question::Scenario"
"Scenario: #{sub_question.text}\n\n"
when "Question::Essay", "Question::Upload"
"#{QuestionTextFormatterService.new(sub_question).essay_type.chomp}\n\n"
when "Question::Traditional", "Question::SelectAllThatApply", "Question::DragAndDrop"
"#{QuestionTextFormatterService.new(sub_question).traditional_type}\n"
when "Question::Matching"
"#{QuestionTextFormatterService.new(sub_question).matching_type}\n"
when "Question::Categorization"
"#{QuestionTextFormatterService.new(sub_question).categorization_type}\n"
when "Question::BowTie"
"#{QuestionTextFormatterService.new(sub_question).bowtie_type}\n"
else
"#{self.class.new(sub_question, true).format_by_type}\n"
end
end

private

def format_question_header
"Question Type: #{question_type}\nQuestion: #{@question.text}\n\n"
def question_type
@question.class.type_name.titleize
end

def format_essay_content
Expand All @@ -90,7 +106,7 @@ def format_categories(data)
end.join("\n")
end

def format_bowtie_sections
def format_bow_tie_sections
sections = ['center', 'left', 'right'].map do |section|
answers = @question.data[section]['answers'].map.with_index do |answer, index|
format_traditional_answer(answer, index)
Expand All @@ -100,35 +116,11 @@ def format_bowtie_sections
sections.join("\n")
end

def question_type
type_name = @question.type.slice(10..-1).titleize
return 'Multiple Choice' if @question.type == 'Question::Traditional'
type_name
end

def format_html(html)
rich_text = Nokogiri::HTML(html)
rich_text.css('a').each { |link| link.replace("#{link.text} (#{link['href']})") }
rich_text.css('p').each { |p| p.replace("#{p.text}\n") }
rich_text.css('li').each { |li| li.replace("- #{li.text}\n") }
rich_text.text.strip
end

def format_by_type
case @question.type
when 'Question::Essay', 'Question::Upload'
essay_type
when 'Question::Traditional', 'Question::SelectAllThatApply', 'Question::DragAndDrop'
traditional_type
when 'Question::Matching'
matching_type
when 'Question::Categorization'
categorization_type
when 'Question::BowTie'
bowtie_type
when 'Question::StimulusCaseStudy'
stimulus_type
end
end
end
# rubocop:enable Metrics/ClassLength
17 changes: 17 additions & 0 deletions app/services/plain_text_formatter_service.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# frozen_string_literal: true

##
# Service to handle formatting questions into plain text

class PlainTextFormatterService < BaseFormatterService
protected

def content_divider
"\n==========\n\n"
end

def format_question_header
return "QUESTION TYPE: #{question_type}\nQUESTION: #{@question.text}\n\n" unless @subq
"Subquestion Type: #{question_type}\nSubquestion: #{@question.text}\n\n"
end
end
5 changes: 0 additions & 5 deletions spec/controllers/search_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -146,18 +146,13 @@
let(:question) { FactoryBot.create(:question_traditional) }
let(:user) { FactoryBot.create(:user) }

before do
sign_in user
end

it 'returns a text file' do
get :text_download
expect(response.content_type).to eq('text/plain')
expect(response.headers['Content-Disposition']).to include('questions.txt')
end

it 'includes bookmarked questions in the response' do
# Create a bookmark for the question
Bookmark.create!(user:, question:)

get :text_download
Expand Down
Loading

0 comments on commit 64b697d

Please sign in to comment.