Change census forms to post directly to census tables #19193

drewsamnick · 2017-11-16T01:51:30Z

These changes will allow us to bypass the pegasus FORMS table and write directly to the census tables.

There are a few parts:

New controller to expose the API for the ajax requests to post the data
Changes to the /yourschool and HoC census forms to post to the new API and to rename fields and values to be compatible.

This should have no impact to the HoC signup form.

drewsamnick · 2017-11-16T01:52:12Z

Save census data directly to new tables

Erin007 · 2017-11-16T16:34:53Z

apps/src/sites/hourofcode.com/pages/public/index.js

@@ -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" ||


nit: break this long line up

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.

Erin007 · 2017-11-16T16:38:52Z

apps/src/templates/census2017/CensusForm.jsx

@@ -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);
  }


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

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

Erin007 · 2017-11-16T16:41:01Z

apps/src/templates/census2017/CensusForm.jsx

@@ -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) {
      $.ajax({
-        url: "/forms/Census2017",
+        url: "/dashboardapi/v1/census/CensusYourSchool2017v4",


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

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.

Erin007 · 2017-11-16T16:47:50Z

apps/src/templates/census2017/CensusForm.jsx

@@ -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) {


Maybe tighten with something like:

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

Erin007 · 2017-11-16T16:49:14Z

apps/src/templates/census2017/CensusForm.jsx

@@ -300,7 +314,7 @@ class CensusForm extends Component {
                )}
              </div>
              <select
-                name="hoc_s"
+                name="how_many_do_hoc"


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?

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.

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.

Erin007 · 2017-11-16T16:50:39Z

apps/src/templates/census2017/CensusForm.jsx

@@ -544,7 +558,7 @@ class CensusForm extends Component {
              </div>
              <input
                type="text"
-                name="name_s"
+                name="submitter_name"


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

Erin007 · 2017-11-16T16:59:43Z

dashboard/test/controllers/api/v1/census/census_controller_test.rb

+    assert_response 201, @response.body.to_s
+    response = JSON.parse(@response.body)
+    refute response['census_submission_id'].nil?, "census_submission_id expected in response: #{response}"
+  end


Is it worth testing other variations of submissions beyond school? invalid email, etc?

Or confirming schools with punctuation succeed?

What do you mean by "schools with punctuation"?

Erin007

Looks good to me! A few questions, mostly for my own edification.

aoby · 2017-11-16T21:47:04Z

apps/src/sites/hourofcode.com/pages/public/index.js

@@ -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" ||


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.

aoby · 2017-11-16T21:49:30Z

apps/src/templates/census2017/CensusForm.jsx

@@ -300,7 +314,7 @@ class CensusForm extends Component {
                )}
              </div>
              <select
-                name="hoc_s"
+                name="how_many_do_hoc"


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.

aoby · 2017-11-16T21:52:42Z

dashboard/app/controllers/api/v1/census/census_controller.rb

+  ].freeze
+
+  # POST /dashboardapi/v1/census/<form_version>
+  def new_submission


nit: Any reason we're not using the standard create action?

Just lack of awareness on my part. I'll make this change.

Drew Samnick added 6 commits November 15, 2017 17:18

Add way to have duplicate school_info returned directly

2ab1a00

Add email format validation to CensusSubmission

0693cd5

Rename one_hour_per_week to less_than_one_hour_per_week

f40d483

Fix apostrophe in dont_know value

1f038af

Create API to handle posting directly to census_submissions

89b23b7

Change census forms to post to new API rather than pegasus forms

405bc60

drewsamnick requested review from breville, aoby and Erin007 November 16, 2017 01:51

Erin007 reviewed Nov 16, 2017

View reviewed changes

Erin007 approved these changes Nov 16, 2017

View reviewed changes

drewsamnick merged commit 0d17472 into staging Nov 16, 2017

aoby reviewed Nov 16, 2017

View reviewed changes

drewsamnick mentioned this pull request Nov 17, 2017

Fix passing school id to census form #19251

Merged

balderdash deleted the census-form-api branch September 20, 2018 17:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change census forms to post directly to census tables #19193

Change census forms to post directly to census tables #19193

drewsamnick commented Nov 16, 2017

drewsamnick commented Nov 16, 2017

Erin007 Nov 16, 2017

aoby Nov 16, 2017

Erin007 Nov 16, 2017

drewsamnick Nov 16, 2017

Erin007 Nov 16, 2017 •

edited

drewsamnick Nov 16, 2017

Erin007 Nov 16, 2017

Erin007 Nov 16, 2017

drewsamnick Nov 16, 2017

aoby Nov 16, 2017

Erin007 Nov 16, 2017

Erin007 Nov 16, 2017

Erin007 Nov 16, 2017

drewsamnick Nov 16, 2017

Erin007 left a comment

aoby Nov 16, 2017

aoby Nov 16, 2017

aoby Nov 16, 2017

drewsamnick Nov 16, 2017

		@@ -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) {

Change census forms to post directly to census tables #19193

Change census forms to post directly to census tables #19193

Conversation

drewsamnick commented Nov 16, 2017

drewsamnick commented Nov 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Erin007 Nov 16, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Erin007 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Erin007 Nov 16, 2017 •

edited