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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Change census forms to post directly to census tables #19193

Merged
merged 6 commits into from
Nov 16, 2017
Merged

Change census forms to post directly to census tables #19193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2ab1a00
Add way to have duplicate school_info returned directly
Nov 15, 2017
0693cd5
Add email format validation to CensusSubmission
Nov 15, 2017
f40d483
Rename one_hour_per_week to less_than_one_hour_per_week
Nov 16, 2017
1f038af
Fix apostrophe in dont_know value
Nov 16, 2017
89b23b7
Create API to handle posting directly to census_submissions
Nov 16, 2017
405bc60
Change census forms to post to new API rather than pegasus forms
Nov 16, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 13 additions & 18 deletions apps/src/sites/hourofcode.com/pages/public/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@ $(document).ready(function () {
});

function checkShowCensusFollowUp() {
if ($("#twenty-hour-how-much").val() === "some" || $("#twenty-hour-how-much").val() === "all" || $("#ten-hour-how-much").val() === "some" ||
$("#ten-hour-how-much").val() === "all") {
if ($("#twenty-hour-how-much").val() === "SOME" || $("#twenty-hour-how-much").val() === "ALL" || $("#ten-hour-how-much").val() === "SOME" ||
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: break this long line up

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. From our styleguide:

Stick to an 80 character line length when possible, except for long URLs in comments or long regular expressions. Occasionally wiggling between 80 and 120 characters is OK in cases where it greatly improves the readability over a multiple line split.

$("#ten-hour-how-much").val() === "ALL") {
$('#followup_questions').show();
} else {
$('#followup_questions').hide();
Expand All @@ -140,7 +140,7 @@ $(document).ready(function () {
});

$('#role').change(function () {
if ($(this).val() === "teacher" || $(this).val() === "administrator") {
if ($(this).val() === "TEACHER" || $(this).val() === "ADMINISTRATOR") {
$('#pledge').show();
} else {
$('#pledge').hide();
Expand All @@ -153,23 +153,18 @@ function showCensusForm(data) {
$('#signup-header').hide();
$('#join-us-header').hide();
$('#submit').hide();
// Copy all of the hoc-signup inputs to the census form
$('.main-form :input').each(
function (index) {
var input = $(this);
var name = input.attr('name');
if (name !== undefined) {
var newInput = document.createElement("input");
newInput.value = input.val();
newInput.setAttribute("name", input.attr('name'));
newInput.setAttribute("type", "hidden");
$('#census-form').append(newInput);
}
}
);

$('#census-header').show();
$('#thanks-header').show();
$('#census-form').show();

// Copy relevant hoc-signup inputs to the census form
$('#census_email').val($('#hoc-email').val());
$('#census_name').val($('#hoc-name').val());
$('#census_school_id').val($('#nces_school').val());
$('#census_country').val($('#country').val());
$('#census_school_name').val($('#school-name').val());
$('#census_location').val($('#hoc-event-location').val());
}

function gotoThankYouPage() {
Expand Down Expand Up @@ -269,7 +264,7 @@ function censusFormSubmit() {
$("#census_submit").attr('disabled','disabled');

$.ajax({
url: "/forms/HocCensus" + hocYear,
url: "/dashboardapi/v1/census/CensusHoc2017v3",
type: "post",
dataType: "json",
data: $("#census-form").serialize()
Expand Down
74 changes: 44 additions & 30 deletions apps/src/templates/census2017/CensusForm.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,14 @@ class CensusForm extends Component {
checkShowFollowUp() {
const twentyHours = this.state.submission.twentyHours;
this.setState({
showFollowUp: (twentyHours === 'some' || twentyHours === 'all')
showFollowUp: (twentyHours === 'SOME' || twentyHours === 'ALL')
}, this.checkShowPledge);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like for HoC signup we also show the follow up questions if tenHours === "ALL". Is that also the case for /yourschool?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They actually have different checks. I'm already changing so much here that I didn't want to change this behavior also.


checkShowPledge() {
const role = this.state.submission.role;
this.setState({
showPledge: (role === 'teacher' || role === 'administrator')
showPledge: (role === 'TEACHER' || role === 'ADMINISTRATOR')
});
}

Expand Down Expand Up @@ -132,22 +132,36 @@ class CensusForm extends Component {
window.location.href = "/yourschool/thankyou";
}

// Here we're using the built-in functionality of pegasus form helpers to
// validate the email address. It's the only server-side validation for this
// form; all other validations are done client-side before the POST request is
// submitted. This slightly atypical approach because the logic for email
// validation is more complex and there wasn't a need to duplicate what already
// exists; the other validations are much more straightforward to implement
// here in the React.
// The response in the error case is JSON with an entry for
// each submitted field that is problematic. The specifics of
// the problem are not important here since we just need a boolean value
// of whether there was an error or not.
processError(error) {
if (error.responseJSON.email_s[0] === "invalid") {
const errorMap = {
submitter_email_address: 'invalidEmail',
class_frequency: 'frequency',
nces_school_s: 'nces',
submitter_role: 'role',
how_many_do_hoc: 'hoc',
how_many_after_school: 'afterSchool',
how_many_10_hours: 'tenHours',
how_many_20_hours: 'twentyHours',
country: 'country',
school_type: 'school',
state: 'school',
zip: 'school',
school_name: 'school'
};

const errorJSON = error.responseJSON;
Object.keys(errorJSON).map((key) => {
const errorKey = errorMap[key];
let newErrors = this.state.errors;
newErrors[errorKey] = true;
this.setState({
errors: {
...this.state.errors,
invalidEmail: true
}
errors: newErrors
});
}
});
}

validateSchoolDropdown() {
Expand Down Expand Up @@ -207,7 +221,7 @@ class CensusForm extends Component {
const { errors } = this.state;
if (!errors.email && !errors.topics && !errors.frequency && !errors.school && !errors.nces && !errors.role && !errors.hoc && !errors.afterSchool && !errors.tenHours && !errors.twentyHours && !errors.country) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe tighten with something like:

if (Object.values(errors).every(error => !error)) {
  // Submit...

$.ajax({
url: "/forms/Census2017",
url: "/dashboardapi/v1/census/CensusYourSchool2017v4",
Copy link
Contributor

@Erin007 Erin007 Nov 16, 2017 •
edited

Choose a reason for hiding this comment

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

should this match line 267 in apps/src/sites/hourofcode.com/pages/public/index.js? What are the different versions of the form?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The different form versions map to different models with different validation. They all write to the same table (census_submission) but with a different value for 'type'. This also allows us to map responses back to which version of the form was used.

type: "post",
dataType: "json",
data: $('#census-form').serialize()
Expand Down Expand Up @@ -243,7 +257,7 @@ class CensusForm extends Component {
{i18n.yourSchoolTellUs()}
</h2>
<form id="census-form">
<input type="hidden" id="version" name="version" value="4"/>
<input type="hidden" id="school_year" name="school_year" value="2017"/>
<CountryAutocompleteDropdown
onChange={this.handleDropdownChange.bind("country")}
value={submission.country}
Expand Down Expand Up @@ -300,7 +314,7 @@ class CensusForm extends Component {
)}
</div>
<select
name="hoc_s"
name="how_many_do_hoc"
Copy link
Contributor

Choose a reason for hiding this comment

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

So we don't need the data type _s appended to the name because that's an expectation of Pegasus forms, but not of your new table?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I changed the naming in the forms to match the models and controller rather than making the models and controller match the existing form fields.

Copy link
Contributor

Choose a reason for hiding this comment

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

The _s suffixes are for SOLR to understand the data types when we index Pegasus forms. We don't use SOLR on the dashboard side, so don't need it.

value={this.state.submission.hoc}
onChange={this.handleChange.bind(this, 'hoc')}
style={styles.dropdown}
Expand All @@ -327,7 +341,7 @@ class CensusForm extends Component {
)}
</div>
<select
name="after_school_s"
name="how_many_after_school"
value={this.state.submission.afterSchool}
onChange={this.handleChange.bind(this, 'afterSchool')}
style={styles.dropdown}
Expand All @@ -354,7 +368,7 @@ class CensusForm extends Component {
)}
</div>
<select
name="ten_hours_s"
name="how_many_10_hours"
value={this.state.submission.tenHours}
onChange={this.handleChange.bind(this, 'tenHours')}
style={styles.dropdown}
Expand All @@ -381,7 +395,7 @@ class CensusForm extends Component {
)}
</div>
<select
name="twenty_hours_s"
name="how_many_20_hours"
value={this.state.submission.twentyHours}
onChange={this.handleChange.bind(this, 'twentyHours')}
style={styles.dropdown}
Expand All @@ -401,7 +415,7 @@ class CensusForm extends Component {
<label>
<input
type="checkbox"
name="otherCS_b"
name="other_classes_under_20_hours"
checked={submission.otherCS}
onChange={() => this.toggleOtherCS()}
/>
Expand Down Expand Up @@ -435,14 +449,14 @@ class CensusForm extends Component {
&nbsp;
<input
type="text"
name="topic_other_desc_s"
name="topic_other_description"
value={this.state.otherTopicsDesc}
onChange={this.updateOtherTopicsDesc.bind(this)}
style={styles.inputInline}
/>
</div>
<div style={styles.leftMargin}>
{this.topicCheckbox("topic_dont_know_b", i18n.iDontKnow())}
{this.topicCheckbox("topic_do_not_know", i18n.iDontKnow())}
</div>
</div>
<label>
Expand All @@ -456,7 +470,7 @@ class CensusForm extends Component {
</div>
)}
<select
name="followup_frequency_s"
name="class_frequency"
value={this.state.submission.followUpFrequency}
onChange={this.handleChange.bind(this, 'followUpFrequency')}
style={styles.wideDropdown}
Expand All @@ -477,7 +491,7 @@ class CensusForm extends Component {
</div>
<textarea
type="text"
name="followup_more_s"
name="tell_us_more"
value={this.state.submission.followUpMore}
onChange={this.handleChange.bind(this, 'followUpMore')}
style={styles.textArea}
Expand All @@ -496,7 +510,7 @@ class CensusForm extends Component {
</div>
)}
<select
name="role_s"
name="submitter_role"
value={this.state.submission.role}
onChange={this.handleChange.bind(this, 'role')}
style={styles.wideDropdown}
Expand Down Expand Up @@ -529,7 +543,7 @@ class CensusForm extends Component {
)}
<input
type="text"
name="email_s"
name="submitter_email_address"
value={this.state.submission.email}
onChange={this.handleChange.bind(this, 'email')}
placeholder={i18n.yourEmailPlaceholder()}
Expand All @@ -544,7 +558,7 @@ class CensusForm extends Component {
</div>
<input
type="text"
name="name_s"
name="submitter_name"
Copy link
Contributor

Choose a reason for hiding this comment

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

submitter_name and pledged are much clearer than the previous names - thanks for updating 🙂

value={this.state.submission.name}
onChange={this.handleChange.bind(this, 'name')}
placeholder={i18n.yourName()}
Expand All @@ -557,7 +571,7 @@ class CensusForm extends Component {
<label>
<input
type="checkbox"
name="pledge_b"
name="pledged"
checked={submission.acceptedPledge}
onChange={() => this.togglePledge()}
/>
Expand Down
42 changes: 21 additions & 21 deletions apps/src/templates/census2017/censusQuestions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,61 +2,61 @@ import i18n from "@cdo/locale";

export const howManyStudents = [
{value: "", display: ""},
{value: "none", display: i18n.none()},
{value: "some", display: i18n.some()},
{value: "all", display: i18n.all()},
{value: "dont_know", display: i18n.iDontKnow()}
{value: "NONE", display: i18n.none()},
{value: "SOME", display: i18n.some()},
{value: "ALL", display: i18n.all()},
{value: "I DON'T KNOW", display: i18n.iDontKnow()}
];

export const roleOptions = [
{value: "", display: ""},
{value: "teacher", display: i18n.teacher()},
{value: "administrator", display: i18n.administrator()},
{value: "parent", display: i18n.parent()},
{value: "volunteer", display: i18n.volunteer()},
{value: "other", display: i18n.other()}
{value: "TEACHER", display: i18n.teacher()},
{value: "ADMINISTRATOR", display: i18n.administrator()},
{value: "PARENT", display: i18n.parent()},
{value: "VOLUNTEER", display: i18n.volunteer()},
{value: "OTHER", display: i18n.other()}
];

export const courseTopics = [{
name: "topic_blocks_b",
name: "topic_blocks",
label: i18n.censusBlockBased()
},
{
name: "topic_text_b",
name: "topic_text",
label: i18n.censusTextBased()
},
{
name: "topic_robots_b",
name: "topic_robots",
label: i18n.censusPhysicalComputing()
},
{
name: "topic_internet_b",
name: "topic_internet",
label: i18n.censusInternet()
},
{
name: "topic_security_b",
name: "topic_security",
label: i18n.censusCybersecurity()
},
{
name: "topic_data_b",
name: "topic_data",
label: i18n.censusDataAnalysis()
},
{
name: "topic_web_design_b",
name: "topic_web_design",
label: i18n.censusWebDesign()
},
{
name: "topic_game_design_b",
name: "topic_game_design",
label: i18n.censusGameDesign()
}
];

export const frequencyOptions = [
{value: "", display: ""},
{value: "less_than_one", display: i18n.censusFrequency1()},
{value: "one_to_three", display: i18n.censusFrequency1to3()},
{value: "three_plus", display: i18n.censusFrequency3plus()},
{value: "dont_know", display: i18n.iDontKnow()}
{value: "LESS THAN ONE HOUR PER WEEK", display: i18n.censusFrequency1()},
{value: "ONE TO THREE HOURS PER WEEK", display: i18n.censusFrequency1to3()},
{value: "THREE PLUS HOURS PER WEEK", display: i18n.censusFrequency3plus()},
{value: "I DON'T KNOW", display: i18n.iDontKnow()}
];

export const pledge = i18n.censusPledge();
8 changes: 4 additions & 4 deletions bin/oneoff/move_census_data.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,16 +184,16 @@ def cleanup_how_many(how_many)

def cleanup_frequency(frequency)
{
"less_than_one" => Census::CensusSubmission::CLASS_FREQUENCIES[:one_hour_per_week],
"less_than_one" => Census::CensusSubmission::CLASS_FREQUENCIES[:less_than_one_hour_per_week],
"one_to_three" => Census::CensusSubmission::CLASS_FREQUENCIES[:one_to_three_hours_per_week],
"three_plus" => Census::CensusSubmission::CLASS_FREQUENCIES[:three_plus_hours_per_week],
"dont_know" => Census::CensusSubmission::CLASS_FREQUENCIES[:dont_know],
"< 1 hour per week" => Census::CensusSubmission::CLASS_FREQUENCIES[:one_hour_per_week],
"< 1 hour per week" => Census::CensusSubmission::CLASS_FREQUENCIES[:less_than_one_hour_per_week],
"1-3 hours per week" => Census::CensusSubmission::CLASS_FREQUENCIES[:one_to_three_hours_per_week],
"3+ hours per week" => Census::CensusSubmission::CLASS_FREQUENCIES[:three_plus_hours_per_week],
"I don't know" => Census::CensusSubmission::CLASS_FREQUENCIES[:dont_know],
"haftada 1 saatten az" => Census::CensusSubmission::CLASS_FREQUENCIES[:one_hour_per_week],
"< 1 valanda per savaitę" => Census::CensusSubmission::CLASS_FREQUENCIES[:one_hour_per_week],
"haftada 1 saatten az" => Census::CensusSubmission::CLASS_FREQUENCIES[:less_than_one_hour_per_week],
"< 1 valanda per savaitę" => Census::CensusSubmission::CLASS_FREQUENCIES[:less_than_one_hour_per_week],
"1-3 Stunden pro Woche" => Census::CensusSubmission::CLASS_FREQUENCIES[:one_to_three_hours_per_week],
"1-3 uur per week" => Census::CensusSubmission::CLASS_FREQUENCIES[:one_to_three_hours_per_week],
"haftada 1 ile 3 saat arası" => Census::CensusSubmission::CLASS_FREQUENCIES[:one_to_three_hours_per_week],
Expand Down