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

Teacher Application: Fixup old applications with new "Pay fee" question changes #27273

Merged
merged 6 commits into from Feb 27, 2019

Conversation

islemaster
Copy link
Contributor

@islemaster islemaster commented Feb 27, 2019

In #27203 I changed the possible answers to a teacher application question. I failed to consider that these changes would make existing application records invalid, and did not know that we depend on the ability to update these records later, after the principal approves them. We learned about the issue through this Honeybadger error.

The possible answers to that question changed like this:

Before After
Yes, my school or I will be able to pay the full program fee. Yes, my school will be able to pay the full program fee.
No, my school or I will not be able to pay the program fee. I would like to be considered for a scholarship. No, my school will not be able to pay the program fee. I would like to be considered for a scholarship.
Not applicable: there is no program fee for teachers in my region. Removed
Not applicable: there is no Regional Partner in my region. Removed
I don't know. I don't know.

Our fix for this issue has two parts:

  1. Backfill script
    We are running a script to update previous teacher applications so that their answers match the new set of available answers.
    • For the first two answers, where we simply removed the "or I" from the answer, we are updating the answer text (and assuming the basic meaning of the answer is the same).
  • For the removed answers, we are simply clearing the field rather than try to coerce to one of the first two answers and risk answering incorrectly on behalf of a teacher.
  1. Question no longer required on the server
    Because the above backfill leaves some applications with no answer to a conditionally-required question, we are removing the required-ness of this question on the server to bring all applications back into a valid state. The question is still required on the client for new applications.

This PR includes a script for (1) and the code changes for (2). We'll ship them together and then manually run the script on production.

if hash[:pay_fee] == 'Yes, my school or I will be able to pay the full program fee.'
hash[:pay_fee] = 'Yes, my school will be able to pay the full program fee.'
elsif hash[:pay_fee] == 'Yes, my school or I will be able to pay the full program fee.'
hash[:pay_fee] = 'Yes, my school will be able to pay the full program fee.'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This second clause is a mistake! Wrong text. I will fix.

next
end

if application.update_column :form_data, hash.to_json

Choose a reason for hiding this comment

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

small thing: form.rb has a method called update_form_data_hash

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated!

@hacodeorg
Copy link
Contributor

I don't have any comment on the code, it looks good. However, going back to update previous answers feels uneasy. Once something is written down it shouldn't be changed. It could be appended though. I don't have a concrete idea how to satisfy all the following requirements yet: 1) be flexible in changing questions+answers; 2) do not change historical data; and 3) be simple and do not over-engineering.

@islemaster
Copy link
Contributor Author

I totally agree @hacodeorg - I think it's a broader conversation than this immediate need. It would be nice to have a formal way to "version" applications to handle changes like this. We've also discussed not wanting to create a new model for the application every year, even though we have a lot of question changes. I think it's a little late to rethink our approach for this year's application, but for next year (building out probably August-September) let's reconsider how we handle changes like this 👍 .

@islemaster islemaster merged commit e83cde4 into staging Feb 27, 2019
@islemaster islemaster deleted the brad-pay-fee-backfill branch February 27, 2019 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants