From 74817fb45bca710ab14b8d9ec95aea07644559c8 Mon Sep 17 00:00:00 2001 From: Brendan Reville Date: Thu, 21 Jun 2018 20:20:27 +1000 Subject: [PATCH 01/89] PD: international opt-in form --- apps/Gruntfile.js | 2 + .../InternationalOptin.jsx | 180 ++++++++++++++++++ .../pages/pd/international_optin/new.js | 12 ++ .../v1/pd/international_optins_controller.rb | 15 ++ .../pd/international_optin_controller.rb | 18 ++ .../app/models/pd/international_optin.rb | 58 ++++++ .../pd/international_optin/new.html.haml | 14 ++ .../pd/international_optin/thanks.html.haml | 1 + dashboard/config/routes.rb | 4 + ...20110434_create_pd_international_optins.rb | 9 + dashboard/db/schema.rb | 10 +- .../test/factories/pd_international_optins.rb | 19 ++ .../models/pd/international_optin_test.rb | 7 + lib/cdo/email_preference_constants.rb | 1 + lib/international_optin_people.rb | 19 ++ 15 files changed, 368 insertions(+), 1 deletion(-) create mode 100644 apps/src/code-studio/pd/international_optin/InternationalOptin.jsx create mode 100644 apps/src/sites/studio/pages/pd/international_optin/new.js create mode 100644 dashboard/app/controllers/api/v1/pd/international_optins_controller.rb create mode 100644 dashboard/app/controllers/pd/international_optin_controller.rb create mode 100644 dashboard/app/models/pd/international_optin.rb create mode 100644 dashboard/app/views/pd/international_optin/new.html.haml create mode 100644 dashboard/app/views/pd/international_optin/thanks.html.haml create mode 100644 dashboard/db/migrate/20180620110434_create_pd_international_optins.rb create mode 100644 dashboard/test/factories/pd_international_optins.rb create mode 100644 dashboard/test/models/pd/international_optin_test.rb create mode 100644 lib/international_optin_people.rb diff --git a/apps/Gruntfile.js b/apps/Gruntfile.js index ac156353626e4..de1891740185b 100644 --- a/apps/Gruntfile.js +++ b/apps/Gruntfile.js @@ -513,6 +513,8 @@ describe('entry tests', () => { 'pd/professional_learning_landing/index': './src/sites/studio/pages/pd/professional_learning_landing/index.js', 'pd/regional_partner_contact/new': './src/sites/studio/pages/pd/regional_partner_contact/new.js', + 'pd/international_optin/new': './src/sites/studio/pages/pd/international_optin/new.js', + 'peer_reviews/dashboard': './src/sites/studio/pages/peer_reviews/dashboard.js', 'code.org/public/teacher-dashboard/index': './src/sites/code.org/pages/public/teacher-dashboard/index.js', diff --git a/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx b/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx new file mode 100644 index 0000000000000..2522a2c5f21de --- /dev/null +++ b/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx @@ -0,0 +1,180 @@ +import React from 'react'; +import FormController from '../form_components/FormController'; +import FormComponent from '../form_components/FormComponent'; +import {FormGroup} from 'react-bootstrap'; + +export default class InternationalOptin extends FormController { + /** + * @override + */ + onSuccessfulSubmit(data) { + window.location = `/pd/international_optin/${data.id}/thanks`; + } + + /** + * @override + */ + serializeFormData() { + const formData = super.serializeFormData(); + //Object.assign(formData['form_data'], this.getDistrictData()); + + return formData; + } + + /** + * @override + */ + getPageComponents() { + return [ + InternationalOptinComponent + ]; + } +} + +class InternationalOptinComponent extends FormComponent { + render() { + return ( + + { + this.buildFieldGroup({ + name: 'firstName', + label: 'First Name', + type: 'text', + required: true + }) + } + { + this.buildFieldGroup({ + name: 'firstNamePreferred', + label: 'Preferred First Name', + type: 'text', + required: false + }) + } + { + this.buildFieldGroup({ + name: 'lastName', + label: 'Last Name', + type: 'text', + required: true + }) + } + { + this.buildFieldGroup({ + name: 'email', + label: 'Email', + type: 'text', + required: true + }) + } + { + this.buildFieldGroup({ + name: 'emailAlternate', + label: 'Alternate Email', + type: 'text', + required: true + }) + } + { + this.buildButtonsFromOptions({ + name: 'gender', + label: 'Gender identity', + type: 'radio', + required: true + }) + } + { + this.buildFieldGroup({ + name: 'schoolName', + label: 'School Name', + type: 'text', + required: true + }) + } + { + this.buildFieldGroup({ + name: 'schoolCity', + label: 'School City', + type: 'text', + required: true + }) + } + { + this.buildButtonsFromOptions({ + name: 'schoolCountry', + label: 'School Country', + type: 'radio', + required: true + }) + } + { + this.buildButtonsFromOptions({ + name: 'ages', + label: 'Which age(s) do you teach this year?', + type: 'check', + required: true + }) + } + { + this.buildButtonsFromOptions({ + name: 'subjects', + label: 'Which subject(s) do you teach this year?', + type: 'check', + required: true, + includeOther: true + }) + } + { + this.buildButtonsFromOptions({ + name: 'resources', + label: 'Which of the following CS education resources do you use?', + type: 'check', + required: true, + includeOther: true + }) + } + { + this.buildButtonsFromOptions({ + name: 'robotics', + label: 'Which of the following robotics resources do you use?', + type: 'check', + required: true, + includeOther: true + }) + } + { + this.buildSelectFieldGroupFromOptions({ + name: 'workshopOrganizer', + label: 'Workshop Organizer', + required: true + }) + } + { + this.buildSelectFieldGroupFromOptions({ + name: 'workshopFacilitator', + label: 'Workshop Facilitator', + required: true + }) + } + { + this.buildSelectFieldGroupFromOptions({ + name: 'workshopCourse', + label: 'Workshop Course', + required: true + }) + } + { + this.buildButtonsFromOptions({ + name: 'optIn', + label: 'I want to share my data with Code.org, International Partner', + type: 'radio', + required: true + }) + } + + ); + } +} + +InternationalOptinComponent.associatedFields = + ['firstName', 'lastName', 'title', 'email', 'optIn']; diff --git a/apps/src/sites/studio/pages/pd/international_optin/new.js b/apps/src/sites/studio/pages/pd/international_optin/new.js new file mode 100644 index 0000000000000..b642c1bc7a040 --- /dev/null +++ b/apps/src/sites/studio/pages/pd/international_optin/new.js @@ -0,0 +1,12 @@ +import React from 'react'; +import ReactDOM from 'react-dom'; +import InternationalOptin from '@cdo/apps/code-studio/pd/international_optin/InternationalOptin'; +import getScriptData from '@cdo/apps/util/getScriptData'; + +document.addEventListener("DOMContentLoaded", function (event) { + ReactDOM.render( + , document.getElementById('application-container') + ); +}); diff --git a/dashboard/app/controllers/api/v1/pd/international_optins_controller.rb b/dashboard/app/controllers/api/v1/pd/international_optins_controller.rb new file mode 100644 index 0000000000000..f1898c1fdefa9 --- /dev/null +++ b/dashboard/app/controllers/api/v1/pd/international_optins_controller.rb @@ -0,0 +1,15 @@ +class Api::V1::Pd::InternationalOptinsController < Api::V1::Pd::FormsController + def new_form + @contact_form = ::Pd::InternationalOptin.new + end + + def on_successful_create + EmailPreference.upsert!( + email: @contact_form.email, + opt_in: @contact_form.opt_in?, + ip_address: request.ip, + source: EmailPreference::FORM_PD_INTERNATIONAL_OPTIN, + form_kind: "0" + ) + end +end diff --git a/dashboard/app/controllers/pd/international_optin_controller.rb b/dashboard/app/controllers/pd/international_optin_controller.rb new file mode 100644 index 0000000000000..3befa19bcc44b --- /dev/null +++ b/dashboard/app/controllers/pd/international_optin_controller.rb @@ -0,0 +1,18 @@ +class Pd::InternationalOptinController < ApplicationController + load_resource :international_optin, class: 'Pd::InternationalOptin', id_param: :contact_id, only: [:thanks] + + # GET /pd/international_optins/new + def new + @script_data = { + props: { + options: Pd::InternationalOptin.options.camelize_keys, + apiEndpoint: "/api/v1/pd/international_optins" + }.to_json + } + end + + # Get /pd/international_optins/:contact_id/thanks + def thanks + @regional_partner = @international_optin.try(:regional_partner) + end +end diff --git a/dashboard/app/models/pd/international_optin.rb b/dashboard/app/models/pd/international_optin.rb new file mode 100644 index 0000000000000..299962782848e --- /dev/null +++ b/dashboard/app/models/pd/international_optin.rb @@ -0,0 +1,58 @@ +# == Schema Information +# +# Table name: pd_international_optins +# +# id :integer not null, primary key +# user_id :integer +# form_data :text(65535) +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# index_pd_international_optins_on_user_id (user_id) +# + +require 'international_optin_people' + +class Pd::InternationalOptin < ApplicationRecord + include Pd::Form + + def self.required_fields + [ + :first_name, + :last_name, + :email, + :opt_in + ] + end + + def self.options + super.merge( + { + title: %w(Mr. Mrs. Ms. Dr.), + role: ['Teacher', 'School Administrator', 'District Administrator'], + gradeLevels: ['High School', 'Middle School', 'Elementary School'], + program: ['CS Fundamentals (Pre-K - 5th grade)', 'CS Discoveries (6 - 10th grade)', 'CS Principles (appropriate for 9th - 12th grade, and can be implemented as an AP or introductory course)'], + optIn: ['Yes', 'No'], + gender: ['Female', 'Male', 'Other', 'Prefer not to answer'], + ages: ['< 6 years old', '7-8 years old', '9-10 years old', '11-12 years old', '13-14 years old', '15-16 years old', '17-18 years old', '19+ years old'], + subjects: ['Computer Science', 'ICT', 'Math', 'Science', 'History / Social Studies', 'Language Arts', 'English as a Foreign Language', 'Music', 'Art'], + resources: ['Bootstrap', 'CodeCademy', 'Google CS First', 'Khan Academy', 'Kodable', 'Lightbot', 'Scratch', 'Tynker'], + robotics: ['Grok Learning', 'Kodable', 'LEGO Education', 'Microbit', 'Ozobot', 'Sphero', 'Raspberry Pi', 'Wonder Workshop'], + schoolCountry: %w(Canada Chile Israel Malaysia Mexico Thailand), + workshopOrganizer: get_international_optin_partners, + workshopFacilitator: get_international_optin_facilitators, + workshopCourse: ['CS Fundamentals (Courses A-F)', 'CS Fundamentals (Pre-Express or Express)'] + } + ) + end + + def email + sanitize_form_data_hash[:email] + end + + def opt_in? + sanitize_form_data_hash[:opt_in].downcase == "yes" + end +end diff --git a/dashboard/app/views/pd/international_optin/new.html.haml b/dashboard/app/views/pd/international_optin/new.html.haml new file mode 100644 index 0000000000000..df4156ab0c1cc --- /dev/null +++ b/dashboard/app/views/pd/international_optin/new.html.haml @@ -0,0 +1,14 @@ +- @page_title = 'International opt-in' + +- content_for(:head) do + = stylesheet_link_tag 'css/pd', media: 'all' +%script{src: minifiable_asset_path('js/pd/international_optin/new.js'), data: @script_data} + +#application-header + %h3 + International opt-in + +%p + This page is for international opt-in. + +%div#application-container diff --git a/dashboard/app/views/pd/international_optin/thanks.html.haml b/dashboard/app/views/pd/international_optin/thanks.html.haml new file mode 100644 index 0000000000000..655f2281dea9e --- /dev/null +++ b/dashboard/app/views/pd/international_optin/thanks.html.haml @@ -0,0 +1 @@ +Thanks for doing an international opt-in! diff --git a/dashboard/config/routes.rb b/dashboard/config/routes.rb index fde1b95ae2581..99706de9004f8 100644 --- a/dashboard/config/routes.rb +++ b/dashboard/config/routes.rb @@ -447,6 +447,7 @@ module OPS post :workshop_surveys, to: 'workshop_surveys#create' post :teachercon_surveys, to: 'teachercon_surveys#create' post :regional_partner_contacts, to: 'regional_partner_contacts#create' + post :international_optins, to: 'international_optins#create' get :regional_partner_workshops, to: 'regional_partner_workshops#index' get 'regional_partner_workshops/find', to: 'regional_partner_workshops#find' @@ -544,6 +545,9 @@ module OPS get 'regional_partner_contact/new', to: 'regional_partner_contact#new' get 'regional_partner_contact/:contact_id/thanks', to: 'regional_partner_contact#thanks' + get 'international_optin/new', to: 'international_optin#new' + get 'international_optin/:contact_id/thanks', to: 'international_optin#thanks' + # React-router will handle sub-routes on the client. get 'application_dashboard/*path', to: 'application_dashboard#index' get 'application_dashboard', to: 'application_dashboard#index' diff --git a/dashboard/db/migrate/20180620110434_create_pd_international_optins.rb b/dashboard/db/migrate/20180620110434_create_pd_international_optins.rb new file mode 100644 index 0000000000000..24ebc8231822d --- /dev/null +++ b/dashboard/db/migrate/20180620110434_create_pd_international_optins.rb @@ -0,0 +1,9 @@ +class CreatePdInternationalOptins < ActiveRecord::Migration[5.0] + def change + create_table :pd_international_optins do |t| + t.references :user, index: true + t.text :form_data + t.timestamps null: false + end + end +end diff --git a/dashboard/db/schema.rb b/dashboard/db/schema.rb index 03f573cf7963c..388825947f9b3 100644 --- a/dashboard/db/schema.rb +++ b/dashboard/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20180607153724) do +ActiveRecord::Schema.define(version: 20180620110434) do create_table "activities", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci" do |t| t.integer "user_id" @@ -672,6 +672,14 @@ t.index ["pd_application_id"], name: "index_pd_fit_weekend1819_registrations_on_pd_application_id", using: :btree end + create_table "pd_international_optins", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci" do |t| + t.integer "user_id" + t.text "form_data", limit: 65535 + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["user_id"], name: "index_pd_international_optins_on_user_id", using: :btree + end + create_table "pd_payment_terms", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci" do |t| t.integer "regional_partner_id", null: false t.date "start_date", null: false diff --git a/dashboard/test/factories/pd_international_optins.rb b/dashboard/test/factories/pd_international_optins.rb new file mode 100644 index 0000000000000..acdd16330dbb0 --- /dev/null +++ b/dashboard/test/factories/pd_international_optins.rb @@ -0,0 +1,19 @@ +# == Schema Information +# +# Table name: pd_international_optins +# +# id :integer not null, primary key +# user_id :integer +# form_data :text(65535) +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# index_pd_international_optins_on_user_id (user_id) +# + +FactoryGirl.define do + factory :pd_international_optin, class: 'Pd::InternationalOptin' do + end +end diff --git a/dashboard/test/models/pd/international_optin_test.rb b/dashboard/test/models/pd/international_optin_test.rb new file mode 100644 index 0000000000000..91e92117e5304 --- /dev/null +++ b/dashboard/test/models/pd/international_optin_test.rb @@ -0,0 +1,7 @@ +require 'test_helper' + +class Pd::InternationalOptinTest < ActiveSupport::TestCase + # test "the truth" do + # assert true + # end +end diff --git a/lib/cdo/email_preference_constants.rb b/lib/cdo/email_preference_constants.rb index 04357d226725e..033655c860fc1 100644 --- a/lib/cdo/email_preference_constants.rb +++ b/lib/cdo/email_preference_constants.rb @@ -15,6 +15,7 @@ module EmailPreferenceConstants FORM_HOC_SIGN_UP = 'FORM_HOC_SIGN_UP'.freeze, FORM_CLASS_SUBMIT = 'FORM_CLASS_SUBMIT'.freeze, FORM_PETITION = 'FORM_PETITION'.freeze, + FORM_PD_INTERNATIONAL_OPTIN = 'FORM_PD_INTERNATIONAL_OPTIN'.freeze, # A one-time automated script sets all Petition submissions younger than 16 and older than 13 to opted out to comply # with GDPR. AUTOMATED_OPT_OUT_UNDER_16 = 'AUTOMATED_OPT_OUT_UNDER_16'.freeze diff --git a/lib/international_optin_people.rb b/lib/international_optin_people.rb new file mode 100644 index 0000000000000..27713f7457533 --- /dev/null +++ b/lib/international_optin_people.rb @@ -0,0 +1,19 @@ +INTERNATIONAL_OPTIN_FACILITATORS = [ + "First facilitator", + "Second facilitator", + "Third faciliattor" +].freeze + +INTERNATIONAL_OPTIN_PARTNERS = [ + "First partner", + "Second partner", + "Third partner" +].freeze + +def get_international_optin_facilitators + INTERNATIONAL_OPTIN_FACILITATORS +end + +def get_international_optin_partners + INTERNATIONAL_OPTIN_PARTNERS +end From 12feb46262490538ce62686b623cb1d1e28ff397 Mon Sep 17 00:00:00 2001 From: Brendan Reville Date: Fri, 22 Jun 2018 12:37:16 +1000 Subject: [PATCH 02/89] PD: International opt-in work --- .../InternationalOptin.jsx | 34 +++++++++++++++---- .../pd/international_optin_controller.rb | 4 +++ .../app/models/pd/international_optin.rb | 2 +- 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx b/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx index 2522a2c5f21de..10b3e093e0c5b 100644 --- a/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx +++ b/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx @@ -1,9 +1,13 @@ -import React from 'react'; +import React, {PropTypes} from 'react'; import FormController from '../form_components/FormController'; import FormComponent from '../form_components/FormComponent'; import {FormGroup} from 'react-bootstrap'; export default class InternationalOptin extends FormController { + static propTypes = { + accountEmail: PropTypes.string.isRequired + }; + /** * @override */ @@ -29,9 +33,23 @@ export default class InternationalOptin extends FormController { InternationalOptinComponent ]; } + + /** + * @override + */ + getPageProps() { + return { + ...super.getPageProps(), + accountEmail: this.props.accountEmail + }; + } } class InternationalOptinComponent extends FormComponent { + static propTypes = { + accountEmail: PropTypes.string.isRequired + }; + render() { return ( @@ -59,26 +77,28 @@ class InternationalOptinComponent extends FormComponent { required: true }) } + { this.buildFieldGroup({ name: 'email', label: 'Email', type: 'text', - required: true + value: this.props.accountEmail, + readOnly: true }) } + { this.buildFieldGroup({ name: 'emailAlternate', label: 'Alternate Email', - type: 'text', - required: true + type: 'text' }) } { this.buildButtonsFromOptions({ name: 'gender', - label: 'Gender identity', + label: 'Gender Identity', type: 'radio', required: true }) @@ -129,7 +149,7 @@ class InternationalOptinComponent extends FormComponent { name: 'resources', label: 'Which of the following CS education resources do you use?', type: 'check', - required: true, + required: false, includeOther: true }) } @@ -138,7 +158,7 @@ class InternationalOptinComponent extends FormComponent { name: 'robotics', label: 'Which of the following robotics resources do you use?', type: 'check', - required: true, + required: false, includeOther: true }) } diff --git a/dashboard/app/controllers/pd/international_optin_controller.rb b/dashboard/app/controllers/pd/international_optin_controller.rb index 3befa19bcc44b..3fb6d5dee3e13 100644 --- a/dashboard/app/controllers/pd/international_optin_controller.rb +++ b/dashboard/app/controllers/pd/international_optin_controller.rb @@ -3,9 +3,13 @@ class Pd::InternationalOptinController < ApplicationController # GET /pd/international_optins/new def new + return render '/pd/application/teacher_application/logged_out' unless current_user + return render '/pd/application/teacher_application/not_teacher' unless current_user.teacher? + @script_data = { props: { options: Pd::InternationalOptin.options.camelize_keys, + accountEmail: current_user.email, apiEndpoint: "/api/v1/pd/international_optins" }.to_json } diff --git a/dashboard/app/models/pd/international_optin.rb b/dashboard/app/models/pd/international_optin.rb index 299962782848e..473901457bf04 100644 --- a/dashboard/app/models/pd/international_optin.rb +++ b/dashboard/app/models/pd/international_optin.rb @@ -35,7 +35,7 @@ def self.options gradeLevels: ['High School', 'Middle School', 'Elementary School'], program: ['CS Fundamentals (Pre-K - 5th grade)', 'CS Discoveries (6 - 10th grade)', 'CS Principles (appropriate for 9th - 12th grade, and can be implemented as an AP or introductory course)'], optIn: ['Yes', 'No'], - gender: ['Female', 'Male', 'Other', 'Prefer not to answer'], + gender: ['Male', 'Female', 'Non-binary', 'Preferred term not listed', 'Prefer not to answer'], ages: ['< 6 years old', '7-8 years old', '9-10 years old', '11-12 years old', '13-14 years old', '15-16 years old', '17-18 years old', '19+ years old'], subjects: ['Computer Science', 'ICT', 'Math', 'Science', 'History / Social Studies', 'Language Arts', 'English as a Foreign Language', 'Music', 'Art'], resources: ['Bootstrap', 'CodeCademy', 'Google CS First', 'Khan Academy', 'Kodable', 'Lightbot', 'Scratch', 'Tynker'], From ded35b6e49cf6246fc247cbe48e69c4ceaabf7e4 Mon Sep 17 00:00:00 2001 From: Brendan Reville Date: Sat, 23 Jun 2018 22:06:17 +1000 Subject: [PATCH 03/89] PD: international opt-in form gets loc'ed labels --- .../InternationalOptin.jsx | 42 ++++++++++--------- .../pd/international_optin_controller.rb | 3 +- .../app/models/pd/international_optin.rb | 25 +++++++++++ dashboard/config/locales/en.yml | 18 ++++++++ 4 files changed, 68 insertions(+), 20 deletions(-) diff --git a/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx b/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx index 10b3e093e0c5b..e8109d5f4c4eb 100644 --- a/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx +++ b/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx @@ -5,7 +5,8 @@ import {FormGroup} from 'react-bootstrap'; export default class InternationalOptin extends FormController { static propTypes = { - accountEmail: PropTypes.string.isRequired + accountEmail: PropTypes.string.isRequired, + labels: PropTypes.object.isRequired }; /** @@ -40,7 +41,8 @@ export default class InternationalOptin extends FormController { getPageProps() { return { ...super.getPageProps(), - accountEmail: this.props.accountEmail + accountEmail: this.props.accountEmail, + labels: this.props.labels }; } } @@ -51,12 +53,14 @@ class InternationalOptinComponent extends FormComponent { }; render() { + const labels = this.props.labels; + return ( { this.buildFieldGroup({ name: 'firstName', - label: 'First Name', + label: labels.firstName, type: 'text', required: true }) @@ -64,7 +68,7 @@ class InternationalOptinComponent extends FormComponent { { this.buildFieldGroup({ name: 'firstNamePreferred', - label: 'Preferred First Name', + label: labels.firstNamePreferred, type: 'text', required: false }) @@ -72,7 +76,7 @@ class InternationalOptinComponent extends FormComponent { { this.buildFieldGroup({ name: 'lastName', - label: 'Last Name', + label: labels.lastName, type: 'text', required: true }) @@ -81,7 +85,7 @@ class InternationalOptinComponent extends FormComponent { { this.buildFieldGroup({ name: 'email', - label: 'Email', + label: labels.email, type: 'text', value: this.props.accountEmail, readOnly: true @@ -91,14 +95,14 @@ class InternationalOptinComponent extends FormComponent { { this.buildFieldGroup({ name: 'emailAlternate', - label: 'Alternate Email', + label: labels.emailAlternate, type: 'text' }) } { this.buildButtonsFromOptions({ name: 'gender', - label: 'Gender Identity', + label: labels.gender, type: 'radio', required: true }) @@ -106,7 +110,7 @@ class InternationalOptinComponent extends FormComponent { { this.buildFieldGroup({ name: 'schoolName', - label: 'School Name', + label: labels.schoolName, type: 'text', required: true }) @@ -114,7 +118,7 @@ class InternationalOptinComponent extends FormComponent { { this.buildFieldGroup({ name: 'schoolCity', - label: 'School City', + label: labels.schoolCity, type: 'text', required: true }) @@ -122,7 +126,7 @@ class InternationalOptinComponent extends FormComponent { { this.buildButtonsFromOptions({ name: 'schoolCountry', - label: 'School Country', + label: labels.schoolCountry, type: 'radio', required: true }) @@ -130,7 +134,7 @@ class InternationalOptinComponent extends FormComponent { { this.buildButtonsFromOptions({ name: 'ages', - label: 'Which age(s) do you teach this year?', + label: labels.ages, type: 'check', required: true }) @@ -138,7 +142,7 @@ class InternationalOptinComponent extends FormComponent { { this.buildButtonsFromOptions({ name: 'subjects', - label: 'Which subject(s) do you teach this year?', + label: labels.subjects, type: 'check', required: true, includeOther: true @@ -147,7 +151,7 @@ class InternationalOptinComponent extends FormComponent { { this.buildButtonsFromOptions({ name: 'resources', - label: 'Which of the following CS education resources do you use?', + label: labels.resources, type: 'check', required: false, includeOther: true @@ -156,7 +160,7 @@ class InternationalOptinComponent extends FormComponent { { this.buildButtonsFromOptions({ name: 'robotics', - label: 'Which of the following robotics resources do you use?', + label: labels.robotics, type: 'check', required: false, includeOther: true @@ -165,28 +169,28 @@ class InternationalOptinComponent extends FormComponent { { this.buildSelectFieldGroupFromOptions({ name: 'workshopOrganizer', - label: 'Workshop Organizer', + label: labels.workshopOrganizer, required: true }) } { this.buildSelectFieldGroupFromOptions({ name: 'workshopFacilitator', - label: 'Workshop Facilitator', + label: labels.workshopFacilitator, required: true }) } { this.buildSelectFieldGroupFromOptions({ name: 'workshopCourse', - label: 'Workshop Course', + label: labels.workshopCourse, required: true }) } { this.buildButtonsFromOptions({ name: 'optIn', - label: 'I want to share my data with Code.org, International Partner', + label: labels.optIn, type: 'radio', required: true }) diff --git a/dashboard/app/controllers/pd/international_optin_controller.rb b/dashboard/app/controllers/pd/international_optin_controller.rb index 3fb6d5dee3e13..7efa36ba3de79 100644 --- a/dashboard/app/controllers/pd/international_optin_controller.rb +++ b/dashboard/app/controllers/pd/international_optin_controller.rb @@ -10,7 +10,8 @@ def new props: { options: Pd::InternationalOptin.options.camelize_keys, accountEmail: current_user.email, - apiEndpoint: "/api/v1/pd/international_optins" + apiEndpoint: "/api/v1/pd/international_optins", + labels: Pd::InternationalOptin.labels }.to_json } end diff --git a/dashboard/app/models/pd/international_optin.rb b/dashboard/app/models/pd/international_optin.rb index 473901457bf04..df2f983ddff82 100644 --- a/dashboard/app/models/pd/international_optin.rb +++ b/dashboard/app/models/pd/international_optin.rb @@ -48,6 +48,31 @@ def self.options ) end + def self.labels + keys = %w( + firstName + preferredFirstName + firstName + firstNamePreferred + lastName + email + emailAlternate + gender + schoolName + schoolCity + schoolCountry + ages + subjects + resources + robotics + workshopOrganizer + workshopFacilitator + workshopCourse + optIn + ) + Hash[keys.collect {|v| [v, I18n.t("pd.form_labels.#{v.underscore}")]}] + end + def email sanitize_form_data_hash[:email] end diff --git a/dashboard/config/locales/en.yml b/dashboard/config/locales/en.yml index 57c6787990ce7..374b85e8128b0 100644 --- a/dashboard/config/locales/en.yml +++ b/dashboard/config/locales/en.yml @@ -1021,6 +1021,24 @@ en: things_you_liked_s: 'What were the two things you liked most about the activities you did in this workshop and why?' things_you_would_change_s: 'What are the two things you would change about the activities you did in this workshop?' anything_else_s: 'Is there anything else you’d like to tell us about your experience at this workshop?' + form_labels: + first_name: 'First Name' + first_name_preferred: 'Preferred First Name' + last_name: 'Last Name' + email: 'Email' + email_alternate: 'Alternate Email' + gender: 'Gender Identity' + school_name: 'School Name' + school_city: 'School City' + school_country: 'School Country' + ages: 'Which age(s) do you teach this year?' + subjects: 'Which subject(s) do you teach this year?' + resources: 'Which of the following CS education resources do you use?' + robotics: 'Which of the following robotics resources do you use?' + workshop_organizer: 'Workshop Organizer' + workshop_facilitator: 'Workshop Facilitator' + workshop_course: 'Workshop Course' + opt_in: 'I want to share my data with Code.org, International Partner' courses_category: 'Full Courses' cookie_banner: message: 'By continuing to browse our site or clicking "I agree," you agree to the storing of cookies on your computer or device.' From 1359b0a1e7e32f9cdd0b3a493406255b4cf2994a Mon Sep 17 00:00:00 2001 From: Brendan Reville Date: Mon, 25 Jun 2018 17:25:42 +1000 Subject: [PATCH 04/89] PD: international opt-in gets loc'ed options --- .../app/models/pd/international_optin.rb | 35 ++++++------ dashboard/config/locales/en.yml | 57 +++++++++++++++++++ 2 files changed, 74 insertions(+), 18 deletions(-) diff --git a/dashboard/app/models/pd/international_optin.rb b/dashboard/app/models/pd/international_optin.rb index df2f983ddff82..1b03a4f441912 100644 --- a/dashboard/app/models/pd/international_optin.rb +++ b/dashboard/app/models/pd/international_optin.rb @@ -28,24 +28,23 @@ def self.required_fields end def self.options - super.merge( - { - title: %w(Mr. Mrs. Ms. Dr.), - role: ['Teacher', 'School Administrator', 'District Administrator'], - gradeLevels: ['High School', 'Middle School', 'Elementary School'], - program: ['CS Fundamentals (Pre-K - 5th grade)', 'CS Discoveries (6 - 10th grade)', 'CS Principles (appropriate for 9th - 12th grade, and can be implemented as an AP or introductory course)'], - optIn: ['Yes', 'No'], - gender: ['Male', 'Female', 'Non-binary', 'Preferred term not listed', 'Prefer not to answer'], - ages: ['< 6 years old', '7-8 years old', '9-10 years old', '11-12 years old', '13-14 years old', '15-16 years old', '17-18 years old', '19+ years old'], - subjects: ['Computer Science', 'ICT', 'Math', 'Science', 'History / Social Studies', 'Language Arts', 'English as a Foreign Language', 'Music', 'Art'], - resources: ['Bootstrap', 'CodeCademy', 'Google CS First', 'Khan Academy', 'Kodable', 'Lightbot', 'Scratch', 'Tynker'], - robotics: ['Grok Learning', 'Kodable', 'LEGO Education', 'Microbit', 'Ozobot', 'Sphero', 'Raspberry Pi', 'Wonder Workshop'], - schoolCountry: %w(Canada Chile Israel Malaysia Mexico Thailand), - workshopOrganizer: get_international_optin_partners, - workshopFacilitator: get_international_optin_facilitators, - workshopCourse: ['CS Fundamentals (Courses A-F)', 'CS Fundamentals (Pre-Express or Express)'] - } - ) + entry_keys = { + gender: %w(male female non_binary not_listed none), + schoolCountry: %w(canada chile israel malaysia mexico thailand), + ages: %w(ages_under_6 ages_7_8 ages_9_10 ages_11_12 ages_13_14 ages_15_16 ages_17_18 ages_19_over), + subjects: %w(cs ict math science history la efl music art), + resources: %w(bootstrap codecademy csfirst khan kodable lightbot scratch tynker), + robotics: %w(grok kodable lego microbit ozobot sphero raspberry wonder), + workshopCourse: %w(csf_af csf_express), + optIn: %w(opt_in_yes opt_in_no) + } + + entries = Hash[entry_keys.map {|k, v| [k, v.map {|s| I18n.t("pd.form_entries.#{k.to_s.underscore}.#{s.underscore}")}]}] + + entries[:workshopOrganizer] = get_international_optin_partners + entries[:workshopFacilitator] = get_international_optin_facilitators + + super.merge(entries) end def self.labels diff --git a/dashboard/config/locales/en.yml b/dashboard/config/locales/en.yml index 374b85e8128b0..ccda2f5ecc33e 100644 --- a/dashboard/config/locales/en.yml +++ b/dashboard/config/locales/en.yml @@ -1039,6 +1039,63 @@ en: workshop_facilitator: 'Workshop Facilitator' workshop_course: 'Workshop Course' opt_in: 'I want to share my data with Code.org, International Partner' + form_entries: + gender: + male: 'Male' + female: 'Female' + non_binary: 'Non-binary' + not_listed: 'Preferred term not listed' + none: 'Prefer not to answer' + school_country: + canada: 'Canada' + chile: 'Chile' + israel: 'Israel' + malaysia: 'Malaysia' + mexico: 'Mexico' + thailand: 'Thailand' + ages: + ages_under_6: '< 6 years old' + ages_7_8: '7-8 years old' + ages_9_10: '9-10 years old' + ages_11_12: '11-12 years old' + ages_13_14: '13-14 years old' + ages_15_16: '15-16 years old' + ages_17_18: '17-18 years old' + ages_19_over: '19+ years old' + subjects: + cs: 'Computer Science' + ict: 'ICT' + math: 'Math' + science: 'Science' + history: 'History / Social Studies' + la: 'Language Arts' + efl: 'English as a Foreign Language' + music: 'Music' + art: 'Art' + resources: + bootstrap: 'Bootstrap' + codecademy: 'CodeCademy' + csfirst: 'Google CS First' + khan: 'Khan Academy' + kodable: 'Kodable' + lightbot: 'Lightbot' + scratch: 'Scratch' + tynker: 'Tynker' + robotics: + grok: 'Grok Learning' + kodable: 'Kodable' + lego: 'LEGO Education' + microbit: 'Microbit' + ozobot: 'Ozobot' + sphero: 'Sphero' + raspberry: 'Raspberry Pi' + wonder: 'Wonder Workshop' + workshop_course: + csf_af: 'CS Fundamentals (Courses A-F)' + csf_express: 'CS Fundamentals (Pre-Express or Express)' + opt_in: + opt_in_yes: 'Yes' + opt_in_no: 'No' courses_category: 'Full Courses' cookie_banner: message: 'By continuing to browse our site or clicking "I agree," you agree to the storing of cookies on your computer or device.' From caaa78ef966ea0f411e15de1830d59d80f71d936 Mon Sep 17 00:00:00 2001 From: Brendan Reville Date: Mon, 25 Jun 2018 23:01:13 +1000 Subject: [PATCH 05/89] PD: international opt-in, some validation & testing --- .../InternationalOptin.jsx | 11 +++-- .../app/models/pd/international_optin.rb | 23 ++++++++- .../international_optins_controller_test.rb | 49 +++++++++++++++++++ dashboard/test/factories/pd_factories.rb | 5 ++ .../test/factories/pd_international_optins.rb | 19 ------- .../models/pd/international_optin_test.rb | 37 ++++++++++++-- 6 files changed, 117 insertions(+), 27 deletions(-) create mode 100644 dashboard/test/controllers/api/v1/pd/international_optins_controller_test.rb delete mode 100644 dashboard/test/factories/pd_international_optins.rb diff --git a/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx b/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx index e8109d5f4c4eb..bafe004d9f19b 100644 --- a/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx +++ b/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx @@ -88,7 +88,8 @@ class InternationalOptinComponent extends FormComponent { label: labels.email, type: 'text', value: this.props.accountEmail, - readOnly: true + readOnly: true, + required: true }) } @@ -200,5 +201,9 @@ class InternationalOptinComponent extends FormComponent { } } -InternationalOptinComponent.associatedFields = - ['firstName', 'lastName', 'title', 'email', 'optIn']; +InternationalOptinComponent.associatedFields = [ + 'firstName', 'firstNamePreferred', 'lastName', 'email', 'emailAlternate', + 'gender', 'schoolName', 'schoolCity', 'schoolCountry', 'ages', 'subjects', + 'resources', 'robotics', 'workshopOrganizer', 'workshopFacilitator', + 'workshopCourse', 'optIn' +]; diff --git a/dashboard/app/models/pd/international_optin.rb b/dashboard/app/models/pd/international_optin.rb index 1b03a4f441912..e02ea38714cab 100644 --- a/dashboard/app/models/pd/international_optin.rb +++ b/dashboard/app/models/pd/international_optin.rb @@ -18,11 +18,24 @@ class Pd::InternationalOptin < ApplicationRecord include Pd::Form + belongs_to :user + + validate :validate_email + def self.required_fields [ :first_name, :last_name, :email, + :gender, + :school_city, + :school_country, + :ages, + :subjects, + :resources, + :workshop_organizer, + :workshop_facilitator, + :workshop_course, :opt_in ] end @@ -49,8 +62,6 @@ def self.options def self.labels keys = %w( - firstName - preferredFirstName firstName firstNamePreferred lastName @@ -79,4 +90,12 @@ def email def opt_in? sanitize_form_data_hash[:opt_in].downcase == "yes" end + + private + + def validate_email + hash = sanitize_form_data_hash + + add_key_error(:email) unless Cdo::EmailValidator.email_address?(hash[:email]) + end end diff --git a/dashboard/test/controllers/api/v1/pd/international_optins_controller_test.rb b/dashboard/test/controllers/api/v1/pd/international_optins_controller_test.rb new file mode 100644 index 0000000000000..19c3dbb8615e6 --- /dev/null +++ b/dashboard/test/controllers/api/v1/pd/international_optins_controller_test.rb @@ -0,0 +1,49 @@ +require 'test_helper' + +class Api::V1::Pd::RegionalPartnerContactsControllerTest < ::ActionController::TestCase + SAMPLE_FORM_DATA = { + firstName: 'First', + firstNamePreferred: 'Preferred', + lastName: 'Last', + email: 'foo@bar.com', + emailAlternate: 'footoo@bar.com', + gender: 'Prefer not to answer', + schoolName: 'School Name', + schoolCity: 'School City', + schoolCountry: 'School Country', + ages: ['19+ years old'], + subjects: ['ICT'], + resources: ['Kodable'], + robotics: ['LEGO Education'], + workshopOrganizer: 'Workshop Organizer', + workshopFacilitator: 'Workshop Facilitator', + workshopCourse: 'Workshop Course', + optIn: 'Yes' + } + + test 'create creates a new international optin' do + assert_creates Pd::InternationalOptin do + put :create, params: { + form_data: SAMPLE_FORM_DATA + } + end + + assert_response :created + end + + test 'create returns appropriate errors if international optin data is missing' do + new_form = SAMPLE_FORM_DATA.dup + new_form.delete :lastName + + assert_does_not_create Pd::InternationalOptin do + put :create, params: { + form_data: new_form + } + end + + assert_response :bad_request + response_body = JSON.parse(@response.body) + + assert_equal 'Please fill out the fields about your school above', response_body['general_error'] + end +end diff --git a/dashboard/test/factories/pd_factories.rb b/dashboard/test/factories/pd_factories.rb index b687c6a0e82ec..12ac2bc8a38c9 100644 --- a/dashboard/test/factories/pd_factories.rb +++ b/dashboard/test/factories/pd_factories.rb @@ -543,6 +543,11 @@ form_data nil end + factory :pd_international_optin, class: 'Pd::InternationalOptin' do + user nil + form_data nil + end + factory :pd_regional_partner_cohort, class: 'Pd::RegionalPartnerCohort' do course Pd::Workshop::COURSE_CSP end diff --git a/dashboard/test/factories/pd_international_optins.rb b/dashboard/test/factories/pd_international_optins.rb deleted file mode 100644 index acdd16330dbb0..0000000000000 --- a/dashboard/test/factories/pd_international_optins.rb +++ /dev/null @@ -1,19 +0,0 @@ -# == Schema Information -# -# Table name: pd_international_optins -# -# id :integer not null, primary key -# user_id :integer -# form_data :text(65535) -# created_at :datetime not null -# updated_at :datetime not null -# -# Indexes -# -# index_pd_international_optins_on_user_id (user_id) -# - -FactoryGirl.define do - factory :pd_international_optin, class: 'Pd::InternationalOptin' do - end -end diff --git a/dashboard/test/models/pd/international_optin_test.rb b/dashboard/test/models/pd/international_optin_test.rb index 91e92117e5304..211ff84941c90 100644 --- a/dashboard/test/models/pd/international_optin_test.rb +++ b/dashboard/test/models/pd/international_optin_test.rb @@ -1,7 +1,38 @@ require 'test_helper' class Pd::InternationalOptinTest < ActiveSupport::TestCase - # test "the truth" do - # assert true - # end + FORM_DATA = { + firstName: 'First', + firstNamePreferred: 'Preferred', + lastName: 'Last', + email: 'foo@bar.com', + emailAlternate: 'footoo@bar.com', + gender: 'Prefer not to answer', + schoolName: 'School Name', + schoolCity: 'School City', + schoolCountry: 'School Country', + ages: ['19+ years old'], + subjects: ['ICT'], + resources: ['Kodable'], + robotics: ['LEGO Education'], + workshopOrganizer: 'Workshop Organizer', + workshopFacilitator: 'Workshop Facilitator', + workshopCourse: 'Workshop Course', + optIn: 'Yes' + } + + test 'Test international optin validation' do + optin = build :pd_international_optin, form_data: {}.to_json + refute optin.valid? + + assert build(:pd_international_optin, form_data: FORM_DATA.to_json).valid? + + refute build( + :pd_international_optin, form_data: FORM_DATA.merge( + { + ages: nil, + } + ).to_json + ).valid? + end end From ce89358619ba7c80273a685c5fe6891f5bacbb1d Mon Sep 17 00:00:00 2001 From: Brendan Reville Date: Tue, 26 Jun 2018 16:42:04 +1000 Subject: [PATCH 06/89] PD: international opt-in gets date picker --- .../InternationalOptin.jsx | 57 ++++++++++++++++--- .../v1/pd/international_optins_controller.rb | 16 +++--- .../pd/international_optin_controller.rb | 5 -- .../app/models/pd/international_optin.rb | 15 ----- lib/international_optin_people.rb | 2 +- 5 files changed, 58 insertions(+), 37 deletions(-) diff --git a/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx b/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx index bafe004d9f19b..e77207ca0be69 100644 --- a/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx +++ b/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx @@ -1,7 +1,15 @@ import React, {PropTypes} from 'react'; import FormController from '../form_components/FormController'; import FormComponent from '../form_components/FormComponent'; -import {FormGroup} from 'react-bootstrap'; +import DatePicker from '../workshop_dashboard/components/date_picker'; +import moment from 'moment'; +import {DATE_FORMAT} from '../workshop_dashboard/workshopConstants'; +import { + Row, + Col, + ControlLabel, + FormGroup +} from 'react-bootstrap'; export default class InternationalOptin extends FormController { static propTypes = { @@ -47,13 +55,23 @@ export default class InternationalOptin extends FormController { } } + class InternationalOptinComponent extends FormComponent { static propTypes = { accountEmail: PropTypes.string.isRequired }; + handleDateChange = (date) => { + // Don't allow null. If the date is cleared, default again to today. + date = date || moment(); + super.handleChange({date: date.format(DATE_FORMAT)}); + }; + render() { const labels = this.props.labels; + const date = (this.props.data && this.props.data.date) ? + moment(this.props.data.date, DATE_FORMAT) : moment(); + // todo: add validation return ( @@ -81,18 +99,15 @@ class InternationalOptinComponent extends FormComponent { required: true }) } - { this.buildFieldGroup({ name: 'email', label: labels.email, type: 'text', value: this.props.accountEmail, - readOnly: true, - required: true + readOnly: true }) } - { this.buildFieldGroup({ name: 'emailAlternate', @@ -167,6 +182,30 @@ class InternationalOptinComponent extends FormComponent { includeOther: true }) } + + + + + Date + * + + + + + + + + + { this.buildSelectFieldGroupFromOptions({ name: 'workshopOrganizer', @@ -202,8 +241,8 @@ class InternationalOptinComponent extends FormComponent { } InternationalOptinComponent.associatedFields = [ - 'firstName', 'firstNamePreferred', 'lastName', 'email', 'emailAlternate', - 'gender', 'schoolName', 'schoolCity', 'schoolCountry', 'ages', 'subjects', - 'resources', 'robotics', 'workshopOrganizer', 'workshopFacilitator', - 'workshopCourse', 'optIn' + 'firstName', 'firstNamePreferred', 'lastName', 'emailAlternate', 'gender', + 'schoolName', 'schoolCity', 'schoolCountry', 'ages', 'subjects', 'resources', + 'robotics', 'workshopOrganizer', 'workshopFacilitator', 'workshopCourse', + 'optIn' ]; diff --git a/dashboard/app/controllers/api/v1/pd/international_optins_controller.rb b/dashboard/app/controllers/api/v1/pd/international_optins_controller.rb index f1898c1fdefa9..fa48197433c29 100644 --- a/dashboard/app/controllers/api/v1/pd/international_optins_controller.rb +++ b/dashboard/app/controllers/api/v1/pd/international_optins_controller.rb @@ -4,12 +4,14 @@ def new_form end def on_successful_create - EmailPreference.upsert!( - email: @contact_form.email, - opt_in: @contact_form.opt_in?, - ip_address: request.ip, - source: EmailPreference::FORM_PD_INTERNATIONAL_OPTIN, - form_kind: "0" - ) + if current_user.email + EmailPreference.upsert!( + email: current_user.email, + opt_in: @contact_form.opt_in?, + ip_address: request.ip, + source: EmailPreference::FORM_PD_INTERNATIONAL_OPTIN, + form_kind: "0" + ) + end end end diff --git a/dashboard/app/controllers/pd/international_optin_controller.rb b/dashboard/app/controllers/pd/international_optin_controller.rb index 7efa36ba3de79..a714187699cf6 100644 --- a/dashboard/app/controllers/pd/international_optin_controller.rb +++ b/dashboard/app/controllers/pd/international_optin_controller.rb @@ -15,9 +15,4 @@ def new }.to_json } end - - # Get /pd/international_optins/:contact_id/thanks - def thanks - @regional_partner = @international_optin.try(:regional_partner) - end end diff --git a/dashboard/app/models/pd/international_optin.rb b/dashboard/app/models/pd/international_optin.rb index e02ea38714cab..fb88b44687803 100644 --- a/dashboard/app/models/pd/international_optin.rb +++ b/dashboard/app/models/pd/international_optin.rb @@ -20,13 +20,10 @@ class Pd::InternationalOptin < ApplicationRecord belongs_to :user - validate :validate_email - def self.required_fields [ :first_name, :last_name, - :email, :gender, :school_city, :school_country, @@ -83,19 +80,7 @@ def self.labels Hash[keys.collect {|v| [v, I18n.t("pd.form_labels.#{v.underscore}")]}] end - def email - sanitize_form_data_hash[:email] - end - def opt_in? sanitize_form_data_hash[:opt_in].downcase == "yes" end - - private - - def validate_email - hash = sanitize_form_data_hash - - add_key_error(:email) unless Cdo::EmailValidator.email_address?(hash[:email]) - end end diff --git a/lib/international_optin_people.rb b/lib/international_optin_people.rb index 27713f7457533..09e402d5295e3 100644 --- a/lib/international_optin_people.rb +++ b/lib/international_optin_people.rb @@ -1,7 +1,7 @@ INTERNATIONAL_OPTIN_FACILITATORS = [ "First facilitator", "Second facilitator", - "Third faciliattor" + "Third facilitator" ].freeze INTERNATIONAL_OPTIN_PARTNERS = [ From a93d3ec9689684f65d315021473c7425bbbf005d Mon Sep 17 00:00:00 2001 From: Brendan Reville Date: Thu, 28 Jun 2018 16:07:31 +1000 Subject: [PATCH 07/89] PD: some international opt-in code review feedback --- apps/i18n/common/en_us.json | 1 + .../InternationalOptin.jsx | 14 +++--- .../v1/pd/international_optins_controller.rb | 22 ++++----- .../pd/international_optin_controller.rb | 1 + dashboard/app/models/ability.rb | 2 + .../app/models/pd/international_optin.rb | 15 +++++-- .../teacher_application/no_teacher_email.haml | 8 ++++ ...20110434_create_pd_international_optins.rb | 4 +- dashboard/db/schema.rb | 4 +- .../international_optins_controller_test.rb | 45 +++++++++++-------- .../models/pd/international_optin_test.rb | 15 +++---- lib/cdo/delete_accounts_helper.rb | 1 + lib/international_optin_people.rb | 28 +++++------- 13 files changed, 94 insertions(+), 66 deletions(-) create mode 100644 dashboard/app/views/pd/application/teacher_application/no_teacher_email.haml diff --git a/apps/i18n/common/en_us.json b/apps/i18n/common/en_us.json index 78d651351cca7..d73d14cd3ad1b 100644 --- a/apps/i18n/common/en_us.json +++ b/apps/i18n/common/en_us.json @@ -999,6 +999,7 @@ "secret": "Secret", "seeAllTutorials": "See all tutorials", "selectACourse": "Select a course or unit", + "selectAnOption": "Please select an option...", "selectAssessment": "Select an assessment or survey", "selectGoogleClassroom": "Select a Google Classroom", "selectCleverSection": "Select a Clever section", diff --git a/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx b/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx index e77207ca0be69..be4908e538e85 100644 --- a/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx +++ b/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx @@ -10,6 +10,7 @@ import { ControlLabel, FormGroup } from 'react-bootstrap'; +import i18n from '@cdo/locale'; export default class InternationalOptin extends FormController { static propTypes = { @@ -29,7 +30,7 @@ export default class InternationalOptin extends FormController { */ serializeFormData() { const formData = super.serializeFormData(); - //Object.assign(formData['form_data'], this.getDistrictData()); + formData.form_data.email = this.props.accountEmail; return formData; } @@ -217,14 +218,16 @@ class InternationalOptinComponent extends FormComponent { this.buildSelectFieldGroupFromOptions({ name: 'workshopFacilitator', label: labels.workshopFacilitator, - required: true + required: true, + placeholder: i18n.selectAnOption() }) } { this.buildSelectFieldGroupFromOptions({ name: 'workshopCourse', label: labels.workshopCourse, - required: true + required: true, + placeholder: i18n.selectAnOption() }) } { @@ -232,7 +235,8 @@ class InternationalOptinComponent extends FormComponent { name: 'optIn', label: labels.optIn, type: 'radio', - required: true + required: true, + placeholder: i18n.selectAnOption() }) } @@ -241,7 +245,7 @@ class InternationalOptinComponent extends FormComponent { } InternationalOptinComponent.associatedFields = [ - 'firstName', 'firstNamePreferred', 'lastName', 'emailAlternate', 'gender', + 'firstName', 'firstNamePreferred', 'lastName', 'email', 'emailAlternate', 'gender', 'schoolName', 'schoolCity', 'schoolCountry', 'ages', 'subjects', 'resources', 'robotics', 'workshopOrganizer', 'workshopFacilitator', 'workshopCourse', 'optIn' diff --git a/dashboard/app/controllers/api/v1/pd/international_optins_controller.rb b/dashboard/app/controllers/api/v1/pd/international_optins_controller.rb index fa48197433c29..a3fc698e28f37 100644 --- a/dashboard/app/controllers/api/v1/pd/international_optins_controller.rb +++ b/dashboard/app/controllers/api/v1/pd/international_optins_controller.rb @@ -1,17 +1,19 @@ class Api::V1::Pd::InternationalOptinsController < Api::V1::Pd::FormsController + authorize_resource class: 'Pd::InternationalOptin', only: :create + def new_form - @contact_form = ::Pd::InternationalOptin.new + @contact_form = ::Pd::InternationalOptin.new( + user: current_user + ) end def on_successful_create - if current_user.email - EmailPreference.upsert!( - email: current_user.email, - opt_in: @contact_form.opt_in?, - ip_address: request.ip, - source: EmailPreference::FORM_PD_INTERNATIONAL_OPTIN, - form_kind: "0" - ) - end + EmailPreference.upsert!( + email: @contact_form.email, + opt_in: @contact_form.opt_in?, + ip_address: request.ip, + source: EmailPreference::FORM_PD_INTERNATIONAL_OPTIN, + form_kind: "0" + ) end end diff --git a/dashboard/app/controllers/pd/international_optin_controller.rb b/dashboard/app/controllers/pd/international_optin_controller.rb index a714187699cf6..5accba7d41c5e 100644 --- a/dashboard/app/controllers/pd/international_optin_controller.rb +++ b/dashboard/app/controllers/pd/international_optin_controller.rb @@ -5,6 +5,7 @@ class Pd::InternationalOptinController < ApplicationController def new return render '/pd/application/teacher_application/logged_out' unless current_user return render '/pd/application/teacher_application/not_teacher' unless current_user.teacher? + return render '/pd/application/teacher_application/no_teacher_email' unless current_user.email @script_data = { props: { diff --git a/dashboard/app/models/ability.rb b/dashboard/app/models/ability.rb index 3c0a2d97ad5a1..0d0adda978971 100644 --- a/dashboard/app/models/ability.rb +++ b/dashboard/app/models/ability.rb @@ -48,6 +48,7 @@ def initialize(user) Pd::Application::ApplicationBase, Pd::Application::Facilitator1819Application, Pd::Application::Teacher1819Application, + Pd::InternationalOptin, :maker_discount ] @@ -92,6 +93,7 @@ def initialize(user) can [:new, :create, :read], Pd::WorkshopMaterialOrder, user_id: user.id can [:new, :create, :read], Pd::Application::Facilitator1819Application, user_id: user.id can [:new, :create, :read], Pd::Application::Teacher1819Application, user_id: user.id + can :create, Pd::InternationalOptin, user_id: user.id can :manage, :maker_discount end diff --git a/dashboard/app/models/pd/international_optin.rb b/dashboard/app/models/pd/international_optin.rb index fb88b44687803..0d8d9c6c647e8 100644 --- a/dashboard/app/models/pd/international_optin.rb +++ b/dashboard/app/models/pd/international_optin.rb @@ -3,8 +3,8 @@ # Table name: pd_international_optins # # id :integer not null, primary key -# user_id :integer -# form_data :text(65535) +# user_id :integer not null +# form_data :text(65535) not null # created_at :datetime not null # updated_at :datetime not null # @@ -17,9 +17,12 @@ class Pd::InternationalOptin < ApplicationRecord include Pd::Form + include InternationalOptinPeople belongs_to :user + validates_presence_of :user_id, :form_data + def self.required_fields [ :first_name, @@ -51,8 +54,8 @@ def self.options entries = Hash[entry_keys.map {|k, v| [k, v.map {|s| I18n.t("pd.form_entries.#{k.to_s.underscore}.#{s.underscore}")}]}] - entries[:workshopOrganizer] = get_international_optin_partners - entries[:workshopFacilitator] = get_international_optin_facilitators + entries[:workshopOrganizer] = INTERNATIONAL_OPTIN_PARTNERS + entries[:workshopFacilitator] = INTERNATIONAL_OPTIN_FACILITATORS super.merge(entries) end @@ -83,4 +86,8 @@ def self.labels def opt_in? sanitize_form_data_hash[:opt_in].downcase == "yes" end + + def email + sanitize_form_data_hash[:email] + end end diff --git a/dashboard/app/views/pd/application/teacher_application/no_teacher_email.haml b/dashboard/app/views/pd/application/teacher_application/no_teacher_email.haml new file mode 100644 index 0000000000000..24a3773f686bc --- /dev/null +++ b/dashboard/app/views/pd/application/teacher_application/no_teacher_email.haml @@ -0,0 +1,8 @@ +- content_for(:head) do + = stylesheet_link_tag 'css/pd', media: 'all' + +%p + Your account doesn’t have an email address. Please + = link_to edit_user_registration_path do + visit your account settings + to add your email address before completing this form. diff --git a/dashboard/db/migrate/20180620110434_create_pd_international_optins.rb b/dashboard/db/migrate/20180620110434_create_pd_international_optins.rb index 24ebc8231822d..2fc4124b0af3c 100644 --- a/dashboard/db/migrate/20180620110434_create_pd_international_optins.rb +++ b/dashboard/db/migrate/20180620110434_create_pd_international_optins.rb @@ -1,8 +1,8 @@ class CreatePdInternationalOptins < ActiveRecord::Migration[5.0] def change create_table :pd_international_optins do |t| - t.references :user, index: true - t.text :form_data + t.references :user, index: true, null: false + t.text :form_data, null: false t.timestamps null: false end end diff --git a/dashboard/db/schema.rb b/dashboard/db/schema.rb index c6682ca4ebad1..a06b5cd2fe58a 100644 --- a/dashboard/db/schema.rb +++ b/dashboard/db/schema.rb @@ -673,8 +673,8 @@ end create_table "pd_international_optins", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci" do |t| - t.integer "user_id" - t.text "form_data", limit: 65535 + t.integer "user_id", null: false + t.text "form_data", limit: 65535, null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["user_id"], name: "index_pd_international_optins_on_user_id", using: :btree diff --git a/dashboard/test/controllers/api/v1/pd/international_optins_controller_test.rb b/dashboard/test/controllers/api/v1/pd/international_optins_controller_test.rb index 19c3dbb8615e6..906bae7c72790 100644 --- a/dashboard/test/controllers/api/v1/pd/international_optins_controller_test.rb +++ b/dashboard/test/controllers/api/v1/pd/international_optins_controller_test.rb @@ -1,49 +1,58 @@ require 'test_helper' -class Api::V1::Pd::RegionalPartnerContactsControllerTest < ::ActionController::TestCase +class Api::V1::Pd::InternationalOptinsControllerTest < ::ActionController::TestCase SAMPLE_FORM_DATA = { - firstName: 'First', - firstNamePreferred: 'Preferred', - lastName: 'Last', + first_name: 'First', + first_name_preferred: 'Preferred', + last_name: 'Last', email: 'foo@bar.com', - emailAlternate: 'footoo@bar.com', + email_alternate: 'footoo@bar.com', gender: 'Prefer not to answer', - schoolName: 'School Name', - schoolCity: 'School City', - schoolCountry: 'School Country', + school_name: 'School Name', + school_city: 'School City', + school_country: 'School Country', ages: ['19+ years old'], subjects: ['ICT'], resources: ['Kodable'], robotics: ['LEGO Education'], - workshopOrganizer: 'Workshop Organizer', - workshopFacilitator: 'Workshop Facilitator', - workshopCourse: 'Workshop Course', - optIn: 'Yes' + workshop_organizer: 'Workshop Organizer', + workshop_facilitator: 'Workshop Facilitator', + workshop_course: 'Workshop Course', + opt_in: 'Yes' } + self.use_transactional_test_case = true + setup_all do + @teacher = create :teacher + end + test 'create creates a new international optin' do + sign_in @teacher assert_creates Pd::InternationalOptin do put :create, params: { - form_data: SAMPLE_FORM_DATA + form_data: SAMPLE_FORM_DATA, + user: @teacher } + assert_response :created end assert_response :created end test 'create returns appropriate errors if international optin data is missing' do + sign_in @teacher + new_form = SAMPLE_FORM_DATA.dup - new_form.delete :lastName + new_form.delete :last_name assert_does_not_create Pd::InternationalOptin do put :create, params: { - form_data: new_form + form_data: new_form, + user: @teacher } + assert_response :bad_request end assert_response :bad_request - response_body = JSON.parse(@response.body) - - assert_equal 'Please fill out the fields about your school above', response_body['general_error'] end end diff --git a/dashboard/test/models/pd/international_optin_test.rb b/dashboard/test/models/pd/international_optin_test.rb index 211ff84941c90..297b0791090fb 100644 --- a/dashboard/test/models/pd/international_optin_test.rb +++ b/dashboard/test/models/pd/international_optin_test.rb @@ -22,17 +22,16 @@ class Pd::InternationalOptinTest < ActiveSupport::TestCase } test 'Test international optin validation' do - optin = build :pd_international_optin, form_data: {}.to_json - refute optin.valid? + teacher = create :teacher - assert build(:pd_international_optin, form_data: FORM_DATA.to_json).valid? + refute build(:pd_international_optin, form_data: {}.to_json, user_id: teacher.id).valid? refute build( - :pd_international_optin, form_data: FORM_DATA.merge( - { - ages: nil, - } - ).to_json + :pd_international_optin, form_data: FORM_DATA.merge({ages: nil}).to_json, user_id: teacher.id ).valid? + + assert build(:pd_international_optin, form_data: FORM_DATA.to_json, user_id: teacher.id).valid? + + refute build(:pd_international_optin, form_data: FORM_DATA.to_json).valid? end end diff --git a/lib/cdo/delete_accounts_helper.rb b/lib/cdo/delete_accounts_helper.rb index aa32290331551..db66181f8ea75 100644 --- a/lib/cdo/delete_accounts_helper.rb +++ b/lib/cdo/delete_accounts_helper.rb @@ -116,6 +116,7 @@ def clean_and_destroy_pd_content(user_id) Pd::FacilitatorProgramRegistration.where(user_id: user_id).each(&:clear_form_data) Pd::RegionalPartnerProgramRegistration.where(user_id: user_id).each(&:clear_form_data) Pd::WorkshopMaterialOrder.where(user_id: user_id).each(&:clear_data) + Pd::InternationalOptin.where(user_id: user_id).each(&:clear_form_data) pd_enrollment_id = Pd::Enrollment.where(user_id: user_id).pluck(:id).first if pd_enrollment_id diff --git a/lib/international_optin_people.rb b/lib/international_optin_people.rb index 09e402d5295e3..496601aa98425 100644 --- a/lib/international_optin_people.rb +++ b/lib/international_optin_people.rb @@ -1,19 +1,13 @@ -INTERNATIONAL_OPTIN_FACILITATORS = [ - "First facilitator", - "Second facilitator", - "Third facilitator" -].freeze +module InternationalOptinPeople + INTERNATIONAL_OPTIN_FACILITATORS = [ + "First facilitator", + "Second facilitator", + "Third facilitator" + ].freeze -INTERNATIONAL_OPTIN_PARTNERS = [ - "First partner", - "Second partner", - "Third partner" -].freeze - -def get_international_optin_facilitators - INTERNATIONAL_OPTIN_FACILITATORS -end - -def get_international_optin_partners - INTERNATIONAL_OPTIN_PARTNERS + INTERNATIONAL_OPTIN_PARTNERS = [ + "First partner", + "Second partner", + "Third partner" + ].freeze end From 5369d9971538bd8cb2e9a595ff12995909f788ea Mon Sep 17 00:00:00 2001 From: Brendan Reville Date: Fri, 29 Jun 2018 23:42:10 +1000 Subject: [PATCH 08/89] PD: international opt-in has better handing for "Other:" options Checking "Other:" for one of the three choices that supports it now satisfies the requirement (for two of them) that an option is checked. This is written to the server something like this: {"form_data":{"robotics":["Other:","Wonder Workshop"],"robotics_other":"My custom information"}} Some extra work is done in the textFieldMap setup to handle the "Other:" string being localized. --- .../InternationalOptin.jsx | 29 ++++++++++++++----- .../app/models/pd/international_optin.rb | 6 ++-- dashboard/config/locales/en.yml | 3 ++ 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx b/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx index be4908e538e85..861d342b9774c 100644 --- a/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx +++ b/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx @@ -31,7 +31,6 @@ export default class InternationalOptin extends FormController { serializeFormData() { const formData = super.serializeFormData(); formData.form_data.email = this.props.accountEmail; - return formData; } @@ -74,6 +73,18 @@ class InternationalOptinComponent extends FormComponent { moment(this.props.data.date, DATE_FORMAT) : moment(); // todo: add validation + let textFieldMapSubjects = {}; + const lastSubjectsKey = this.props.options.subjects.slice(-1)[0]; + textFieldMapSubjects[lastSubjectsKey] = "other"; + + let textFieldMapResources = {}; + const lastResourcesKey = this.props.options.resources.slice(-1)[0]; + textFieldMapResources[lastResourcesKey] = "other"; + + let textFieldMapRobotics = {}; + const lastRoboticsKey = this.props.options.robotics.slice(-1)[0]; + textFieldMapRobotics[lastRoboticsKey] = "other"; + return ( { @@ -157,32 +168,33 @@ class InternationalOptinComponent extends FormComponent { }) } { - this.buildButtonsFromOptions({ + this.buildButtonsWithAdditionalTextFieldsFromOptions({ name: 'subjects', label: labels.subjects, type: 'check', required: true, - includeOther: true + textFieldMap: textFieldMapSubjects }) } { - this.buildButtonsFromOptions({ + this.buildButtonsWithAdditionalTextFieldsFromOptions({ name: 'resources', label: labels.resources, type: 'check', required: false, - includeOther: true + textFieldMap: textFieldMapResources }) } { - this.buildButtonsFromOptions({ + this.buildButtonsWithAdditionalTextFieldsFromOptions({ name: 'robotics', label: labels.robotics, type: 'check', required: false, - includeOther: true + textFieldMap: textFieldMapRobotics }) } + Date: Fri, 29 Jun 2018 11:49:49 -0700 Subject: [PATCH 09/89] Make authenticationOptions and userType available in ManageLinkedAccounts --- apps/i18n/common/en_us.json | 1 + .../lib/ui/accounts/ManageLinkedAccounts.jsx | 34 ++++++++++++++++--- .../ManageLinkedAccountsController.js | 7 ++-- .../studio/pages/devise/registrations/edit.js | 9 +++-- .../views/devise/registrations/edit.html.haml | 3 +- 5 files changed, 43 insertions(+), 11 deletions(-) diff --git a/apps/i18n/common/en_us.json b/apps/i18n/common/en_us.json index da5cdeb9f0cc1..0b410e727ea70 100644 --- a/apps/i18n/common/en_us.json +++ b/apps/i18n/common/en_us.json @@ -491,6 +491,7 @@ "enableMakerDialogDescription": "Maker Toolkit is a feature used in the Computer Science Discoveries curriculum. See the setup page for more details:", "enableMakerDialogSetupPageLinkText": "Maker Toolkit Setup", "enablePairProgramming": "Enable Pair Programming", + "encrypted": "encrypted", "end": "end", "englishOnlyWarning": "Sorry! This stage is not available in your language. The puzzles in this stage use a mix of English words and characters that can’t be translated right now. You can move on to Stage {nextStage}.", "enterSectionCode": "Enter section code", diff --git a/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx b/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx index 99b1bdc3b8a72..89dcb8927cecf 100644 --- a/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx +++ b/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx @@ -10,10 +10,34 @@ const OAUTH_PROVIDERS = { GOOGLE: 'google_oauth2', FACEBOOK: 'facebook', CLEVER: 'clever', - MICROSOFT: 'microsoft', + MICROSOFT: 'windowslive', }; +const ENCRYPTED = `*** ${i18n.encrypted()} ***`; export default class ManageLinkedAccounts extends React.Component { + static propTypes = { + userType: PropTypes.string.isRequired, + authenticationOptions: PropTypes.arrayOf(PropTypes.shape({ + credential_type: PropTypes.string.isRequired, + email: PropTypes.string, + hashed_email: PropTypes.string, + })), + }; + + getEmailForProvider = (provider) => { + const {authenticationOptions, userType} = this.props; + const match = authenticationOptions.find((option) => { + return option.credential_type === provider; + }); + + if (match) { + if (userType === 'student') { + return ENCRYPTED; + } + return match.email; + } + }; + render() { return (
@@ -31,25 +55,25 @@ export default class ManageLinkedAccounts extends React.Component { {}} /> {}} /> {}} /> {}} cannotDisconnect /> diff --git a/apps/src/lib/ui/accounts/ManageLinkedAccountsController.js b/apps/src/lib/ui/accounts/ManageLinkedAccountsController.js index 39ac83f0c0ed0..4a616686e9878 100644 --- a/apps/src/lib/ui/accounts/ManageLinkedAccountsController.js +++ b/apps/src/lib/ui/accounts/ManageLinkedAccountsController.js @@ -3,9 +3,12 @@ import ReactDOM from 'react-dom'; import ManageLinkedAccounts from './ManageLinkedAccounts'; export default class ManageLinkedAccountsController { - constructor(mountPoint) { + constructor(mountPoint, userType, authenticationOptions) { ReactDOM.render( - , + , mountPoint ); } diff --git a/apps/src/sites/studio/pages/devise/registrations/edit.js b/apps/src/sites/studio/pages/devise/registrations/edit.js index a785340edaeaa..5afcd4e7643d8 100644 --- a/apps/src/sites/studio/pages/devise/registrations/edit.js +++ b/apps/src/sites/studio/pages/devise/registrations/edit.js @@ -8,7 +8,7 @@ import getScriptData from '@cdo/apps/util/getScriptData'; // Values loaded from scriptData are always initial values, not the latest // (possibly unsaved) user-edited values on the form. const scriptData = getScriptData('edit'); -const {userAge, userType, isPasswordRequired} = scriptData; +const {userAge, userType, isPasswordRequired, authenticationOptions} = scriptData; $(document).ready(() => { new ChangeEmailController({ @@ -31,10 +31,13 @@ $(document).ready(() => { new AddPasswordController($('#add-password-form'), addPasswordMountPoint); } - const manageLinkedAccountsMountPoint = document.getElementById('manage-linked-accounts'); if (manageLinkedAccountsMountPoint) { - new ManageLinkedAccountsController(manageLinkedAccountsMountPoint); + new ManageLinkedAccountsController( + manageLinkedAccountsMountPoint, + userType, + authenticationOptions, + ); } initializeCreatePersonalAccountControls(); diff --git a/dashboard/app/views/devise/registrations/edit.html.haml b/dashboard/app/views/devise/registrations/edit.html.haml index 4871b18d6ed56..163ff39a3059d 100644 --- a/dashboard/app/views/devise/registrations/edit.html.haml +++ b/dashboard/app/views/devise/registrations/edit.html.haml @@ -209,7 +209,8 @@ userAge: current_user.age, userType: current_user.user_type, isOauth: current_user.oauth?, - isPasswordRequired: current_user.encrypted_password.present? + isPasswordRequired: current_user.encrypted_password.present?, + authenticationOptions: current_user.authentication_options, }.to_json } %script{src: minifiable_asset_path('js/devise/registrations/edit.js'), data: script_data} From 0c106573b132f8e1a55a393ef49b9ea6e47228f1 Mon Sep 17 00:00:00 2001 From: Maddie Kasula Date: Fri, 29 Jun 2018 13:38:20 -0700 Subject: [PATCH 10/89] Add connectToGoogle to test /users/auth/google_oauth2/connect --- apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx b/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx index 89dcb8927cecf..e7cd99b91d78c 100644 --- a/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx +++ b/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx @@ -1,5 +1,6 @@ import React, {PropTypes} from 'react'; import ReactTooltip from 'react-tooltip'; +import $ from 'jquery'; import _ from 'lodash'; import i18n from '@cdo/locale'; import color from '@cdo/apps/util/color'; @@ -38,6 +39,17 @@ export default class ManageLinkedAccounts extends React.Component { } }; + connectToGoogle = () => { + $.ajax({ + url: '/users/auth/google_oauth2/connect', + method: 'GET' + }).done(result => { + console.log(result); + }).fail((jqXhr, status) => { + console.log(jqXhr, status); + }); + }; + render() { return (
@@ -56,7 +68,7 @@ export default class ManageLinkedAccounts extends React.Component { type={OAUTH_PROVIDERS.GOOGLE} displayName={i18n.manageLinkedAccounts_google_oauth2()} email={this.getEmailForProvider(OAUTH_PROVIDERS.GOOGLE)} - onClick={() => {}} + onClick={this.connectToGoogle} /> Date: Fri, 29 Jun 2018 15:48:08 -0700 Subject: [PATCH 11/89] Navigate to /connect instead of AJAX request --- .../lib/ui/accounts/ManageLinkedAccounts.jsx | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx b/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx index e7cd99b91d78c..91906403f22aa 100644 --- a/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx +++ b/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx @@ -1,8 +1,8 @@ import React, {PropTypes} from 'react'; import ReactTooltip from 'react-tooltip'; -import $ from 'jquery'; import _ from 'lodash'; import i18n from '@cdo/locale'; +import {navigateToHref} from '@cdo/apps/utils'; import color from '@cdo/apps/util/color'; import {tableLayoutStyles} from "@cdo/apps/templates/tables/tableConstants"; import BootstrapButton from './BootstrapButton'; @@ -25,29 +25,29 @@ export default class ManageLinkedAccounts extends React.Component { })), }; - getEmailForProvider = (provider) => { - const {authenticationOptions, userType} = this.props; - const match = authenticationOptions.find((option) => { + getProviderConnection = (provider) => { + return this.props.authenticationOptions.find((option) => { return option.credential_type === provider; }); + }; + + getEmailForProvider = (provider) => { + const match = this.getProviderConnection(provider); if (match) { - if (userType === 'student') { + if (this.props.userType === 'student') { return ENCRYPTED; } return match.email; } }; - connectToGoogle = () => { - $.ajax({ - url: '/users/auth/google_oauth2/connect', - method: 'GET' - }).done(result => { - console.log(result); - }).fail((jqXhr, status) => { - console.log(jqXhr, status); - }); + toggleProviderConnection = (provider) => { + if (this.getProviderConnection(provider)) { + // TODO: (madelynkasula) disconnect from provider + } else { + navigateToHref(`/users/auth/${provider}/connect`); + } }; render() { @@ -68,7 +68,7 @@ export default class ManageLinkedAccounts extends React.Component { type={OAUTH_PROVIDERS.GOOGLE} displayName={i18n.manageLinkedAccounts_google_oauth2()} email={this.getEmailForProvider(OAUTH_PROVIDERS.GOOGLE)} - onClick={this.connectToGoogle} + onClick={() => this.toggleProviderConnection(OAUTH_PROVIDERS.GOOGLE)} /> Date: Mon, 2 Jul 2018 12:05:42 +1000 Subject: [PATCH 12/89] PD: international opt-in naming: Optin->OptIn, optin -> opt_in Also loc'ed the three standalone HAML pages. --- apps/Gruntfile.js | 2 +- .../InternationalOptIn.jsx} | 10 +++++----- .../new.js | 4 ++-- ...ller.rb => international_opt_ins_controller.rb} | 8 ++++---- ...oller.rb => international_opt_in_controller.rb} | 12 ++++++------ dashboard/app/models/ability.rb | 4 ++-- ...ernational_optin.rb => international_opt_in.rb} | 14 +++++++------- .../teacher_application/logged_out.haml | 11 ++++------- .../teacher_application/no_teacher_email.haml | 5 +---- .../teacher_application/not_teacher.haml | 12 +++--------- .../new.html.haml | 2 +- .../thanks.html.haml | 0 dashboard/config/locales/en.yml | 9 +++++++++ dashboard/config/routes.rb | 6 +++--- ...80620110434_create_pd_international_opt_ins.rb} | 4 ++-- dashboard/db/schema.rb | 4 ++-- ...rb => international_opt_ins_controller_test.rb} | 10 +++++----- dashboard/test/factories/pd_factories.rb | 2 +- ..._optin_test.rb => international_opt_in_test.rb} | 12 ++++++------ lib/cdo/delete_accounts_helper.rb | 2 +- lib/cdo/email_preference_constants.rb | 2 +- ...in_people.rb => international_opt_in_people.rb} | 6 +++--- 22 files changed, 69 insertions(+), 72 deletions(-) rename apps/src/code-studio/pd/{international_optin/InternationalOptin.jsx => international_opt_in/InternationalOptIn.jsx} (96%) rename apps/src/sites/studio/pages/pd/{international_optin => international_opt_in}/new.js (71%) rename dashboard/app/controllers/api/v1/pd/{international_optins_controller.rb => international_opt_ins_controller.rb} (52%) rename dashboard/app/controllers/pd/{international_optin_controller.rb => international_opt_in_controller.rb} (51%) rename dashboard/app/models/pd/{international_optin.rb => international_opt_in.rb} (84%) rename dashboard/app/views/pd/{international_optin => international_opt_in}/new.html.haml (71%) rename dashboard/app/views/pd/{international_optin => international_opt_in}/thanks.html.haml (100%) rename dashboard/db/migrate/{20180620110434_create_pd_international_optins.rb => 20180620110434_create_pd_international_opt_ins.rb} (56%) rename dashboard/test/controllers/api/v1/pd/{international_optins_controller_test.rb => international_opt_ins_controller_test.rb} (79%) rename dashboard/test/models/pd/{international_optin_test.rb => international_opt_in_test.rb} (57%) rename lib/{international_optin_people.rb => international_opt_in_people.rb} (60%) diff --git a/apps/Gruntfile.js b/apps/Gruntfile.js index de1891740185b..c954ba7b51027 100644 --- a/apps/Gruntfile.js +++ b/apps/Gruntfile.js @@ -513,7 +513,7 @@ describe('entry tests', () => { 'pd/professional_learning_landing/index': './src/sites/studio/pages/pd/professional_learning_landing/index.js', 'pd/regional_partner_contact/new': './src/sites/studio/pages/pd/regional_partner_contact/new.js', - 'pd/international_optin/new': './src/sites/studio/pages/pd/international_optin/new.js', + 'pd/international_opt_in/new': './src/sites/studio/pages/pd/international_opt_in/new.js', 'peer_reviews/dashboard': './src/sites/studio/pages/peer_reviews/dashboard.js', diff --git a/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx b/apps/src/code-studio/pd/international_opt_in/InternationalOptIn.jsx similarity index 96% rename from apps/src/code-studio/pd/international_optin/InternationalOptin.jsx rename to apps/src/code-studio/pd/international_opt_in/InternationalOptIn.jsx index 861d342b9774c..459b1cbb63b53 100644 --- a/apps/src/code-studio/pd/international_optin/InternationalOptin.jsx +++ b/apps/src/code-studio/pd/international_opt_in/InternationalOptIn.jsx @@ -12,7 +12,7 @@ import { } from 'react-bootstrap'; import i18n from '@cdo/locale'; -export default class InternationalOptin extends FormController { +export default class InternationalOptIn extends FormController { static propTypes = { accountEmail: PropTypes.string.isRequired, labels: PropTypes.object.isRequired @@ -22,7 +22,7 @@ export default class InternationalOptin extends FormController { * @override */ onSuccessfulSubmit(data) { - window.location = `/pd/international_optin/${data.id}/thanks`; + window.location = `/pd/international_opt_in/${data.id}/thanks`; } /** @@ -39,7 +39,7 @@ export default class InternationalOptin extends FormController { */ getPageComponents() { return [ - InternationalOptinComponent + InternationalOptInComponent ]; } @@ -56,7 +56,7 @@ export default class InternationalOptin extends FormController { } -class InternationalOptinComponent extends FormComponent { +class InternationalOptInComponent extends FormComponent { static propTypes = { accountEmail: PropTypes.string.isRequired }; @@ -257,7 +257,7 @@ class InternationalOptinComponent extends FormComponent { } } -InternationalOptinComponent.associatedFields = [ +InternationalOptInComponent.associatedFields = [ 'firstName', 'firstNamePreferred', 'lastName', 'email', 'emailAlternate', 'gender', 'schoolName', 'schoolCity', 'schoolCountry', 'ages', 'subjects', 'resources', 'robotics', 'workshopOrganizer', 'workshopFacilitator', 'workshopCourse', diff --git a/apps/src/sites/studio/pages/pd/international_optin/new.js b/apps/src/sites/studio/pages/pd/international_opt_in/new.js similarity index 71% rename from apps/src/sites/studio/pages/pd/international_optin/new.js rename to apps/src/sites/studio/pages/pd/international_opt_in/new.js index b642c1bc7a040..a43bcd2d8b30e 100644 --- a/apps/src/sites/studio/pages/pd/international_optin/new.js +++ b/apps/src/sites/studio/pages/pd/international_opt_in/new.js @@ -1,11 +1,11 @@ import React from 'react'; import ReactDOM from 'react-dom'; -import InternationalOptin from '@cdo/apps/code-studio/pd/international_optin/InternationalOptin'; +import InternationalOptIn from '@cdo/apps/code-studio/pd/international_opt_in/InternationalOptIn'; import getScriptData from '@cdo/apps/util/getScriptData'; document.addEventListener("DOMContentLoaded", function (event) { ReactDOM.render( - , document.getElementById('application-container') ); diff --git a/dashboard/app/controllers/api/v1/pd/international_optins_controller.rb b/dashboard/app/controllers/api/v1/pd/international_opt_ins_controller.rb similarity index 52% rename from dashboard/app/controllers/api/v1/pd/international_optins_controller.rb rename to dashboard/app/controllers/api/v1/pd/international_opt_ins_controller.rb index a3fc698e28f37..20c0e0d8fef92 100644 --- a/dashboard/app/controllers/api/v1/pd/international_optins_controller.rb +++ b/dashboard/app/controllers/api/v1/pd/international_opt_ins_controller.rb @@ -1,8 +1,8 @@ -class Api::V1::Pd::InternationalOptinsController < Api::V1::Pd::FormsController - authorize_resource class: 'Pd::InternationalOptin', only: :create +class Api::V1::Pd::InternationalOptInsController < Api::V1::Pd::FormsController + authorize_resource class: 'Pd::InternationalOptIn', only: :create def new_form - @contact_form = ::Pd::InternationalOptin.new( + @contact_form = ::Pd::InternationalOptIn.new( user: current_user ) end @@ -12,7 +12,7 @@ def on_successful_create email: @contact_form.email, opt_in: @contact_form.opt_in?, ip_address: request.ip, - source: EmailPreference::FORM_PD_INTERNATIONAL_OPTIN, + source: EmailPreference::FORM_PD_INTERNATIONAL_OPT_IN, form_kind: "0" ) end diff --git a/dashboard/app/controllers/pd/international_optin_controller.rb b/dashboard/app/controllers/pd/international_opt_in_controller.rb similarity index 51% rename from dashboard/app/controllers/pd/international_optin_controller.rb rename to dashboard/app/controllers/pd/international_opt_in_controller.rb index 5accba7d41c5e..9eeb148835f09 100644 --- a/dashboard/app/controllers/pd/international_optin_controller.rb +++ b/dashboard/app/controllers/pd/international_opt_in_controller.rb @@ -1,7 +1,7 @@ -class Pd::InternationalOptinController < ApplicationController - load_resource :international_optin, class: 'Pd::InternationalOptin', id_param: :contact_id, only: [:thanks] +class Pd::InternationalOptInController < ApplicationController + load_resource :international_opt_in, class: 'Pd::InternationalOptIn', id_param: :contact_id, only: [:thanks] - # GET /pd/international_optins/new + # GET /pd/international_workshopss/new def new return render '/pd/application/teacher_application/logged_out' unless current_user return render '/pd/application/teacher_application/not_teacher' unless current_user.teacher? @@ -9,10 +9,10 @@ def new @script_data = { props: { - options: Pd::InternationalOptin.options.camelize_keys, + options: Pd::InternationalOptIn.options.camelize_keys, accountEmail: current_user.email, - apiEndpoint: "/api/v1/pd/international_optins", - labels: Pd::InternationalOptin.labels + apiEndpoint: "/api/v1/pd/international_opt_ins", + labels: Pd::InternationalOptIn.labels }.to_json } end diff --git a/dashboard/app/models/ability.rb b/dashboard/app/models/ability.rb index 0d0adda978971..4d21ed221651c 100644 --- a/dashboard/app/models/ability.rb +++ b/dashboard/app/models/ability.rb @@ -48,7 +48,7 @@ def initialize(user) Pd::Application::ApplicationBase, Pd::Application::Facilitator1819Application, Pd::Application::Teacher1819Application, - Pd::InternationalOptin, + Pd::InternationalOptIn, :maker_discount ] @@ -93,7 +93,7 @@ def initialize(user) can [:new, :create, :read], Pd::WorkshopMaterialOrder, user_id: user.id can [:new, :create, :read], Pd::Application::Facilitator1819Application, user_id: user.id can [:new, :create, :read], Pd::Application::Teacher1819Application, user_id: user.id - can :create, Pd::InternationalOptin, user_id: user.id + can :create, Pd::InternationalOptIn, user_id: user.id can :manage, :maker_discount end diff --git a/dashboard/app/models/pd/international_optin.rb b/dashboard/app/models/pd/international_opt_in.rb similarity index 84% rename from dashboard/app/models/pd/international_optin.rb rename to dashboard/app/models/pd/international_opt_in.rb index 5ccdaec285af2..0ff032e86a8f6 100644 --- a/dashboard/app/models/pd/international_optin.rb +++ b/dashboard/app/models/pd/international_opt_in.rb @@ -1,6 +1,6 @@ # == Schema Information # -# Table name: pd_international_optins +# Table name: pd_international_opt_ins # # id :integer not null, primary key # user_id :integer not null @@ -10,14 +10,14 @@ # # Indexes # -# index_pd_international_optins_on_user_id (user_id) +# index_pd_international_opt_ins_on_user_id (user_id) # -require 'international_optin_people' +require 'international_opt_in_people' -class Pd::InternationalOptin < ApplicationRecord +class Pd::InternationalOptIn < ApplicationRecord include Pd::Form - include InternationalOptinPeople + include InternationalOptInPeople belongs_to :user @@ -54,8 +54,8 @@ def self.options entries = Hash[entry_keys.map {|k, v| [k, v.map {|s| I18n.t("pd.form_entries.#{k.to_s.underscore}.#{s.underscore}")}]}] - entries[:workshopOrganizer] = INTERNATIONAL_OPTIN_PARTNERS - entries[:workshopFacilitator] = INTERNATIONAL_OPTIN_FACILITATORS + entries[:workshopOrganizer] = INTERNATIONAL_OPT_IN_PARTNERS + entries[:workshopFacilitator] = INTERNATIONAL_OPT_IN_FACILITATORS super.merge(entries) end diff --git a/dashboard/app/views/pd/application/teacher_application/logged_out.haml b/dashboard/app/views/pd/application/teacher_application/logged_out.haml index f3fa5aa8a3262..ea9764183eb8a 100644 --- a/dashboard/app/views/pd/application/teacher_application/logged_out.haml +++ b/dashboard/app/views/pd/application/teacher_application/logged_out.haml @@ -2,17 +2,14 @@ = stylesheet_link_tag 'css/pd', media: 'all' %h1 - Thanks for your interest in the Professional Learning Program! + = I18n.t('logged_out.heading') %p - To get started, you first need to be logged into your Code.org account. - If you’d like more information about the program before you start your application, - please check out the - = link_to('Professional Learning Program overview', 'https://code.org/educate/professional-learning') + '.' + != I18n.t('logged_out.body', url: CDO.code_org_url('/educate/professional-learning')) %div = link_to "/users/sign_in?user_return_to=#{request.fullpath}" do - %button= 'Sign in' + %button= I18n.t('nav.user.signin') = link_to "/users/sign_up?user[user_type]=teacher&user_return_to=#{request.fullpath}" do - %button= 'Create an account' + %button= I18n.t('nav.user.signup') diff --git a/dashboard/app/views/pd/application/teacher_application/no_teacher_email.haml b/dashboard/app/views/pd/application/teacher_application/no_teacher_email.haml index 24a3773f686bc..b5acecd0ae6d1 100644 --- a/dashboard/app/views/pd/application/teacher_application/no_teacher_email.haml +++ b/dashboard/app/views/pd/application/teacher_application/no_teacher_email.haml @@ -2,7 +2,4 @@ = stylesheet_link_tag 'css/pd', media: 'all' %p - Your account doesn’t have an email address. Please - = link_to edit_user_registration_path do - visit your account settings - to add your email address before completing this form. + != I18n.t('no_teacher_email.body', user_registration: edit_user_registration_path) diff --git a/dashboard/app/views/pd/application/teacher_application/not_teacher.haml b/dashboard/app/views/pd/application/teacher_application/not_teacher.haml index 6f810c3c750ca..0d18c0a3a39ca 100644 --- a/dashboard/app/views/pd/application/teacher_application/not_teacher.haml +++ b/dashboard/app/views/pd/application/teacher_application/not_teacher.haml @@ -2,12 +2,6 @@ = stylesheet_link_tag 'css/pd', media: 'all' %p - Thanks for your interest in Code.org’s Professional Learning Program! - You’re currently signed into a student account. Please either - = link_to('create a teacher account', '/users/sign_up?user%5Buser_type%5D=teacher') - or - = link_to('go to your user settings', '/users/edit') - to upgrade your account from student to teacher. - If you’d like more information about the program before you start your application, - please check out the - = link_to('Professional Learning Program overview', 'https://code.org/educate/professional-learning') + '.' + != I18n.t('not_teacher.body', create_teacher: '/users/sign_up?user%5Buser_type%5D=teacher', user_settings: '/users/edit') + + != I18n.t('not_teacher.more', pl_overview: 'https://code.org/educate/professional-learning') diff --git a/dashboard/app/views/pd/international_optin/new.html.haml b/dashboard/app/views/pd/international_opt_in/new.html.haml similarity index 71% rename from dashboard/app/views/pd/international_optin/new.html.haml rename to dashboard/app/views/pd/international_opt_in/new.html.haml index df4156ab0c1cc..3fdb29737e05e 100644 --- a/dashboard/app/views/pd/international_optin/new.html.haml +++ b/dashboard/app/views/pd/international_opt_in/new.html.haml @@ -2,7 +2,7 @@ - content_for(:head) do = stylesheet_link_tag 'css/pd', media: 'all' -%script{src: minifiable_asset_path('js/pd/international_optin/new.js'), data: @script_data} +%script{src: minifiable_asset_path('js/pd/international_opt_in/new.js'), data: @script_data} #application-header %h3 diff --git a/dashboard/app/views/pd/international_optin/thanks.html.haml b/dashboard/app/views/pd/international_opt_in/thanks.html.haml similarity index 100% rename from dashboard/app/views/pd/international_optin/thanks.html.haml rename to dashboard/app/views/pd/international_opt_in/thanks.html.haml diff --git a/dashboard/config/locales/en.yml b/dashboard/config/locales/en.yml index 86aac9d105317..e7edd8f260908 100644 --- a/dashboard/config/locales/en.yml +++ b/dashboard/config/locales/en.yml @@ -1111,3 +1111,12 @@ en: privacy_policy: 'privacy policy' logout: 'Logout' yes: 'Yes' + logged_out: + heading: 'Thanks for your interest in the Professional Learning Program!' + body: 'To get started, you first need to be logged into your Code.org account. If you’d like more information about the program before you start your application, please check out the Professional Learning Program overview.' + not_teacher: + body: 'Thanks for your interest in Code.org’s Professional Learning Program! You’re currently signed into a student account. Please either create a teacher account or go to your user settings to upgrade your account from student to teacher.' + more: 'If you’d like more information about the program before you start your application, please check out the Professional Learning Program overview.' + no_teacher_email: + body: 'Your account doesn’t have an email address. Please visit your account settings to add your email address before completing this form.' + diff --git a/dashboard/config/routes.rb b/dashboard/config/routes.rb index 51385a165a3f5..4c7a0c7ef68b0 100644 --- a/dashboard/config/routes.rb +++ b/dashboard/config/routes.rb @@ -447,7 +447,7 @@ module OPS post :workshop_surveys, to: 'workshop_surveys#create' post :teachercon_surveys, to: 'teachercon_surveys#create' post :regional_partner_contacts, to: 'regional_partner_contacts#create' - post :international_optins, to: 'international_optins#create' + post :international_opt_ins, to: 'international_opt_ins#create' get :regional_partner_workshops, to: 'regional_partner_workshops#index' get 'regional_partner_workshops/find', to: 'regional_partner_workshops#find' @@ -545,8 +545,8 @@ module OPS get 'regional_partner_contact/new', to: 'regional_partner_contact#new' get 'regional_partner_contact/:contact_id/thanks', to: 'regional_partner_contact#thanks' - get 'international_optin/new', to: 'international_optin#new' - get 'international_optin/:contact_id/thanks', to: 'international_optin#thanks' + get 'international_opt_in/new', to: 'international_opt_in#new' + get 'international_opt_in/:contact_id/thanks', to: 'international_opt_in#thanks' # React-router will handle sub-routes on the client. get 'application_dashboard/*path', to: 'application_dashboard#index' diff --git a/dashboard/db/migrate/20180620110434_create_pd_international_optins.rb b/dashboard/db/migrate/20180620110434_create_pd_international_opt_ins.rb similarity index 56% rename from dashboard/db/migrate/20180620110434_create_pd_international_optins.rb rename to dashboard/db/migrate/20180620110434_create_pd_international_opt_ins.rb index 2fc4124b0af3c..b5da051a930a9 100644 --- a/dashboard/db/migrate/20180620110434_create_pd_international_optins.rb +++ b/dashboard/db/migrate/20180620110434_create_pd_international_opt_ins.rb @@ -1,6 +1,6 @@ -class CreatePdInternationalOptins < ActiveRecord::Migration[5.0] +class CreatePdInternationalOptIns < ActiveRecord::Migration[5.0] def change - create_table :pd_international_optins do |t| + create_table :pd_international_opt_ins do |t| t.references :user, index: true, null: false t.text :form_data, null: false t.timestamps null: false diff --git a/dashboard/db/schema.rb b/dashboard/db/schema.rb index a06b5cd2fe58a..8c653f14a17d4 100644 --- a/dashboard/db/schema.rb +++ b/dashboard/db/schema.rb @@ -672,12 +672,12 @@ t.index ["pd_application_id"], name: "index_pd_fit_weekend1819_registrations_on_pd_application_id", using: :btree end - create_table "pd_international_optins", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci" do |t| + create_table "pd_international_opt_ins", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci" do |t| t.integer "user_id", null: false t.text "form_data", limit: 65535, null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.index ["user_id"], name: "index_pd_international_optins_on_user_id", using: :btree + t.index ["user_id"], name: "index_pd_international_opt_ins_on_user_id", using: :btree end create_table "pd_payment_terms", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci" do |t| diff --git a/dashboard/test/controllers/api/v1/pd/international_optins_controller_test.rb b/dashboard/test/controllers/api/v1/pd/international_opt_ins_controller_test.rb similarity index 79% rename from dashboard/test/controllers/api/v1/pd/international_optins_controller_test.rb rename to dashboard/test/controllers/api/v1/pd/international_opt_ins_controller_test.rb index 906bae7c72790..dbfebbe8b67ce 100644 --- a/dashboard/test/controllers/api/v1/pd/international_optins_controller_test.rb +++ b/dashboard/test/controllers/api/v1/pd/international_opt_ins_controller_test.rb @@ -1,6 +1,6 @@ require 'test_helper' -class Api::V1::Pd::InternationalOptinsControllerTest < ::ActionController::TestCase +class Api::V1::Pd::InternationalOptInsControllerTest < ::ActionController::TestCase SAMPLE_FORM_DATA = { first_name: 'First', first_name_preferred: 'Preferred', @@ -26,9 +26,9 @@ class Api::V1::Pd::InternationalOptinsControllerTest < ::ActionController::TestC @teacher = create :teacher end - test 'create creates a new international optin' do + test 'create creates a new international opt-in' do sign_in @teacher - assert_creates Pd::InternationalOptin do + assert_creates Pd::InternationalOptIn do put :create, params: { form_data: SAMPLE_FORM_DATA, user: @teacher @@ -39,13 +39,13 @@ class Api::V1::Pd::InternationalOptinsControllerTest < ::ActionController::TestC assert_response :created end - test 'create returns appropriate errors if international optin data is missing' do + test 'create returns appropriate errors if international opt-in data is missing' do sign_in @teacher new_form = SAMPLE_FORM_DATA.dup new_form.delete :last_name - assert_does_not_create Pd::InternationalOptin do + assert_does_not_create Pd::InternationalOptIn do put :create, params: { form_data: new_form, user: @teacher diff --git a/dashboard/test/factories/pd_factories.rb b/dashboard/test/factories/pd_factories.rb index 12ac2bc8a38c9..d29a592c0eca1 100644 --- a/dashboard/test/factories/pd_factories.rb +++ b/dashboard/test/factories/pd_factories.rb @@ -543,7 +543,7 @@ form_data nil end - factory :pd_international_optin, class: 'Pd::InternationalOptin' do + factory :pd_international_opt_in, class: 'Pd::InternationalOptIn' do user nil form_data nil end diff --git a/dashboard/test/models/pd/international_optin_test.rb b/dashboard/test/models/pd/international_opt_in_test.rb similarity index 57% rename from dashboard/test/models/pd/international_optin_test.rb rename to dashboard/test/models/pd/international_opt_in_test.rb index 297b0791090fb..34fac4d20a993 100644 --- a/dashboard/test/models/pd/international_optin_test.rb +++ b/dashboard/test/models/pd/international_opt_in_test.rb @@ -1,6 +1,6 @@ require 'test_helper' -class Pd::InternationalOptinTest < ActiveSupport::TestCase +class Pd::InternationalOptInTest < ActiveSupport::TestCase FORM_DATA = { firstName: 'First', firstNamePreferred: 'Preferred', @@ -21,17 +21,17 @@ class Pd::InternationalOptinTest < ActiveSupport::TestCase optIn: 'Yes' } - test 'Test international optin validation' do + test 'Test international opt-in validation' do teacher = create :teacher - refute build(:pd_international_optin, form_data: {}.to_json, user_id: teacher.id).valid? + refute build(:pd_international_opt_in, form_data: {}.to_json, user_id: teacher.id).valid? refute build( - :pd_international_optin, form_data: FORM_DATA.merge({ages: nil}).to_json, user_id: teacher.id + :pd_international_opt_in, form_data: FORM_DATA.merge({ages: nil}).to_json, user_id: teacher.id ).valid? - assert build(:pd_international_optin, form_data: FORM_DATA.to_json, user_id: teacher.id).valid? + assert build(:pd_international_opt_in, form_data: FORM_DATA.to_json, user_id: teacher.id).valid? - refute build(:pd_international_optin, form_data: FORM_DATA.to_json).valid? + refute build(:pd_international_opt_in, form_data: FORM_DATA.to_json).valid? end end diff --git a/lib/cdo/delete_accounts_helper.rb b/lib/cdo/delete_accounts_helper.rb index db66181f8ea75..cd6472b131c5d 100644 --- a/lib/cdo/delete_accounts_helper.rb +++ b/lib/cdo/delete_accounts_helper.rb @@ -116,7 +116,7 @@ def clean_and_destroy_pd_content(user_id) Pd::FacilitatorProgramRegistration.where(user_id: user_id).each(&:clear_form_data) Pd::RegionalPartnerProgramRegistration.where(user_id: user_id).each(&:clear_form_data) Pd::WorkshopMaterialOrder.where(user_id: user_id).each(&:clear_data) - Pd::InternationalOptin.where(user_id: user_id).each(&:clear_form_data) + Pd::InternationalOptIn.where(user_id: user_id).each(&:clear_form_data) pd_enrollment_id = Pd::Enrollment.where(user_id: user_id).pluck(:id).first if pd_enrollment_id diff --git a/lib/cdo/email_preference_constants.rb b/lib/cdo/email_preference_constants.rb index 033655c860fc1..dc39a810a5a85 100644 --- a/lib/cdo/email_preference_constants.rb +++ b/lib/cdo/email_preference_constants.rb @@ -15,7 +15,7 @@ module EmailPreferenceConstants FORM_HOC_SIGN_UP = 'FORM_HOC_SIGN_UP'.freeze, FORM_CLASS_SUBMIT = 'FORM_CLASS_SUBMIT'.freeze, FORM_PETITION = 'FORM_PETITION'.freeze, - FORM_PD_INTERNATIONAL_OPTIN = 'FORM_PD_INTERNATIONAL_OPTIN'.freeze, + FORM_PD_INTERNATIONAL_OPT_IN = 'FORM_PD_INTERNATIONAL_OPT_IN'.freeze, # A one-time automated script sets all Petition submissions younger than 16 and older than 13 to opted out to comply # with GDPR. AUTOMATED_OPT_OUT_UNDER_16 = 'AUTOMATED_OPT_OUT_UNDER_16'.freeze diff --git a/lib/international_optin_people.rb b/lib/international_opt_in_people.rb similarity index 60% rename from lib/international_optin_people.rb rename to lib/international_opt_in_people.rb index 496601aa98425..6b30b190fd85f 100644 --- a/lib/international_optin_people.rb +++ b/lib/international_opt_in_people.rb @@ -1,11 +1,11 @@ -module InternationalOptinPeople - INTERNATIONAL_OPTIN_FACILITATORS = [ +module InternationalOptInPeople + INTERNATIONAL_OPT_IN_FACILITATORS = [ "First facilitator", "Second facilitator", "Third facilitator" ].freeze - INTERNATIONAL_OPTIN_PARTNERS = [ + INTERNATIONAL_OPT_IN_PARTNERS = [ "First partner", "Second partner", "Third partner" From bcadab5e2ac43afc4fc5d4752eae3f68b0ce8f36 Mon Sep 17 00:00:00 2001 From: Brendan Reville Date: Mon, 2 Jul 2018 12:46:07 +1000 Subject: [PATCH 13/89] PD: international opt-in gets more strings & loc --- apps/i18n/common/en_us.json | 2 ++ .../pd/form_components/FormController.jsx | 7 +++--- .../app/models/pd/international_opt_in.rb | 1 + .../teacher_application/logged_out.haml | 4 ++-- .../teacher_application/no_teacher_email.haml | 2 +- .../teacher_application/not_teacher.haml | 5 ++-- .../pd/international_opt_in/new.html.haml | 9 +++++--- .../pd/international_opt_in/thanks.html.haml | 2 +- dashboard/config/locales/en.yml | 23 +++++++++++-------- 9 files changed, 34 insertions(+), 21 deletions(-) diff --git a/apps/i18n/common/en_us.json b/apps/i18n/common/en_us.json index d73d14cd3ad1b..038d879dbb49b 100644 --- a/apps/i18n/common/en_us.json +++ b/apps/i18n/common/en_us.json @@ -538,6 +538,8 @@ "findLocalClassDescription": "Find a local after-school program, summer camp, or school to learn in person.", "findLocalClassButton": "Find a class", "finish": "Finish", + "formErrorsBelow": "Please correct the errors below.", + "formServerError": "Something went wrong on our end; please try again later.", "forTeachersOnly": "For Teachers Only", "fromDaysAgo": "(From {number} days ago)", "gdprDialogHeader": "Do you agree to using a website based in the United States?", diff --git a/apps/src/code-studio/pd/form_components/FormController.jsx b/apps/src/code-studio/pd/form_components/FormController.jsx index 7d67345a75509..92f9fd7fe7fe2 100644 --- a/apps/src/code-studio/pd/form_components/FormController.jsx +++ b/apps/src/code-studio/pd/form_components/FormController.jsx @@ -6,6 +6,7 @@ import { FormGroup, Pagination, } from 'react-bootstrap'; +import i18n from '@cdo/locale'; const styles = { pageButtons: { @@ -211,14 +212,14 @@ export default class FormController extends React.Component { // and display the generic error header this.setState({ errors: data.responseJSON.errors.form_data, - errorHeader: "Please correct the errors below." + errorHeader: i18n.formErrorsBelow() }); } } else { // Otherwise, something unknown went wrong on the server this.setState({ globalError: true, - errorHeader: "Something went wrong on our end; please try again later." + errorHeader: i18n.formServerError() }); } this.setState({ @@ -405,7 +406,7 @@ export default class FormController extends React.Component { shouldShowSubmit() { return this.state.currentPage === this.getPageComponents().length - 1; } - static submitButtonText = "Submit"; + static submitButtonText = i18n.submit(); /** * @returns {Element} diff --git a/dashboard/app/models/pd/international_opt_in.rb b/dashboard/app/models/pd/international_opt_in.rb index 0ff032e86a8f6..bc9876b178186 100644 --- a/dashboard/app/models/pd/international_opt_in.rb +++ b/dashboard/app/models/pd/international_opt_in.rb @@ -28,6 +28,7 @@ def self.required_fields :first_name, :last_name, :gender, + :school_name, :school_city, :school_country, :ages, diff --git a/dashboard/app/views/pd/application/teacher_application/logged_out.haml b/dashboard/app/views/pd/application/teacher_application/logged_out.haml index ea9764183eb8a..3f2449e81a3c2 100644 --- a/dashboard/app/views/pd/application/teacher_application/logged_out.haml +++ b/dashboard/app/views/pd/application/teacher_application/logged_out.haml @@ -2,10 +2,10 @@ = stylesheet_link_tag 'css/pd', media: 'all' %h1 - = I18n.t('logged_out.heading') + = I18n.t('pd.logged_out.heading') %p - != I18n.t('logged_out.body', url: CDO.code_org_url('/educate/professional-learning')) + != I18n.t('pd.logged_out.body', url: CDO.code_org_url('/educate/professional-learning')) %div = link_to "/users/sign_in?user_return_to=#{request.fullpath}" do diff --git a/dashboard/app/views/pd/application/teacher_application/no_teacher_email.haml b/dashboard/app/views/pd/application/teacher_application/no_teacher_email.haml index b5acecd0ae6d1..951d398289740 100644 --- a/dashboard/app/views/pd/application/teacher_application/no_teacher_email.haml +++ b/dashboard/app/views/pd/application/teacher_application/no_teacher_email.haml @@ -2,4 +2,4 @@ = stylesheet_link_tag 'css/pd', media: 'all' %p - != I18n.t('no_teacher_email.body', user_registration: edit_user_registration_path) + != I18n.t('pd.no_teacher_email.body', user_registration: edit_user_registration_path) diff --git a/dashboard/app/views/pd/application/teacher_application/not_teacher.haml b/dashboard/app/views/pd/application/teacher_application/not_teacher.haml index 0d18c0a3a39ca..0d76101f8f0db 100644 --- a/dashboard/app/views/pd/application/teacher_application/not_teacher.haml +++ b/dashboard/app/views/pd/application/teacher_application/not_teacher.haml @@ -2,6 +2,7 @@ = stylesheet_link_tag 'css/pd', media: 'all' %p - != I18n.t('not_teacher.body', create_teacher: '/users/sign_up?user%5Buser_type%5D=teacher', user_settings: '/users/edit') + != I18n.t('pd.not_teacher.body', create_teacher: '/users/sign_up?user%5Buser_type%5D=teacher', user_settings: '/users/edit') - != I18n.t('not_teacher.more', pl_overview: 'https://code.org/educate/professional-learning') +%p + != I18n.t('pd.not_teacher.more', pl_overview: CDO.code_org_url('/educate/professional-learning')) diff --git a/dashboard/app/views/pd/international_opt_in/new.html.haml b/dashboard/app/views/pd/international_opt_in/new.html.haml index 3fdb29737e05e..1b033d8518406 100644 --- a/dashboard/app/views/pd/international_opt_in/new.html.haml +++ b/dashboard/app/views/pd/international_opt_in/new.html.haml @@ -1,4 +1,4 @@ -- @page_title = 'International opt-in' +- @page_title = I18n.t('pd.international_opt_in.title') - content_for(:head) do = stylesheet_link_tag 'css/pd', media: 'all' @@ -6,9 +6,12 @@ #application-header %h3 - International opt-in + = I18n.t('pd.international_opt_in.title') %p - This page is for international opt-in. + = I18n.t('pd.international_opt_in.intro') + +%p + = I18n.t('pd.international_opt_in.instructions') %div#application-container diff --git a/dashboard/app/views/pd/international_opt_in/thanks.html.haml b/dashboard/app/views/pd/international_opt_in/thanks.html.haml index 655f2281dea9e..2ba9b35e5982a 100644 --- a/dashboard/app/views/pd/international_opt_in/thanks.html.haml +++ b/dashboard/app/views/pd/international_opt_in/thanks.html.haml @@ -1 +1 @@ -Thanks for doing an international opt-in! += I18n.t('pd.international_opt_in.thanks') diff --git a/dashboard/config/locales/en.yml b/dashboard/config/locales/en.yml index e7edd8f260908..2a85017c91ff7 100644 --- a/dashboard/config/locales/en.yml +++ b/dashboard/config/locales/en.yml @@ -1038,7 +1038,7 @@ en: workshop_organizer: 'Workshop Organizer' workshop_facilitator: 'Workshop Facilitator' workshop_course: 'Workshop Course' - opt_in: 'I want to share my data with Code.org, International Partner' + opt_in: 'I agree that Code.org can share my contact information and aggregate data about my classes with the Code.org International Partner in my country.' form_entries: gender: male: 'Male' @@ -1099,6 +1099,19 @@ en: opt_in: opt_in_yes: 'Yes' opt_in_no: 'No' + international_opt_in: + title: 'Workshop Attendance for non-U.S. Teachers' + intro: "Thank you for taking time to receive training on Code.org’s curriculum. Our mission to bring computer science education opportunities to all children in all schools around the world would not be possible without you." + instructions: "Please fill out the form below to indicate that 1) you attended a training workshop run by one of Code.org’s international partners, and 2) you agree to share your information with our international partner in your country so that we can help to support your journey as a computer science teacher." + thanks: "Thank you for submitting! We look forward to supporting you as you get to know Code.org’s curriculum and tools. Please keep sharing your questions and feedback with us by writing to us at support@code.org or by posting on the teacher forum!" + logged_out: + heading: 'Thanks for your interest in the Professional Learning Program!' + body: 'To get started, you first need to be logged into your Code.org account. If you’d like more information about the program before you start your application, please check out the Professional Learning Program overview.' + not_teacher: + body: 'Thanks for your interest in Code.org’s Professional Learning Program! You’re currently signed into a student account. Please either create a teacher account or go to your user settings to upgrade your account from student to teacher.' + more: 'If you’d like more information about the program before you start your application, please check out the Professional Learning Program overview.' + no_teacher_email: + body: 'Your account doesn’t have an email address. Please visit your account settings to add your email address before completing this form.' courses_category: 'Full Courses' cookie_banner: message: 'By continuing to browse our site or clicking "I agree," you agree to the storing of cookies on your computer or device.' @@ -1111,12 +1124,4 @@ en: privacy_policy: 'privacy policy' logout: 'Logout' yes: 'Yes' - logged_out: - heading: 'Thanks for your interest in the Professional Learning Program!' - body: 'To get started, you first need to be logged into your Code.org account. If you’d like more information about the program before you start your application, please check out the Professional Learning Program overview.' - not_teacher: - body: 'Thanks for your interest in Code.org’s Professional Learning Program! You’re currently signed into a student account. Please either create a teacher account or go to your user settings to upgrade your account from student to teacher.' - more: 'If you’d like more information about the program before you start your application, please check out the Professional Learning Program overview.' - no_teacher_email: - body: 'Your account doesn’t have an email address. Please visit your account settings to add your email address before completing this form.' From 48f93f1ce0af914de84e45e44f2857f91ff7cd57 Mon Sep 17 00:00:00 2001 From: Brendan Reville Date: Mon, 2 Jul 2018 12:57:49 +1000 Subject: [PATCH 14/89] PD: international opt-in URL now /pd/international_workshop --- .../pd/international_opt_in/InternationalOptIn.jsx | 2 +- dashboard/config/routes.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/src/code-studio/pd/international_opt_in/InternationalOptIn.jsx b/apps/src/code-studio/pd/international_opt_in/InternationalOptIn.jsx index 459b1cbb63b53..1b6881376596c 100644 --- a/apps/src/code-studio/pd/international_opt_in/InternationalOptIn.jsx +++ b/apps/src/code-studio/pd/international_opt_in/InternationalOptIn.jsx @@ -22,7 +22,7 @@ export default class InternationalOptIn extends FormController { * @override */ onSuccessfulSubmit(data) { - window.location = `/pd/international_opt_in/${data.id}/thanks`; + window.location = `/pd/international_workshop/${data.id}/thanks`; } /** diff --git a/dashboard/config/routes.rb b/dashboard/config/routes.rb index 4c7a0c7ef68b0..cd58d4b748417 100644 --- a/dashboard/config/routes.rb +++ b/dashboard/config/routes.rb @@ -545,8 +545,8 @@ module OPS get 'regional_partner_contact/new', to: 'regional_partner_contact#new' get 'regional_partner_contact/:contact_id/thanks', to: 'regional_partner_contact#thanks' - get 'international_opt_in/new', to: 'international_opt_in#new' - get 'international_opt_in/:contact_id/thanks', to: 'international_opt_in#thanks' + get 'international_workshop', to: 'international_opt_in#new' + get 'international_workshop/:contact_id/thanks', to: 'international_opt_in#thanks' # React-router will handle sub-routes on the client. get 'application_dashboard/*path', to: 'application_dashboard#index' From f44b7310ae4573457b55892968cb9cdc0f4ead7d Mon Sep 17 00:00:00 2001 From: Brendan Reville Date: Tue, 3 Jul 2018 10:43:37 +1000 Subject: [PATCH 15/89] PD: international opt-in gets email & legal opt-ins --- .../pd/international_opt_in/InternationalOptIn.jsx | 13 ++++++++++--- .../api/v1/pd/international_opt_ins_controller.rb | 2 +- dashboard/app/models/pd/international_opt_in.rb | 13 ++++++++----- dashboard/config/locales/en.yml | 5 +++-- .../v1/pd/international_opt_ins_controller_test.rb | 3 ++- .../test/models/pd/international_opt_in_test.rb | 3 ++- 6 files changed, 26 insertions(+), 13 deletions(-) diff --git a/apps/src/code-studio/pd/international_opt_in/InternationalOptIn.jsx b/apps/src/code-studio/pd/international_opt_in/InternationalOptIn.jsx index 1b6881376596c..86bfb5d2da3e6 100644 --- a/apps/src/code-studio/pd/international_opt_in/InternationalOptIn.jsx +++ b/apps/src/code-studio/pd/international_opt_in/InternationalOptIn.jsx @@ -245,13 +245,20 @@ class InternationalOptInComponent extends FormComponent { } { this.buildButtonsFromOptions({ - name: 'optIn', - label: labels.optIn, + name: 'emailOptIn', + label: labels.emailOptIn, type: 'radio', required: true, placeholder: i18n.selectAnOption() }) } + { + this.buildSingleCheckbox({ + name: 'legalOptIn', + label: labels.legalOptIn, + required: true + }) + } ); } @@ -261,5 +268,5 @@ InternationalOptInComponent.associatedFields = [ 'firstName', 'firstNamePreferred', 'lastName', 'email', 'emailAlternate', 'gender', 'schoolName', 'schoolCity', 'schoolCountry', 'ages', 'subjects', 'resources', 'robotics', 'workshopOrganizer', 'workshopFacilitator', 'workshopCourse', - 'optIn' + 'emailOptIn', 'legalOptIn' ]; diff --git a/dashboard/app/controllers/api/v1/pd/international_opt_ins_controller.rb b/dashboard/app/controllers/api/v1/pd/international_opt_ins_controller.rb index 20c0e0d8fef92..51b03860c391f 100644 --- a/dashboard/app/controllers/api/v1/pd/international_opt_ins_controller.rb +++ b/dashboard/app/controllers/api/v1/pd/international_opt_ins_controller.rb @@ -10,7 +10,7 @@ def new_form def on_successful_create EmailPreference.upsert!( email: @contact_form.email, - opt_in: @contact_form.opt_in?, + opt_in: @contact_form.email_opt_in?, ip_address: request.ip, source: EmailPreference::FORM_PD_INTERNATIONAL_OPT_IN, form_kind: "0" diff --git a/dashboard/app/models/pd/international_opt_in.rb b/dashboard/app/models/pd/international_opt_in.rb index bc9876b178186..ba3949419b994 100644 --- a/dashboard/app/models/pd/international_opt_in.rb +++ b/dashboard/app/models/pd/international_opt_in.rb @@ -37,7 +37,8 @@ def self.required_fields :workshop_organizer, :workshop_facilitator, :workshop_course, - :opt_in + :email_opt_in, + :legal_opt_in ] end @@ -50,7 +51,8 @@ def self.options resources: %w(bootstrap codecademy csfirst khan kodable lightbot scratch tynker other), robotics: %w(grok kodable lego microbit ozobot sphero raspberry wonder other), workshopCourse: %w(csf_af csf_express), - optIn: %w(opt_in_yes opt_in_no) + emailOptIn: %w(opt_in_yes opt_in_no), + legalOptIn: %w(opt_in_yes opt_in_no) } entries = Hash[entry_keys.map {|k, v| [k, v.map {|s| I18n.t("pd.form_entries.#{k.to_s.underscore}.#{s.underscore}")}]}] @@ -79,13 +81,14 @@ def self.labels workshopOrganizer workshopFacilitator workshopCourse - optIn + emailOptIn + legalOptIn ) Hash[keys.collect {|v| [v, I18n.t("pd.form_labels.#{v.underscore}")]}] end - def opt_in? - sanitize_form_data_hash[:opt_in].downcase == "yes" + def email_opt_in? + sanitize_form_data_hash[:email_opt_in].downcase == "yes" end def email diff --git a/dashboard/config/locales/en.yml b/dashboard/config/locales/en.yml index 2a85017c91ff7..c20676c373229 100644 --- a/dashboard/config/locales/en.yml +++ b/dashboard/config/locales/en.yml @@ -1038,7 +1038,8 @@ en: workshop_organizer: 'Workshop Organizer' workshop_facilitator: 'Workshop Facilitator' workshop_course: 'Workshop Course' - opt_in: 'I agree that Code.org can share my contact information and aggregate data about my classes with the Code.org International Partner in my country.' + email_opt_in: 'I agree that Code.org can share my contact information and aggregate data about my classes with the Code.org International Partner in my country.' + legal_opt_in: 'By submitting this form, you are agreeing to allow Code.org to share information on how you use Code.org and the Professional Learning resources with the Code.org International Partner in your country, school district, and facilitators. We will share your contact information, which courses/units you are using and aggregate data about your classes with these partners. This includes the number of students in your classes, the demographic breakdown of your classroom, and the name of your school and district. We will not share any information about individual students with our partners: all student information will be de-identified and aggregated. Our International Partners and facilitators are contractually obliged to treat this information with the same level of confidentiality as Code.org.' form_entries: gender: male: 'Male' @@ -1096,7 +1097,7 @@ en: workshop_course: csf_af: 'CS Fundamentals (Courses A-F)' csf_express: 'CS Fundamentals (Pre-Express or Express)' - opt_in: + email_opt_in: opt_in_yes: 'Yes' opt_in_no: 'No' international_opt_in: diff --git a/dashboard/test/controllers/api/v1/pd/international_opt_ins_controller_test.rb b/dashboard/test/controllers/api/v1/pd/international_opt_ins_controller_test.rb index dbfebbe8b67ce..e933b64987a6b 100644 --- a/dashboard/test/controllers/api/v1/pd/international_opt_ins_controller_test.rb +++ b/dashboard/test/controllers/api/v1/pd/international_opt_ins_controller_test.rb @@ -18,7 +18,8 @@ class Api::V1::Pd::InternationalOptInsControllerTest < ::ActionController::TestC workshop_organizer: 'Workshop Organizer', workshop_facilitator: 'Workshop Facilitator', workshop_course: 'Workshop Course', - opt_in: 'Yes' + email_opt_in: 'Yes', + legal_opt_in: true } self.use_transactional_test_case = true diff --git a/dashboard/test/models/pd/international_opt_in_test.rb b/dashboard/test/models/pd/international_opt_in_test.rb index 34fac4d20a993..e332d884a4b91 100644 --- a/dashboard/test/models/pd/international_opt_in_test.rb +++ b/dashboard/test/models/pd/international_opt_in_test.rb @@ -18,7 +18,8 @@ class Pd::InternationalOptInTest < ActiveSupport::TestCase workshopOrganizer: 'Workshop Organizer', workshopFacilitator: 'Workshop Facilitator', workshopCourse: 'Workshop Course', - optIn: 'Yes' + emailOptIn: 'Yes', + legalOptIn: true } test 'Test international opt-in validation' do From 7da127e62b749c0bdfdaea0e1cb2b0fd45b19d97 Mon Sep 17 00:00:00 2001 From: eric Date: Mon, 2 Jul 2018 19:05:27 -0700 Subject: [PATCH 16/89] Multi-auth: fix silent takeover --- .../omniauth_callbacks_controller.rb | 36 ++++++--- .../omniauth_callbacks_controller_test.rb | 79 +++++++++++++++++++ 2 files changed, 104 insertions(+), 11 deletions(-) diff --git a/dashboard/app/controllers/omniauth_callbacks_controller.rb b/dashboard/app/controllers/omniauth_callbacks_controller.rb index cb64da749349b..985758bd93ce8 100644 --- a/dashboard/app/controllers/omniauth_callbacks_controller.rb +++ b/dashboard/app/controllers/omniauth_callbacks_controller.rb @@ -79,8 +79,8 @@ def login # If email is already taken, persisted? will be false because of a validation failure check_and_apply_oauth_takeover(@user) sign_in_user - elsif allows_silent_takeover(@user) - silent_takeover(@user) + elsif allows_silent_takeover(@user, auth_hash) + silent_takeover(@user, auth_hash) elsif (looked_up_user = User.find_by_email_or_hashed_email(@user.email)) # Note that @user.email is populated by User.from_omniauth even for students if looked_up_user.provider == 'clever' @@ -174,14 +174,29 @@ def just_authorized_google_classroom(user, params) scopes.include?('classroom.rosters.readonly') end - def silent_takeover(oauth_user) + def silent_takeover(oauth_user, auth_hash) # Copy oauth details to primary account @user = User.find_by_email_or_hashed_email(oauth_user.email) - @user.provider = oauth_user.provider - @user.uid = oauth_user.uid - @user.oauth_refresh_token = oauth_user.oauth_refresh_token - @user.oauth_token = oauth_user.oauth_token - @user.oauth_token_expiration = oauth_user.oauth_token_expiration + if @user.migrated? + AuthenticationOption.new( + user: @user, + email: oauth_user.email, + hashed_email: oauth_user.hashed_email, + credential_type: auth_hash.provider.to_s, + authentication_id: auth_hash.uid, + data: { + oauth_token: auth_hash.credentials&.token, + oauth_token_expiration: auth_hash.credentials&.expires_at, + oauth_refresh_token: auth_hash.credentials&.refresh_token + } + ).save + else + @user.provider = oauth_user.provider + @user.uid = oauth_user.uid + @user.oauth_refresh_token = oauth_user.oauth_refresh_token + @user.oauth_token = oauth_user.oauth_token + @user.oauth_token_expiration = oauth_user.oauth_token_expiration + end sign_in_user end @@ -190,10 +205,9 @@ def sign_in_user sign_in_and_redirect @user end - def allows_silent_takeover(oauth_user) + def allows_silent_takeover(oauth_user, auth_hash) allow_takeover = oauth_user.provider.present? - allow_takeover &= %w(facebook google_oauth2 windowslive).include?(oauth_user.provider) - # allow_takeover &= oauth_user.email_verified # TODO (eric) - set up and test for different providers + allow_takeover &= %w(facebook google_oauth2 windowslive).include?(auth_hash.provider.to_s) lookup_user = User.find_by_email_or_hashed_email(oauth_user.email) allow_takeover && lookup_user end diff --git a/dashboard/test/controllers/omniauth_callbacks_controller_test.rb b/dashboard/test/controllers/omniauth_callbacks_controller_test.rb index ff3ce86823913..f74cc2d44730d 100644 --- a/dashboard/test/controllers/omniauth_callbacks_controller_test.rb +++ b/dashboard/test/controllers/omniauth_callbacks_controller_test.rb @@ -414,6 +414,85 @@ class OmniauthCallbacksControllerTest < ActionController::TestCase assert_equal migrated_student.id, signed_in_user_id end + test 'login: google_oauth2 silently takes over unmigrated student with matching email' do + email = 'test@foo.xyz' + uid = '654321' + user = create(:student, email: email) + auth = generate_auth_user_hash(provider: 'google_oauth2', uid: uid, user_type: User::TYPE_STUDENT, email: email) + @request.env['omniauth.auth'] = auth + @request.env['omniauth.params'] = {} + assert_does_not_create(User) do + get :google_oauth2 + end + user.reload + assert_equal 'google_oauth2', user.provider + assert_equal user.uid, uid + end + + test 'login: google_oauth2 silently takes over unmigrated teacher with matching email' do + email = 'test@foo.xyz' + uid = '654321' + user = create(:teacher, email: email) + auth = generate_auth_user_hash(provider: 'google_oauth2', uid: uid, user_type: User::TYPE_TEACHER, email: email) + @request.env['omniauth.auth'] = auth + @request.env['omniauth.params'] = {} + assert_does_not_create(User) do + get :google_oauth2 + end + user.reload + assert_equal 'google_oauth2', user.provider + assert_equal user.uid, uid + end + + test 'login: google_oauth2 silently adds authentication_option to migrated student with matching email' do + email = 'test@foo.xyz' + uid = '654321' + user = create(:student, :with_migrated_email_authentication_option, email: email) + auth = generate_auth_user_hash(provider: 'google_oauth2', uid: uid, user_type: User::TYPE_STUDENT, email: email) + @request.env['omniauth.auth'] = auth + @request.env['omniauth.params'] = {} + assert_does_not_create(User) do + get :google_oauth2 + end + user.reload + assert_equal 'migrated', user.provider + found_google = user.authentication_options.any? {|auth_option| auth_option.credential_type == AuthenticationOption::GOOGLE} + assert found_google + end + + test 'login: google_oauth2 silently adds authentication_option to migrated teacher with matching email' do + email = 'test@foo.xyz' + uid = '654321' + user = create(:teacher, :with_migrated_email_authentication_option, email: email) + auth = generate_auth_user_hash(provider: 'google_oauth2', uid: uid, user_type: User::TYPE_TEACHER, email: email) + @request.env['omniauth.auth'] = auth + @request.env['omniauth.params'] = {} + assert_does_not_create(User) do + get :google_oauth2 + end + user.reload + assert_equal 'migrated', user.provider + found_google = user.authentication_options.any? {|auth_option| auth_option.credential_type == AuthenticationOption::GOOGLE} + assert found_google + end + + test 'login: clever does not silently add authentication_option to migrated student with matching email' do + email = 'test@foo.xyz' + uid = '654321' + user = create(:student, :with_migrated_email_authentication_option, email: email) + auth = generate_auth_user_hash(provider: 'clever', uid: uid, user_type: User::TYPE_STUDENT, email: email) + @request.env['omniauth.auth'] = auth + @request.env['omniauth.params'] = {} + assert_creates(User) do + get :clever + end + user.reload + assert_equal 'migrated', user.provider + found_clever = user.authentication_options.any? {|auth_option| auth_option.credential_type == AuthenticationOption::CLEVER} + assert !found_clever + assert_equal 'clever', User.last.provider # NOTE: this will fail when we create migrated users by default + end + test 'connect_provider: returns bad_request if user not migrated' do user = create :user, :unmigrated_facebook_sso Timecop.freeze do From f74ef81f1fd8584d8225be8375bbb7be9c45ab93 Mon Sep 17 00:00:00 2001 From: eric Date: Mon, 2 Jul 2018 19:11:15 -0700 Subject: [PATCH 17/89] Throw exception if save fails --- dashboard/app/controllers/omniauth_callbacks_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dashboard/app/controllers/omniauth_callbacks_controller.rb b/dashboard/app/controllers/omniauth_callbacks_controller.rb index 985758bd93ce8..dd6478749c69f 100644 --- a/dashboard/app/controllers/omniauth_callbacks_controller.rb +++ b/dashboard/app/controllers/omniauth_callbacks_controller.rb @@ -189,7 +189,7 @@ def silent_takeover(oauth_user, auth_hash) oauth_token_expiration: auth_hash.credentials&.expires_at, oauth_refresh_token: auth_hash.credentials&.refresh_token } - ).save + ).save! else @user.provider = oauth_user.provider @user.uid = oauth_user.uid From 457f2e2cca529def4201514225d1b40dae852355 Mon Sep 17 00:00:00 2001 From: eric Date: Mon, 2 Jul 2018 19:56:42 -0700 Subject: [PATCH 18/89] Add tests for connecting with identical email addresses but different providers --- .../omniauth_callbacks_controller_test.rb | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/dashboard/test/controllers/omniauth_callbacks_controller_test.rb b/dashboard/test/controllers/omniauth_callbacks_controller_test.rb index f74cc2d44730d..ab0be458aa7fa 100644 --- a/dashboard/test/controllers/omniauth_callbacks_controller_test.rb +++ b/dashboard/test/controllers/omniauth_callbacks_controller_test.rb @@ -493,6 +493,75 @@ class OmniauthCallbacksControllerTest < ActionController::TestCase assert_equal 'clever', User.last.provider # NOTE: this will fail when we create migrated users by default end + test 'connect_provider: can connect multiple auth options with the same email to the same user' do + email = 'test@xyz.foo' + user = create :user, :multi_auth_migrated, uid: 'some-uid' + AuthenticationOption.new( + { + user: user, + email: email, + hashed_email: User.hash_email(email), + credential_type: 'google_oauth2', + authentication_id: 'some-uid', + data: { + oauth_token: 'fake_token', + oauth_token_expiration: '999999', + oauth_refresh_token: 'fake_refresh_token' + } + } + ).save! + + auth = generate_auth_user_hash(provider: 'facebook', uid: user.uid, refresh_token: '65432', email: email) + @request.env['omniauth.auth'] = auth + + Timecop.freeze do + setup_should_connect_provider(user, 2.days.from_now) + assert_creates(AuthenticationOption) do + get :facebook + end + + user.reload + assert_response :success + assert_equal 2, user.authentication_options.length + end + end + + test 'connect_provider: cannot connect multiple auth options with the same email to a different user' do + email = 'test@xyz.foo' + user_a = create :user, :multi_auth_migrated + AuthenticationOption.new( + { + user: user_a, + email: email, + hashed_email: User.hash_email(email), + credential_type: 'google_oauth2', + authentication_id: 'some-uid', + data: { + oauth_token: 'fake_token', + oauth_token_expiration: '999999', + oauth_refresh_token: 'fake_refresh_token' + } + } + ).save! + + user_b = create :user, :multi_auth_migrated + auth = generate_auth_user_hash(provider: 'facebook', uid: 'some-other-uid', refresh_token: '65432', email: email) + @request.env['omniauth.auth'] = auth + + Timecop.freeze do + setup_should_connect_provider(user_b, 2.days.from_now) + assert_does_not_create(AuthenticationOption) do + get :facebook + end + + assert_response 422 + end + user_a.reload + user_b.reload + assert_equal 1, user_a.authentication_options.length + assert_equal 0, user_b.authentication_options.length + end + test 'connect_provider: returns bad_request if user not migrated' do user = create :user, :unmigrated_facebook_sso Timecop.freeze do From 1ac8483917700babe74164e83013c093e638ae31 Mon Sep 17 00:00:00 2001 From: Dave Bailey Date: Fri, 29 Jun 2018 11:03:18 -0700 Subject: [PATCH 19/89] Revert "Revert "Use commit-specific sources directory on test environment"" --- deployment.rb | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/deployment.rb b/deployment.rb index 71a7f1988c5b9..a3d1f2ad08be6 100644 --- a/deployment.rb +++ b/deployment.rb @@ -11,6 +11,7 @@ require 'cdo/slog' require 'os' require 'cdo/aws/cdo_google_credentials' +require 'cdo/git_utils' def load_yaml_file(path) return nil unless File.file?(path) @@ -25,6 +26,22 @@ def load_languages(path) end end +# Since channel ids are derived from user id and other sequential integer ids +# use a new S3 sources directory for each Test Build to prevent a UI test +# from inadvertently using a channel id from a previous Test Build. +# CircleCI environments already override the sources_s3_directory setting to suffix it with the Circle Build number: +# https://github.com/code-dot-org/code-dot-org/blob/fb53af48ec0598692ed19f340f26d2ed0bd9547b/.circleci/config.yml#L153 +# Detect Circle environment just to be safe. +def sources_s3_dir(environment) + if environment == :production + 'sources' + elsif environment == :test && !ENV['CIRCLECI'] + "sources_#{environment}/#{GitUtils.git_revision_short}" + else + "sources_#{environment}" + end +end + def load_configuration root_dir = File.expand_path('..', __FILE__) root_dir = '/home/ubuntu/website-ci' if root_dir == '/home/ubuntu/Dropbox (Code.org)' @@ -107,7 +124,7 @@ def load_configuration 'assets_s3_bucket' => 'cdo-v3-assets', 'assets_s3_directory' => rack_env == :production ? 'assets' : "assets_#{rack_env}", 'sources_s3_bucket' => 'cdo-v3-sources', - 'sources_s3_directory' => rack_env == :production ? 'sources' : "sources_#{rack_env}", + 'sources_s3_directory' => sources_s3_dir(rack_env), 'use_pusher' => false, 'pusher_app_id' => 'fake_app_id', 'pusher_application_key' => 'fake_application_key', From e67b3c259570168dbd75bef482fd74a64f80eee7 Mon Sep 17 00:00:00 2001 From: Maddie Kasula Date: Tue, 3 Jul 2018 09:43:41 -0700 Subject: [PATCH 20/89] Hook up all providers to toggleProviderConnection --- apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx b/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx index 91906403f22aa..ad10c4514eec0 100644 --- a/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx +++ b/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx @@ -74,20 +74,19 @@ export default class ManageLinkedAccounts extends React.Component { type={OAUTH_PROVIDERS.MICROSOFT} displayName={i18n.manageLinkedAccounts_microsoft()} email={this.getEmailForProvider(OAUTH_PROVIDERS.MICROSOFT)} - onClick={() => {}} + onClick={() => this.toggleProviderConnection(OAUTH_PROVIDERS.MICROSOFT)} /> {}} + onClick={() => this.toggleProviderConnection(OAUTH_PROVIDERS.CLEVER)} /> {}} - cannotDisconnect + onClick={() => this.toggleProviderConnection(OAUTH_PROVIDERS.FACEBOOK)} /> From 7de237b81761f7c8958ef5bfd4940e543c55ed00 Mon Sep 17 00:00:00 2001 From: David Bailey Date: Tue, 3 Jul 2018 09:56:58 -0700 Subject: [PATCH 21/89] replace 'sources_test' with CDO.sources_s3_directory --- shared/test/test_sources.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/shared/test/test_sources.rb b/shared/test/test_sources.rb index 21baa918ffe8e..d190c984490b3 100644 --- a/shared/test/test_sources.rb +++ b/shared/test/test_sources.rb @@ -539,7 +539,7 @@ def test_remix_source_file delete_all_animation_versions(animation_filename_1) delete_all_animation_versions(animation_filename_2) - delete_all_versions(CDO.sources_s3_bucket, "sources_test/1/2/#{MAIN_JSON}") + delete_all_versions(CDO.sources_s3_bucket, "#{CDO.sources_s3_directory}/1/2/#{MAIN_JSON}") delete_all_versions(CDO.animations_s3_bucket, "animations_test/1/2/#{animation_filename_1}") delete_all_versions(CDO.animations_s3_bucket, "animations_test/1/2/#{animation_filename_2}") end @@ -583,7 +583,7 @@ def test_remix_source_file_with_library_animations # Clear original and remixed buckets delete_all_source_versions(MAIN_JSON) - delete_all_versions(CDO.sources_s3_bucket, "sources_test/1/2/#{MAIN_JSON}") + delete_all_versions(CDO.sources_s3_bucket, "#{CDO.sources_s3_directory}/1/2/#{MAIN_JSON}") end def test_remix_not_main @@ -610,7 +610,7 @@ def test_remix_not_main # Clear original and remixed buckets delete_all_source_versions('test.json') - delete_all_versions(CDO.sources_s3_bucket, "sources_test/1/2/test.json") + delete_all_versions(CDO.sources_s3_bucket, "#{CDO.sources_s3_directory}/1/2/test.json") end def test_remix_no_animations @@ -642,7 +642,7 @@ def test_remix_no_animations # Clear original and remixed buckets delete_all_source_versions(MAIN_JSON) - delete_all_versions(CDO.sources_s3_bucket, "sources_test/1/2/#{MAIN_JSON}") + delete_all_versions(CDO.sources_s3_bucket, "#{CDO.sources_s3_directory}/1/2/#{MAIN_JSON}") end def test_remove_under_13_comments @@ -818,7 +818,7 @@ def restore_main_json(version_id) end def delete_all_source_versions(filename) - delete_all_versions(CDO.sources_s3_bucket, "sources_test/1/1/#{filename}") + delete_all_versions(CDO.sources_s3_bucket, "#{CDO.sources_s3_directory}/1/1/#{filename}") end def delete_all_animation_versions(filename) From 97a7dda51ac00804f5bf413d146014ce5453d01e Mon Sep 17 00:00:00 2001 From: Maddie Kasula Date: Tue, 3 Jul 2018 10:52:10 -0700 Subject: [PATCH 22/89] Refactor connect/disconnect into ManageLinkedAccountsController --- .../lib/ui/accounts/ManageLinkedAccounts.jsx | 19 ++++++--- .../ManageLinkedAccountsController.js | 39 +++++++++++++++++-- 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx b/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx index ad10c4514eec0..2a116d00c26b3 100644 --- a/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx +++ b/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx @@ -2,7 +2,6 @@ import React, {PropTypes} from 'react'; import ReactTooltip from 'react-tooltip'; import _ from 'lodash'; import i18n from '@cdo/locale'; -import {navigateToHref} from '@cdo/apps/utils'; import color from '@cdo/apps/util/color'; import {tableLayoutStyles} from "@cdo/apps/templates/tables/tableConstants"; import BootstrapButton from './BootstrapButton'; @@ -19,10 +18,13 @@ export default class ManageLinkedAccounts extends React.Component { static propTypes = { userType: PropTypes.string.isRequired, authenticationOptions: PropTypes.arrayOf(PropTypes.shape({ + id: PropTypes.number.isRequired, credential_type: PropTypes.string.isRequired, email: PropTypes.string, hashed_email: PropTypes.string, - })), + })).isRequired, + connect: PropTypes.func.isRequired, + disconnect: PropTypes.func.isRequired, }; getProviderConnection = (provider) => { @@ -33,7 +35,6 @@ export default class ManageLinkedAccounts extends React.Component { getEmailForProvider = (provider) => { const match = this.getProviderConnection(provider); - if (match) { if (this.props.userType === 'student') { return ENCRYPTED; @@ -43,13 +44,19 @@ export default class ManageLinkedAccounts extends React.Component { }; toggleProviderConnection = (provider) => { - if (this.getProviderConnection(provider)) { - // TODO: (madelynkasula) disconnect from provider + const connection = this.getProviderConnection(provider); + if (connection) { + this.disconnect(connection.id); } else { - navigateToHref(`/users/auth/${provider}/connect`); + this.props.connect(provider); } }; + disconnect = (authOptionId) => { + // TODO: handle errors + this.props.disconnect(authOptionId).then(); + }; + render() { return (
diff --git a/apps/src/lib/ui/accounts/ManageLinkedAccountsController.js b/apps/src/lib/ui/accounts/ManageLinkedAccountsController.js index 4a616686e9878..57f3d4bbd95a0 100644 --- a/apps/src/lib/ui/accounts/ManageLinkedAccountsController.js +++ b/apps/src/lib/ui/accounts/ManageLinkedAccountsController.js @@ -1,15 +1,46 @@ import React from 'react'; import ReactDOM from 'react-dom'; +import $ from 'jquery'; +import {navigateToHref} from '@cdo/apps/utils'; import ManageLinkedAccounts from './ManageLinkedAccounts'; export default class ManageLinkedAccountsController { constructor(mountPoint, userType, authenticationOptions) { + this.mountPoint = mountPoint; + this.userType = userType; + this.authenticationOptions = authenticationOptions; + this.renderManageLinkedAccounts(); + } + + renderManageLinkedAccounts = () => { ReactDOM.render( , - mountPoint + this.mountPoint ); - } + }; + + connect = (provider) => { + navigateToHref(`/users/auth/${provider}/connect`); + }; + + disconnect = (authOptionId) => { + return new Promise((resolve, reject) => { + $.ajax({ + url: `/users/auth/${authOptionId}/disconnect`, + method: 'DELETE' + }).done(result => { + this.authenticationOptions = this.authenticationOptions.filter(option => option.id !== authOptionId); + this.renderManageLinkedAccounts(); + resolve(); + }).fail((jqXhr, status) => { + // TODO: handle errors + reject(); + }); + }); + }; } From 3df6c2524dfa7bb0e414e71e6d9133ea6cca1a4e Mon Sep 17 00:00:00 2001 From: Maddie Kasula Date: Tue, 3 Jul 2018 12:29:44 -0700 Subject: [PATCH 23/89] Add set width for table columns --- apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx b/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx index 2a116d00c26b3..98cca12eaf00f 100644 --- a/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx +++ b/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx @@ -176,6 +176,7 @@ const styles = { paddingLeft: GUTTER, paddingRight: GUTTER, fontWeight: 'normal', + width: tableLayoutStyles.table.width / 3, }, cell: { ...tableLayoutStyles.cell, From 25039a4f76867b55bd49ae5d2fb0e1470ecd1a08 Mon Sep 17 00:00:00 2001 From: Maddie Kasula Date: Tue, 3 Jul 2018 12:59:30 -0700 Subject: [PATCH 24/89] Handle errors, rename for clarity --- .../lib/ui/accounts/ManageLinkedAccounts.jsx | 34 +++++++++++-------- .../ManageLinkedAccountsController.js | 11 ++++-- 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx b/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx index 98cca12eaf00f..75dfc30aaa246 100644 --- a/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx +++ b/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx @@ -27,34 +27,38 @@ export default class ManageLinkedAccounts extends React.Component { disconnect: PropTypes.func.isRequired, }; - getProviderConnection = (provider) => { - return this.props.authenticationOptions.find((option) => { + getAuthenticationOption = (provider) => { + return this.props.authenticationOptions.find(option => { return option.credential_type === provider; }); }; getEmailForProvider = (provider) => { - const match = this.getProviderConnection(provider); - if (match) { + const authOption = this.getAuthenticationOption(provider); + if (authOption) { if (this.props.userType === 'student') { return ENCRYPTED; } - return match.email; + return authOption.email; } }; - toggleProviderConnection = (provider) => { - const connection = this.getProviderConnection(provider); - if (connection) { - this.disconnect(connection.id); + toggleProvider = (provider) => { + const authOption = this.getAuthenticationOption(provider); + if (authOption) { + this.disconnect(authOption.id); } else { this.props.connect(provider); } }; disconnect = (authOptionId) => { - // TODO: handle errors - this.props.disconnect(authOptionId).then(); + this.props.disconnect(authOptionId).then(_, this.onFailure); + }; + + onFailure = (error) => { + // TODO: (madelynkasula) display error to user + console.log(error.message); }; render() { @@ -75,25 +79,25 @@ export default class ManageLinkedAccounts extends React.Component { type={OAUTH_PROVIDERS.GOOGLE} displayName={i18n.manageLinkedAccounts_google_oauth2()} email={this.getEmailForProvider(OAUTH_PROVIDERS.GOOGLE)} - onClick={() => this.toggleProviderConnection(OAUTH_PROVIDERS.GOOGLE)} + onClick={() => this.toggleProvider(OAUTH_PROVIDERS.GOOGLE)} /> this.toggleProviderConnection(OAUTH_PROVIDERS.MICROSOFT)} + onClick={() => this.toggleProvider(OAUTH_PROVIDERS.MICROSOFT)} /> this.toggleProviderConnection(OAUTH_PROVIDERS.CLEVER)} + onClick={() => this.toggleProvider(OAUTH_PROVIDERS.CLEVER)} /> this.toggleProviderConnection(OAUTH_PROVIDERS.FACEBOOK)} + onClick={() => this.toggleProvider(OAUTH_PROVIDERS.FACEBOOK)} /> diff --git a/apps/src/lib/ui/accounts/ManageLinkedAccountsController.js b/apps/src/lib/ui/accounts/ManageLinkedAccountsController.js index 57f3d4bbd95a0..ded0cbbe3ed13 100644 --- a/apps/src/lib/ui/accounts/ManageLinkedAccountsController.js +++ b/apps/src/lib/ui/accounts/ManageLinkedAccountsController.js @@ -37,9 +37,14 @@ export default class ManageLinkedAccountsController { this.authenticationOptions = this.authenticationOptions.filter(option => option.id !== authOptionId); this.renderManageLinkedAccounts(); resolve(); - }).fail((jqXhr, status) => { - // TODO: handle errors - reject(); + }).fail((jqXhr, _) => { + let error; + if (jqXhr.responseText) { + error = new Error(jqXhr.responseText); + } else { + error = new Error('Unexpected failure: ' + jqXhr.status); + } + reject(error); }); }); }; From 8ce51b8122eead437d8cf0b1845fcc8e2ff9d0f2 Mon Sep 17 00:00:00 2001 From: Maddie Kasula Date: Tue, 3 Jul 2018 14:41:17 -0700 Subject: [PATCH 25/89] Add ManageLinkedAccountsController tests --- .../ManageLinkedAccountsControllerTest.js | 114 ++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 apps/test/unit/lib/ui/accounts/ManageLinkedAccountsControllerTest.js diff --git a/apps/test/unit/lib/ui/accounts/ManageLinkedAccountsControllerTest.js b/apps/test/unit/lib/ui/accounts/ManageLinkedAccountsControllerTest.js new file mode 100644 index 0000000000000..f7bf7be759615 --- /dev/null +++ b/apps/test/unit/lib/ui/accounts/ManageLinkedAccountsControllerTest.js @@ -0,0 +1,114 @@ +import ReactDOM from 'react-dom'; +import sinon from 'sinon'; +import {expect, assert} from '../../../../util/configuredChai'; +import ManageLinkedAccountsController from '@cdo/apps/lib/ui/accounts/ManageLinkedAccountsController'; +import * as utils from '@cdo/apps/utils'; + +const mockAuthenticationOptions = [ + {id: 3, credential_type: 'clever'}, + {id: 1, credential_type: 'google_oauth2'}, + {id: 2, credential_type: 'facebook'}, +]; + +describe('ManageLinkedAccountsController', () => { + let controller, mockMountPoint, userType; + + beforeEach(() => { + mockMountPoint = document.createElement('div'); + document.body.appendChild(mockMountPoint); + userType = 'teacher'; + const authenticationOptions = []; + + controller = new ManageLinkedAccountsController(mockMountPoint, userType, authenticationOptions); + + sinon.spy(ReactDOM, 'render'); + }); + + afterEach(() => { + ReactDOM.render.restore(); + document.body.removeChild(mockMountPoint); + }); + + describe('renderManageLinkedAccounts', () => { + it('renders ManageLinkedAccounts', () => { + expect(ReactDOM.render).not.to.have.been.called; + controller.renderManageLinkedAccounts(); + expect(ReactDOM.render).to.have.been.calledOnce; + }); + }); + + describe('connect', () => { + it('navigates to provider connection endpoint', () => { + const navigateToHrefStub = sinon.stub(utils, 'navigateToHref'); + controller.connect('google_oauth2'); + let arg = navigateToHrefStub.getCall(0).args[0]; + expect(navigateToHrefStub).to.have.been.calledOnce; + expect(arg).to.equal('/users/auth/google_oauth2/connect'); + }); + }); + + describe('disconnect', () => { + let server; + const authOptionToBeDeleted = mockAuthenticationOptions[1]; + const authOptionId = authOptionToBeDeleted.id; + + beforeEach(() => { + server = sinon.fakeServer.create(); + }); + + describe('onSuccess', () => { + beforeEach(() => { + controller = new ManageLinkedAccountsController(mockMountPoint, userType, mockAuthenticationOptions); + + server.respondWith( + 'DELETE', + `/users/auth/${authOptionId}/disconnect`, + [204, {"Content-Type": "application/json"}, ""] + ); + }); + + it('removes the disconnected authentication option from the list', () => { + const promise = controller.disconnect(authOptionId); + server.respond(); + promise.then(() => { + expect(controller.authenticationOptions).to.not.include(authOptionToBeDeleted); + }); + }); + + it('calls renderManageLinkedAccounts', async () => { + const renderStub = sinon.stub(controller, 'renderManageLinkedAccounts'); + + expect(renderStub).to.not.have.been.called; + const promise = controller.disconnect(authOptionId); + server.respond(); + promise.then(() => { + expect(renderStub).to.have.been.calledOnce; + }); + }); + }); + + describe('onFailure', () => { + it('rejects with server response text if present', () => { + server.respondWith( + 'DELETE', + `/users/auth/${authOptionId}/disconnect`, + [400, {"Content-Type": "application/json"}, "Oh no!"] + ); + const promise = controller.disconnect(authOptionId); + server.respond(); + assert.isRejected(promise, Error, "Oh no!"); + }); + + it('rejects with default error if server response not present', () => { + server.respondWith( + 'DELETE', + `/users/auth/${authOptionId}/disconnect`, + [400, {"Content-Type": "application/json"}, ""] + ); + const promise = controller.disconnect(authOptionId); + server.respond(); + assert.isRejected(promise, Error, "Unexpected failure: 400"); + }); + }); + }); +}); From 5ca37356216f6c8f0655d23be91c7c486f7d0a80 Mon Sep 17 00:00:00 2001 From: Maddie Kasula Date: Tue, 3 Jul 2018 15:24:45 -0700 Subject: [PATCH 26/89] Add unit tests for ManageLinkedAccounts --- .../lib/ui/accounts/ManageLinkedAccounts.jsx | 8 +- .../ui/accounts/ManageLinkedAccountsTest.jsx | 107 ++++++++++++++++++ 2 files changed, 109 insertions(+), 6 deletions(-) create mode 100644 apps/test/unit/lib/ui/accounts/ManageLinkedAccountsTest.jsx diff --git a/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx b/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx index 75dfc30aaa246..4813f679e16e9 100644 --- a/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx +++ b/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx @@ -12,7 +12,7 @@ const OAUTH_PROVIDERS = { CLEVER: 'clever', MICROSOFT: 'windowslive', }; -const ENCRYPTED = `*** ${i18n.encrypted()} ***`; +export const ENCRYPTED = `*** ${i18n.encrypted()} ***`; export default class ManageLinkedAccounts extends React.Component { static propTypes = { @@ -46,16 +46,12 @@ export default class ManageLinkedAccounts extends React.Component { toggleProvider = (provider) => { const authOption = this.getAuthenticationOption(provider); if (authOption) { - this.disconnect(authOption.id); + this.props.disconnect(authOption.id).then(_, this.onFailure); } else { this.props.connect(provider); } }; - disconnect = (authOptionId) => { - this.props.disconnect(authOptionId).then(_, this.onFailure); - }; - onFailure = (error) => { // TODO: (madelynkasula) display error to user console.log(error.message); diff --git a/apps/test/unit/lib/ui/accounts/ManageLinkedAccountsTest.jsx b/apps/test/unit/lib/ui/accounts/ManageLinkedAccountsTest.jsx new file mode 100644 index 0000000000000..887ce3d251d99 --- /dev/null +++ b/apps/test/unit/lib/ui/accounts/ManageLinkedAccountsTest.jsx @@ -0,0 +1,107 @@ +import React from 'react'; +import {mount} from 'enzyme'; +import sinon from 'sinon'; +import {expect} from '../../../../util/configuredChai'; +import ManageLinkedAccounts, {ENCRYPTED} from '@cdo/apps/lib/ui/accounts/ManageLinkedAccounts'; + +const DEFAULT_PROPS = { + userType: 'student', + authenticationOptions: [], + connect: () => {}, + disconnect: () => {}, +}; + +describe('ManageLinkedAccounts', () => { + it('renders a table with oauth provider rows', () => { + const wrapper = mount( + + ); + expect(wrapper.find('table')).to.exist; + expect(wrapper.find('OauthConnection').at(0)).to.include.text('Google Account'); + expect(wrapper.find('OauthConnection').at(1)).to.include.text('Microsoft Account'); + expect(wrapper.find('OauthConnection').at(2)).to.include.text('Clever Account'); + expect(wrapper.find('OauthConnection').at(3)).to.include.text('Facebook Account'); + }); + + it('renders an empty message for unconnected authentication options', () => { + const wrapper = mount( + + ); + const googleEmailCell = wrapper.find('OauthConnection').at(0).find('td').at(1); + expect(googleEmailCell).to.have.text('Not Connected'); + }); + + it('does not render student email for authentication options', () => { + const authOptions = [ + { + id: 1, + credential_type: 'google_oauth2', + email: 'student@email.com' + } + ]; + const wrapper = mount( + + ); + const googleEmailCell = wrapper.find('OauthConnection').at(0).find('td').at(1); + expect(googleEmailCell).to.have.text(ENCRYPTED); + }); + + it('renders teacher email for authentication options', () => { + const authOptions = [ + { + id: 1, + credential_type: 'google_oauth2', + email: 'teacher@email.com' + } + ]; + const wrapper = mount( + + ); + const googleEmailCell = wrapper.find('OauthConnection').at(0).find('td').at(1); + expect(googleEmailCell).to.have.text('teacher@email.com'); + }); + + it('calls connect if authentication option is not connected', () => { + const connect = sinon.stub(); + const wrapper = mount( + + ); + wrapper.find('BootstrapButton').at(0).simulate('click'); + expect(connect).to.have.been.calledOnce; + }); + + it('calls disconnect if authentication option is connected', () => { + const authOptions = [ + { + id: 1, + credential_type: 'google_oauth2', + email: 'student@email.com' + } + ]; + const disconnect = sinon.stub().resolves(); + const wrapper = mount( + + ); + wrapper.find('BootstrapButton').at(0).simulate('click'); + expect(disconnect).to.have.been.calledOnce; + }); +}); From 3f077edf563c9686ef0153771fa11f6c6760b25c Mon Sep 17 00:00:00 2001 From: Mehal Shah Date: Tue, 26 Jun 2018 16:54:12 -0700 Subject: [PATCH 27/89] New way of rendering survey results in tables - Also take advantage of better sorting order for the questions --- .../single_choice_responses.jsx | 4 +- .../survey_results/text_responses.jsx | 3 +- .../results.jsx | 253 +++++------------- .../resultsTest.js | 65 +++-- .../pd/workshop_survey_results_helper.rb | 5 +- .../concerns/pd/jot_form_backed_form.rb | 3 +- dashboard/lib/pd/jot_form/constants.rb | 7 +- dashboard/lib/pd/jot_form/matrix_question.rb | 17 +- .../lib/pd/jot_form/question_with_options.rb | 16 +- dashboard/lib/pd/jot_form/scale_question.rb | 15 +- dashboard/lib/pd/jot_form/select_question.rb | 39 ++- .../workshop_survey_results_helper_test.rb | 36 ++- .../lib/pd/jot_form/form_questions_test.rb | 47 +++- .../lib/pd/jot_form/matrix_question_test.rb | 16 +- .../lib/pd/jot_form/scale_question_test.rb | 13 +- .../lib/pd/jot_form/select_question_test.rb | 43 +-- 16 files changed, 266 insertions(+), 316 deletions(-) diff --git a/apps/src/code-studio/pd/workshop_dashboard/components/survey_results/single_choice_responses.jsx b/apps/src/code-studio/pd/workshop_dashboard/components/survey_results/single_choice_responses.jsx index c00ecf9d3edf9..27367a533216e 100644 --- a/apps/src/code-studio/pd/workshop_dashboard/components/survey_results/single_choice_responses.jsx +++ b/apps/src/code-studio/pd/workshop_dashboard/components/survey_results/single_choice_responses.jsx @@ -45,10 +45,10 @@ export default class SingleChoiceResponses extends React.Component { } - {possibleAnswer} + {count} - {count} + {possibleAnswer} ); diff --git a/apps/src/code-studio/pd/workshop_dashboard/components/survey_results/text_responses.jsx b/apps/src/code-studio/pd/workshop_dashboard/components/survey_results/text_responses.jsx index 4e3c77634a9ec..fbdc8bad4fa0c 100644 --- a/apps/src/code-studio/pd/workshop_dashboard/components/survey_results/text_responses.jsx +++ b/apps/src/code-studio/pd/workshop_dashboard/components/survey_results/text_responses.jsx @@ -1,6 +1,7 @@ import React, {PropTypes} from 'react'; import {Well} from 'react-bootstrap'; import _ from 'lodash'; +import he from 'he'; export default class TextResponses extends React.Component { static propTypes = { @@ -64,7 +65,7 @@ export default class TextResponses extends React.Component { } renderBullet(text, key) { - const trimmedText = _.trim(text); + const trimmedText = _.trim(he.decode(text)); if (trimmedText) { return (
  • diff --git a/apps/src/code-studio/pd/workshop_dashboard/reports/local_summer_workshop_daily_survey/results.jsx b/apps/src/code-studio/pd/workshop_dashboard/reports/local_summer_workshop_daily_survey/results.jsx index 9fe8a411e9067..becd284a6080a 100644 --- a/apps/src/code-studio/pd/workshop_dashboard/reports/local_summer_workshop_daily_survey/results.jsx +++ b/apps/src/code-studio/pd/workshop_dashboard/reports/local_summer_workshop_daily_survey/results.jsx @@ -1,16 +1,8 @@ import React, {PropTypes} from 'react'; import {Tab, Tabs} from 'react-bootstrap'; -import he from 'he'; - -const styles = { - table: { - width: 'auto', - maxWidth: '50%' - }, - facilitatorResponseList: { - listStyle: 'circle' - } -}; +import SingleChoiceResponses from '../../components/survey_results/single_choice_responses'; +import TextResponses from '../../components/survey_results/text_responses'; +import _ from 'lodash'; export default class Results extends React.Component { static propTypes = { @@ -24,167 +16,76 @@ export default class Results extends React.Component { facilitatorIds: Object.keys(this.props.facilitators) }; - renderSessionResultsTable(session) { - return ( - - - - - - - - { - Object.entries(this.props.questions[session]['general']).map(([question_key, question_data], i) => { - if (['selectValue', 'none'].includes(question_data['answer_type'])) { - return ( - - - - ); - } - }) - } - -
    - This workshop
    - - { - this.computeAverageFromAnswerObject( - this.props.thisWorkshop[session]['general'][question_key], - question_data['max_value'] - ) - } -
    - ); - } - - renderSessionResultsFreeResponse(session) { - return ( -
    - { - Object.entries(this.props.questions[session]['general']).map(([question_key, question_data], i) => { - if (question_data['answer_type'] === 'text') { - return ( -
    - {question_data['text']} - { - this.props.thisWorkshop[session]['general'][question_key].map((answer, j) => ( -
  • - {he.decode(answer)} -
    • - )) - } -
    - ); - } - }) - } -
    - ); - } - - renderFacilitatorSpecificResultsTable(session) { - const hasTableResponses = Object.values(this.props.questions[session]['facilitator']).some((question) => { - return question['answer_type'] === 'selectValue'; - }); + /** + * Render results for either the facilitator specific or general questions + */ + renderResultsForSessionQuestionSection(section, questions, answers) { + return _.compact(Object.keys(questions).map((questionId, i) => { + let question = questions[questionId]; - if (hasTableResponses) { - return ( - - - - - ))} - - - - { - Object.entries(this.props.questions[session]['facilitator']).map(([question_key, question_data], i) => { - if (question_data['answer_type'] === 'selectValue') { - return ( - - - { - this.state.facilitatorIds.map((id, j) => ( - - )) - } - - ); - } - }) - } - -
    - {this.state.facilitatorIds.map((id, i) => ( - - {this.props.facilitators[id]} -
    - {question_data['text']} - - {he.decode(this.props.thisWorkshop[session]['facilitator'][question_key][id])} -
    - ); - } else { - return null; - } - } - - renderFacilitatorSpecificFreeResponses(session) { - return ( -
    - { - Object.entries(this.props.questions[session]['facilitator']).map(([question_key, question_data], i) => { - if (question_data['answer_type'] === 'text') { - return ( -
    - {question_data['text']} - { - this.state.facilitatorIds.map((id) => { - return this.renderFacilitatorSpecificBullets( - this.props.thisWorkshop[session]['facilitator'][question_key], - id - ); - }) - } -
    - ); - } - }) - } -
    - ); - } + if (!question || _.isEmpty(answers[questionId])) { + return null; + } - renderFacilitatorSpecificBullets(responses, facilitatorId) { - const hasResponses = responses && responses[facilitatorId]; - return ( -
  • - {this.props.facilitators[facilitatorId]} -
      - { - hasResponses && responses[facilitatorId].map((response, i) => ( -
    • - {he.decode(response)} -
      • - )) - } -
    -
    • - ); + if (question['answer_type'] === 'scale'|| question['answer_type'] === 'singleSelect') { + return ( + + ); + } else if (question['answer_type'] === 'text') { + return ( + + ); + } else { + // DO NOT CHECK IN. Debugging item right now + return ( +

    + {question['text']} + {question['answer_type']} +

    + ); + } + })); } - renderFacilitatorSpecificSection(session) { + renderResultsForSession(session) { return (
    -

    - Facilitator specific questions + General Questions

    - {this.renderFacilitatorSpecificResultsTable(session)} - {this.renderFacilitatorSpecificFreeResponses(session)} + { + this.renderResultsForSessionQuestionSection( + 'general', + this.props.questions[session]['general'], + this.props.thisWorkshop[session]['general'] + ) + } + { + !_.isEmpty(this.props.questions[session]['facilitator']) && ( +
    +

    + Facilitator Specific Questions +

    + { + this.renderResultsForSessionQuestionSection( + 'facilitator', + this.props.questions[session]['facilitator'], + this.props.thisWorkshop[session]['facilitator'] + ) + } +
    + ) + }
    ); } @@ -193,33 +94,11 @@ export default class Results extends React.Component { return this.props.sessions.map((session, i) => (
    - {this.renderSessionResultsTable(session)} - {this.renderSessionResultsFreeResponse(session)} - { - this.props.thisWorkshop[session]['facilitator'] && this.renderFacilitatorSpecificSection(session) - } + {this.renderResultsForSession(session)}     )); } - computeAverageFromAnswerObject(answerHash, maxValue) { - let sum = 0; - Object.keys(answerHash).map((key) => { - if (Number(key) > 0) { - sum += key * answerHash[key]; - } - }); - - if (sum === 0) { - return ''; - } else { - let average = sum / Object.values(answerHash).reduce((sum, x) => { - return sum + x; - }); - return `${average.toFixed(2)} / ${maxValue}`; - } - } - render() { return ( diff --git a/apps/test/unit/code-studio/pd/workshop_dashboard/reports/local_summer_workshop_daily_survey/resultsTest.js b/apps/test/unit/code-studio/pd/workshop_dashboard/reports/local_summer_workshop_daily_survey/resultsTest.js index 008a286847b39..dba42a2853fc5 100644 --- a/apps/test/unit/code-studio/pd/workshop_dashboard/reports/local_summer_workshop_daily_survey/resultsTest.js +++ b/apps/test/unit/code-studio/pd/workshop_dashboard/reports/local_summer_workshop_daily_survey/resultsTest.js @@ -11,11 +11,23 @@ describe("Local Summer Workshop Daily Survey Results class", () => { 'Pre Workshop': { 'general': { 'q1': {text: 'Matrix header', answer_type: 'none'}, - 'q2': {text: 'Matrix 1', answer_type: 'selectValue', max_value: '5'}, - 'q3': {text: 'Matrix 2', answer_type: 'selectValue', max_value: '5'}, - 'q4': {text: 'Scale 1', answer_type: 'selectValue', max_value: '5'}, + 'q2': {text: 'Matrix header...Matrix 1', answer_type: 'singleSelect', options: ['Poor', 'Fair', 'Good', 'Great', 'Excellent']}, + 'q3': {text: 'Matrix header...Matrix 2', answer_type: 'singleSelect', options: ['Poor', 'Fair', 'Good', 'Great', 'Excellent']}, + 'q4': {text: 'Scale 1', answer_type: 'scale', max_value: '5', options: ['1', '2', '3', '4', '5']}, + 'f1': {text: 'General, Free Response 1', answer_type: 'text'}, + 'f2': {text: 'General, Free Response 2', answer_type: 'text'} + } + }, + 'Day 1': { + 'general': { + 'q1': {text: 'Day 1 Matrix header', answer_type: 'none'}, + 'q2': {text: 'Day 1 Matrix...Matrix 1', answer_type: 'singleSelect', options: ['Poor', 'Fair', 'Good', 'Great', 'Excellent']}, + 'q3': {text: 'Scale 2', answer_type: 'scale', max_value: '5', options: ['1', '2', '3', '4', '5']}, 'f1': {text: 'Day 1, Free Response 1', answer_type: 'text'}, - 'f2': {text: 'Day 1, Free Response 2', answer_type: 'text'} + }, + 'facilitator': { + 'q1': {text: 'Day 1 Facilitator question 1', answer_type: 'text'}, + 'q2': {text: 'Day 1 Facilitator question 2', answer_type: 'text'} } } }} @@ -30,26 +42,45 @@ describe("Local Summer Workshop Daily Survey Results class", () => { 'f2': ['d', 'e', 'f'] }, 'response_count': 10 + }, + 'Day 1': { + 'general': { + 'q1': {}, + 'q2': {1: 2, 4: 5}, + 'q3': {2: 3, 3: 3, 4: 1}, + 'f1': ['g', 'h', 'i'] + }, + 'facilitator': { + 'q1': { + 1: ['j', 'k', 'l'], + 2: ['m', 'n', 'o'] + }, + 'q2': { + 1: ['p', 'q', 'r'], + 2: ['s', 't', 'u'] + } + }, + response_count: 12 } }} - sessions={['Pre Workshop']} + sessions={['Pre Workshop', 'Day 1']} facilitators={{ 1: 'Facilitator 1', 2: 'Facilitator 2' }} /> ); - expect(results.find('Tab')).to.have.length(1); - expect(results.find('table')).to.have.length(1); - expect(results.find('td').map((x) => x.text())).to.deep.equal( - [ - 'Matrix header', '', - 'Matrix 1', '3.86 / 5', - 'Matrix 2', '2.33 / 5', - 'Scale 1', '3.60 / 5', - ] - ); - expect(results.find('.well')).to.have.length(2); // 2 general responses - expect(results.find('.well li').map((x) => x.text())).to.deep.equal('abcdef'.split('')); + expect(results.find('Tab')).to.have.length(2); + let firstTab = results.find('Tab').first(); + let lastTab = results.find('Tab').last(); + + expect(firstTab.find('SingleChoiceResponses')).to.have.length(3); + expect(firstTab.find('TextResponses')).to.have.length(2); + + expect(lastTab.find('SingleChoiceResponses')).to.have.length(2); + expect(lastTab.find('TextResponses')).to.have.length(3); + + expect(firstTab.find('h3').map((x) => x.text())).to.deep.equal(['General Questions']); + expect(lastTab.find('h3').map((x) => x.text())).to.deep.equal(['General Questions', 'Facilitator Specific Questions']); }); }); diff --git a/dashboard/app/helpers/pd/workshop_survey_results_helper.rb b/dashboard/app/helpers/pd/workshop_survey_results_helper.rb index fb25003c49332..bcd014aad81ca 100644 --- a/dashboard/app/helpers/pd/workshop_survey_results_helper.rb +++ b/dashboard/app/helpers/pd/workshop_survey_results_helper.rb @@ -183,6 +183,7 @@ def generate_workshops_survey_summary(workshop, questions) surveys = get_surveys_for_workshops(workshop) workshop_summary = {} + facilitator_map = Hash[*workshop.facilitators.pluck(:id, :name).flatten] # Each session has a general response section. # Some also have a facilitator response section @@ -211,7 +212,7 @@ def generate_workshops_survey_summary(workshop, questions) if current_user&.facilitator? facilitator_responses.slice! current_user.id end - session_summary[:facilitator][q_key] = facilitator_responses + session_summary[:facilitator][q_key] = facilitator_responses.transform_keys {|k| facilitator_map[k]} else # Otherwise, we just want a list of all responses sum = surveys_for_session[response_section].map {|survey| survey[q_key]}.reduce([], :append).compact @@ -239,7 +240,7 @@ def generate_workshops_survey_summary(workshop, questions) facilitator_response_sums.each do |facilitator_id, response_sums| facilitator_response_averages[facilitator_id] = (response_sums[:sum] / response_sums[:responses].to_f).round(2) end - session_summary[:facilitator][q_key] = facilitator_response_averages + session_summary[:facilitator][q_key] = facilitator_response_averages.transform_keys {|k| facilitator_map[k]} else # For non facilitator specific responses, just return a frequency map with # nulls removed diff --git a/dashboard/app/models/concerns/pd/jot_form_backed_form.rb b/dashboard/app/models/concerns/pd/jot_form_backed_form.rb index 65e16d2a7e0db..6a03601cda929 100644 --- a/dashboard/app/models/concerns/pd/jot_form_backed_form.rb +++ b/dashboard/app/models/concerns/pd/jot_form_backed_form.rb @@ -128,7 +128,8 @@ def skip_submission?(form_id, processed_answers) # Skip other environments, and anything without an environment value. # Only keep this environment. - return true if environment != Rails.env + #return true if environment != Rails.env + return true unless environment # Is it a duplicate? These will be prevented in the future, but for now log and skip # TODO(Andrew): prevent duplicates and remove this code. diff --git a/dashboard/lib/pd/jot_form/constants.rb b/dashboard/lib/pd/jot_form/constants.rb index 6eb48567ba835..658385cf7d601 100644 --- a/dashboard/lib/pd/jot_form/constants.rb +++ b/dashboard/lib/pd/jot_form/constants.rb @@ -25,12 +25,9 @@ module Constants ANSWER_TYPES = [ ANSWER_TEXT = 'text'.freeze, - # We can convert the text response to a numeric value for certain single select controls - # (radio, dropdown, scale, part of a matrix). - ANSWER_SELECT_VALUE = 'selectValue'.freeze, - ANSWER_SELECT_TEXT = 'selectText'.freeze, + ANSWER_SCALE = 'scale'.freeze, - # Multi-select is always text + ANSWER_SINGLE_SELECT = 'singleSelect'.freeze, ANSWER_MULTI_SELECT = 'multiSelect'.freeze, # No answer, just question metadata, e.g. matrix heading diff --git a/dashboard/lib/pd/jot_form/matrix_question.rb b/dashboard/lib/pd/jot_form/matrix_question.rb index c36973f2cb818..6eea23bb25306 100644 --- a/dashboard/lib/pd/jot_form/matrix_question.rb +++ b/dashboard/lib/pd/jot_form/matrix_question.rb @@ -49,15 +49,15 @@ def get_value(answer) raise "Unable to process matrix answer: #{answer}" unless answer.is_a? Hash # Matrix answer is a Hash of sub_question => string_answer + # Validate each answer and convert each key to sub_question_index answer.reject {|_, v| v.blank?}.map do |sub_question, sub_answer| sub_question_index = sub_questions.index(sub_question) raise "Unable to find sub-question '#{sub_question}' in matrix question #{id}" unless sub_question_index - sub_answer_index = options.index(sub_answer) - raise "Unable to find '#{sub_answer}' in the options for matrix question #{id}" unless sub_answer_index + raise "Unable to find '#{sub_answer}' in the options for matrix question #{id}" unless options.include? sub_answer # Return a 1-based value - [sub_question_index, sub_answer_index + 1] + [sub_question_index, sub_answer] end.to_h end @@ -68,7 +68,11 @@ def summarize { text: "#{text} #{sub_question}", answer_type: ANSWER_SELECT_VALUE, - max_value: options.length + parent: name, + max_value: options.length, + text: "#{text} #{sub_question}", + answer_type: answer_type, + options: options } ] end.to_h @@ -82,6 +86,11 @@ def process_answer(answer) def generate_sub_question_key(sub_question_index) "#{name}_#{sub_question_index}" end + + # @override + def answer_type + ANSWER_SINGLE_SELECT + end end end end diff --git a/dashboard/lib/pd/jot_form/question_with_options.rb b/dashboard/lib/pd/jot_form/question_with_options.rb index c836166742b4d..a747945e07764 100644 --- a/dashboard/lib/pd/jot_form/question_with_options.rb +++ b/dashboard/lib/pd/jot_form/question_with_options.rb @@ -9,8 +9,22 @@ def to_h ) end + # @override def answer_type - ANSWER_SELECT_VALUE + ANSWER_SINGLE_SELECT + end + + def multi_select? + answer_type == ANSWER_MULTI_SELECT + end + + def single_select? + answer_type == ANSWER_SINGLE_SELECT + end + + # @override + def type_specific_summary + {options: options} end end end diff --git a/dashboard/lib/pd/jot_form/scale_question.rb b/dashboard/lib/pd/jot_form/scale_question.rb index 129f3507f24b7..6f41fe0f98ca1 100644 --- a/dashboard/lib/pd/jot_form/scale_question.rb +++ b/dashboard/lib/pd/jot_form/scale_question.rb @@ -39,7 +39,20 @@ def get_value(answer) # @override def type_specific_summary - {max_value: values.last} + augmented_options = values.map(&:to_s) + augmented_options[0] = "#{augmented_options[0]} - #{options.first}" + augmented_options[-1] = "#{augmented_options[-1]} - #{options.last}" + + { + min_value: values.first, + max_value: values.last, + options: augmented_options + } + end + + # @override + def answer_type + ANSWER_SCALE end end end diff --git a/dashboard/lib/pd/jot_form/select_question.rb b/dashboard/lib/pd/jot_form/select_question.rb index 56cca95e26acc..fbec4c458f7b3 100644 --- a/dashboard/lib/pd/jot_form/select_question.rb +++ b/dashboard/lib/pd/jot_form/select_question.rb @@ -16,7 +16,7 @@ def self.supported_types attr_accessor( :allow_other, :other_text, - :preserve_text + :preserve_text # kept for backward compatibility ) def self.from_jotform_question(jotform_question) @@ -33,20 +33,23 @@ def to_h super.merge( allow_other: allow_other, other_text: other_text, - preserve_text: preserve_text ) end - def answer_type - if type == TYPE_CHECKBOX - ANSWER_MULTI_SELECT - elsif allow_other || preserve_text - ANSWER_SELECT_TEXT - else - # We can only assume a numeric value for single-select (dropdown/radio), - # without an "other" option, and not explicitly set to preserve text. - ANSWER_SELECT_VALUE + # @override + def ensure_valid_answer(answer) + if answer.is_a? Array + answer.each {|sub_answer| ensure_valid_answer(sub_answer)} + elsif !options.include? answer + raise "Unrecognized answer '#{answer}' for question #{id} (Options: #{options.join(',')})" end + + answer + end + + # @override + def answer_type + type == TYPE_CHECKBOX ? ANSWER_MULTI_SELECT : ANSWER_SINGLE_SELECT end def get_value(answer) @@ -59,23 +62,15 @@ def get_value(answer) values_with_other = answer.map {|k, v| k == OTHER_ANSWER_KEY ? v.presence || other_text : v} # It might be a single item or an array - return answer_type == ANSWER_MULTI_SELECT ? values_with_other : values_with_other.first - end - - return answer unless answer_type == ANSWER_SELECT_VALUE - - index = options.index(answer) - unless index - raise "Unrecognized answer '#{answer}' for question #{id} (Options: #{options.to_csv.strip})" + return multi_select? ? values_with_other : values_with_other.first end - # Return a 1-based value - index + 1 + ensure_valid_answer answer end # @override def type_specific_summary - answer_type == ANSWER_SELECT_VALUE ? {max_value: options.length} : {} + {options: options} end end end diff --git a/dashboard/test/helpers/workshop_survey_results_helper_test.rb b/dashboard/test/helpers/workshop_survey_results_helper_test.rb index 1abb201537a91..2c283bd56b83c 100644 --- a/dashboard/test/helpers/workshop_survey_results_helper_test.rb +++ b/dashboard/test/helpers/workshop_survey_results_helper_test.rb @@ -48,7 +48,7 @@ class Pd::WorkshopSurveyResultsHelperTest < ActionView::TestCase id: 1, name: 'sampleDailyScale', text: 'How was your day?', - options: %w(Poor Fair Good Great Excellent), + options: %w(Poor Excellent), values: (1..5).to_a, type: TYPE_SCALE ) @@ -102,8 +102,10 @@ class Pd::WorkshopSurveyResultsHelperTest < ActionView::TestCase general: { 'sampleDailyScale' => { text: 'How was your day?', - answer_type: ANSWER_SELECT_VALUE, - max_value: 5 + answer_type: ANSWER_SCALE, + min_value: 1, + max_value: 5, + options: ['1 - Poor', '2', '3', '4', '5 - Excellent'] }, }, facilitator: { @@ -119,18 +121,24 @@ class Pd::WorkshopSurveyResultsHelperTest < ActionView::TestCase general: { 'sampleMatrix_0' => { text: 'How do you feel about these statements? I am excited for CS Principles', - answer_type: ANSWER_SELECT_VALUE, - max_value: 5 + answer_type: ANSWER_SINGLE_SELECT, + options: %w(Strongly\ Agree Agree Neutral Disagree Strongly\ Disagree), + max_value: 5, + parent: 'sampleMatrix' }, 'sampleMatrix_1' => { text: 'How do you feel about these statements? I am prepared for CS Principles', - answer_type: ANSWER_SELECT_VALUE, - max_value: 5 + answer_type: ANSWER_SINGLE_SELECT, + options: %w(Strongly\ Agree Agree Neutral Disagree Strongly\ Disagree), + max_value: 5, + parent: 'sampleMatrix' }, 'sampleScale' => { text: 'Do you like CS Principles?', - answer_type: ANSWER_SELECT_VALUE, - max_value: 5 + answer_type: ANSWER_SCALE, + min_value: 1, + max_value: 5, + options: ['1 - Strongly Agree', '2', '3', '4', '5 - Strongly Disagree'] }, 'sampleText' => { text: 'Write some thoughts here', @@ -382,21 +390,21 @@ class Pd::WorkshopSurveyResultsHelperTest < ActionView::TestCase assert_equal( { 'Pre Workshop' => { + response_count: 3, general: { 'sampleMatrix_0' => { - 1 => 2, - 4 => 1 + 'Strongly Agree' => 2, + 'Disagree' => 1 }, 'sampleMatrix_1' => { - 2 => 3, + 'Agree' => 3, }, 'sampleScale' => { 2 => 1, 4 => 1 }, 'sampleText' => ['Here are my thoughts', 'More thoughts'] - }, - response_count: 3 + } }, 'Day 1' => daily_expected_results, 'Day 2' => daily_expected_results, diff --git a/dashboard/test/lib/pd/jot_form/form_questions_test.rb b/dashboard/test/lib/pd/jot_form/form_questions_test.rb index 9ad40b9060398..e8c384bffd5a0 100644 --- a/dashboard/test/lib/pd/jot_form/form_questions_test.rb +++ b/dashboard/test/lib/pd/jot_form/form_questions_test.rb @@ -11,11 +11,13 @@ class FormQuestionsTest < ActiveSupport::TestCase @questions = [ TextQuestion.new( id: 1, + order: 1, name: 'text', text: 'text label' ), SelectQuestion.new( id: 2, + order: 2, type: TYPE_RADIO, name: 'singleSelect', text: 'single select label', @@ -23,6 +25,7 @@ class FormQuestionsTest < ActiveSupport::TestCase ), SelectQuestion.new( id: 3, + order: 3, name: 'singleSelectWithOther', type: TYPE_RADIO, text: 'single select with other label', @@ -32,6 +35,7 @@ class FormQuestionsTest < ActiveSupport::TestCase ), SelectQuestion.new( id: 4, + order: 4, type: TYPE_CHECKBOX, name: 'multiSelect', text: 'multi select label', @@ -39,6 +43,7 @@ class FormQuestionsTest < ActiveSupport::TestCase ), SelectQuestion.new( id: 5, + order: 5, type: TYPE_CHECKBOX, name: 'multiSelectWithOther', text: 'multi select with other label', @@ -48,6 +53,7 @@ class FormQuestionsTest < ActiveSupport::TestCase ), ScaleQuestion.new( id: 6, + order: 6, name: 'scale', text: 'scale label', options: %w(From To), @@ -55,6 +61,7 @@ class FormQuestionsTest < ActiveSupport::TestCase ), MatrixQuestion.new( id: 7, + order: 7, name: 'matrix', text: 'How much do you agree or disagree with the following statements about this workshop?', options: %w(Disagree Neutral Agree), @@ -66,6 +73,7 @@ class FormQuestionsTest < ActiveSupport::TestCase ), TextQuestion.new( id: 8, + order: 8, name: 'hidden_text', text: 'This should be hidden', hidden: true @@ -98,39 +106,50 @@ class FormQuestionsTest < ActiveSupport::TestCase }, 'singleSelect' => { text: 'single select label', - answer_type: ANSWER_SELECT_VALUE, - max_value: 3 + answer_type: ANSWER_SINGLE_SELECT, + options: %w(One Two Three) }, 'singleSelectWithOther' => { text: 'single select with other label', - answer_type: ANSWER_SELECT_TEXT + answer_type: ANSWER_SINGLE_SELECT, + options: %w(One Two Three) }, 'multiSelect' => { text: 'multi select label', - answer_type: ANSWER_MULTI_SELECT + answer_type: ANSWER_MULTI_SELECT, + options: %w(One Two Three) }, 'multiSelectWithOther' => { text: 'multi select with other label', - answer_type: ANSWER_MULTI_SELECT + answer_type: ANSWER_MULTI_SELECT, + options: %w(One Two Three) }, 'scale' => { text: 'scale label', - answer_type: ANSWER_SELECT_VALUE, - max_value: 3 + answer_type: ANSWER_SCALE, + min_value: 1, + max_value: 3, + options: ['1 - From', '2', '3 - To'] }, 'matrix_0' => { text: 'How much do you agree or disagree with the following statements about this workshop? I learned something', - answer_type: ANSWER_SELECT_VALUE, + answer_type: ANSWER_SINGLE_SELECT, + options: %w(Disagree Neutral Agree), + parent: 'matrix', max_value: 3 }, 'matrix_1' => { text: 'How much do you agree or disagree with the following statements about this workshop? It was a good use of time', - answer_type: ANSWER_SELECT_VALUE, + answer_type: ANSWER_SINGLE_SELECT, + options: %w(Disagree Neutral Agree), + parent: 'matrix', max_value: 3 }, 'matrix_2' => { text: 'How much do you agree or disagree with the following statements about this workshop? I enjoyed it', - answer_type: ANSWER_SELECT_VALUE, + answer_type: ANSWER_SINGLE_SELECT, + options: %w(Disagree Neutral Agree), + parent: 'matrix', max_value: 3 } } @@ -141,14 +160,14 @@ class FormQuestionsTest < ActiveSupport::TestCase test 'process_answers' do expected_processed_answers = { 'text' => 'this is my text answer', - 'singleSelect' => 2, + 'singleSelect' => 'Two', 'singleSelectWithOther' => 'my other reason', 'multiSelect' => %w(Two Three), 'multiSelectWithOther' => ['Two', 'my other reason'], 'scale' => 2, - 'matrix_0' => 3, - 'matrix_1' => 2, - 'matrix_2' => 3 + 'matrix_0' => 'Agree', + 'matrix_1' => 'Neutral', + 'matrix_2' => 'Agree' } assert_equal expected_processed_answers, @form_questions.process_answers(@jotform_answers) diff --git a/dashboard/test/lib/pd/jot_form/matrix_question_test.rb b/dashboard/test/lib/pd/jot_form/matrix_question_test.rb index e917fffc87d39..d57ba556c5502 100644 --- a/dashboard/test/lib/pd/jot_form/matrix_question_test.rb +++ b/dashboard/test/lib/pd/jot_form/matrix_question_test.rb @@ -24,7 +24,7 @@ class MatrixQuestionTest < ActiveSupport::TestCase assert_equal 'sampleMatrix', question.name assert_equal 'This is a matrix label', question.text assert_equal 1, question.order - assert_equal ANSWER_SELECT_VALUE, question.answer_type + assert_equal ANSWER_SINGLE_SELECT, question.answer_type assert_equal ['Strongly Agree', 'Agree', 'Neutral', 'Disagree', 'Strongly Disagree'], question.options assert_equal ['Question 1', 'Question 2'], question.sub_questions end @@ -43,7 +43,7 @@ class MatrixQuestionTest < ActiveSupport::TestCase } assert_equal( - {0 => 2, 2 => 1}, + {0 => 'Neutral', 2 => 'Agree'}, question.get_value(answer) ) end @@ -81,12 +81,16 @@ class MatrixQuestionTest < ActiveSupport::TestCase expected_summary = { 'sampleMatrix_0' => { text: 'How much do you agree or disagree with the following statements about this workshop? I learned something', - answer_type: ANSWER_SELECT_VALUE, + answer_type: ANSWER_SINGLE_SELECT, + options: %w(Disagree Neutral Agree), + parent: 'sampleMatrix', max_value: 3 }, 'sampleMatrix_1' => { text: 'How much do you agree or disagree with the following statements about this workshop? It was a good use of time', - answer_type: ANSWER_SELECT_VALUE, + answer_type: ANSWER_SINGLE_SELECT, + options: %w(Disagree Neutral Agree), + parent: 'sampleMatrix', max_value: 3 } } @@ -112,8 +116,8 @@ class MatrixQuestionTest < ActiveSupport::TestCase assert_equal( { - 'sampleMatrix_0' => 3, - 'sampleMatrix_1' => 2 + 'sampleMatrix_0' => 'Agree', + 'sampleMatrix_1' => 'Neutral' }, question.process_answer(answer) ) diff --git a/dashboard/test/lib/pd/jot_form/scale_question_test.rb b/dashboard/test/lib/pd/jot_form/scale_question_test.rb index cbbadec020c81..a3444f273f0e2 100644 --- a/dashboard/test/lib/pd/jot_form/scale_question_test.rb +++ b/dashboard/test/lib/pd/jot_form/scale_question_test.rb @@ -26,7 +26,7 @@ class ScaleQuestionTest < ActiveSupport::TestCase assert_equal 'sampleScale', question.name assert_equal 'This is a scale label', question.text assert_equal 1, question.order - assert_equal ANSWER_SELECT_VALUE, question.answer_type + assert_equal ANSWER_SCALE, question.answer_type assert_equal [1, 2, 3, 4, 5], question.values assert_equal ['Strongly Agree', 'Strongly Disagree'], question.options end @@ -48,7 +48,7 @@ class ScaleQuestionTest < ActiveSupport::TestCase name: 'a name', text: 'label', order: 1, - options: %w(One Two Three), + options: %w(From To), values: [1, 2, 3] } @@ -61,14 +61,17 @@ class ScaleQuestionTest < ActiveSupport::TestCase id: 1, name: 'sampleScale', text: 'a label', - values: (1..5).to_a + values: (1..5).to_a, + options: %w(From To) ) expected_summary = { 'sampleScale' => { text: 'a label', - answer_type: ANSWER_SELECT_VALUE, - max_value: 5 + answer_type: ANSWER_SCALE, + min_value: 1, + max_value: 5, + options: ['1 - From', '2', '3', '4', '5 - To'] } } assert_equal expected_summary, question.summarize diff --git a/dashboard/test/lib/pd/jot_form/select_question_test.rb b/dashboard/test/lib/pd/jot_form/select_question_test.rb index a0797b278d21d..222c6d57b3ace 100644 --- a/dashboard/test/lib/pd/jot_form/select_question_test.rb +++ b/dashboard/test/lib/pd/jot_form/select_question_test.rb @@ -7,8 +7,8 @@ class SelectQuestionTest < ActiveSupport::TestCase include Constants { - TYPE_DROPDOWN => ANSWER_SELECT_VALUE, - TYPE_RADIO => ANSWER_SELECT_VALUE, + TYPE_DROPDOWN => ANSWER_SINGLE_SELECT, + TYPE_RADIO => ANSWER_SINGLE_SELECT, TYPE_CHECKBOX => ANSWER_MULTI_SELECT }.each do |type, expected_answer_type| test "parse jotform question data for #{type}" do @@ -39,29 +39,12 @@ class SelectQuestionTest < ActiveSupport::TestCase end end - test 'questions with preserve_text or an other option do not calculate answer values' do - { - TYPE_DROPDOWN => ANSWER_SELECT_TEXT, - TYPE_RADIO => ANSWER_SELECT_TEXT, - TYPE_CHECKBOX => ANSWER_MULTI_SELECT - }.each do |type, expected_answer_type| - assert_equal( - expected_answer_type, - SelectQuestion.new(type: type, preserve_text: true).answer_type - ) - assert_equal( - expected_answer_type, - SelectQuestion.new(type: type, allow_other: true).answer_type - ) - end - end - - test 'get_value for single selection returns the numeric value' do + test 'get_value for single selection returns the single value' do question = SelectQuestion.new(id: 1, type: TYPE_RADIO, options: %w(First Second Third)) - assert_equal 1, question.get_value('First') - assert_equal 2, question.get_value('Second') - assert_equal 3, question.get_value('Third') + assert_equal 'First', question.get_value('First') + assert_equal 'Second', question.get_value('Second') + assert_equal 'Third', question.get_value('Third') e = assert_raises do question.get_value('Invalid') @@ -76,13 +59,6 @@ class SelectQuestionTest < ActiveSupport::TestCase assert_equal %w(Second Third), question.get_value(%w(Second Third)) end - test 'get_value with preserve_text' do - question = SelectQuestion.new(id: 1, options: %w(First Second Third), preserve_text: true) - - assert_equal 'First', question.get_value('First') - assert_equal %w(Second Third), question.get_value(%w(Second Third)) - end - test 'get_value with other' do question = SelectQuestion.new( id: 1, @@ -108,8 +84,7 @@ class SelectQuestionTest < ActiveSupport::TestCase order: 1, options: %w(One Two Three), allow_other: true, - other_text: 'Other', - preserve_text: false + other_text: 'Other' } question = SelectQuestion.new(hash) @@ -128,8 +103,8 @@ class SelectQuestionTest < ActiveSupport::TestCase expected_summary = { 'sampleSelect' => { text: 'a label', - answer_type: ANSWER_SELECT_VALUE, - max_value: 3 + answer_type: ANSWER_SINGLE_SELECT, + options: %w(One Two Three) } } assert_equal expected_summary, question.summarize From f72174bbae1cf9b71fa814ab3a7cf4f3922c3b3a Mon Sep 17 00:00:00 2001 From: Elijah Hamovitz Date: Tue, 3 Jul 2018 17:05:24 -0700 Subject: [PATCH 28/89] Revert "Add 2018 courses to k5_course? helper" --- dashboard/app/models/script.rb | 100 +++++++----------- lib/cdo/script_constants.rb | 5 +- .../code.org/public/congrats/splat.haml | 12 ++- .../sites.v3/code.org/views/csf_congrats.haml | 14 --- 4 files changed, 54 insertions(+), 77 deletions(-) diff --git a/dashboard/app/models/script.rb b/dashboard/app/models/script.rb index a6db6a7af0315..e582843fec3f1 100644 --- a/dashboard/app/models/script.rb +++ b/dashboard/app/models/script.rb @@ -475,48 +475,6 @@ def minecraft? ScriptConstants.script_in_category?(:minecraft, name) end - def k5_draft_course? - ScriptConstants.script_in_category?(:csf2_draft, name) - end - - def csf_international? - ScriptConstants.script_in_category?(:csf_international, name) - end - - def k5_course? - ( - Script::CATEGORIES[:csf_international] + - Script::CATEGORIES[:csf] + - Script::CATEGORIES[:csf_2018] - ).include? name - end - - def csf? - k5_course? || twenty_hour? - end - - def cs_in_a? - name.match(Regexp.union('algebra', 'Algebra')) - end - - def k1? - [ - Script::COURSEA_DRAFT_NAME, - Script::COURSEB_DRAFT_NAME, - Script::COURSEA_NAME, - Script::COURSEB_NAME, - Script::COURSE1_NAME - ].include?(name) - end - - def beta? - Script.beta? name - end - - def self.beta?(name) - name == Script::EDIT_CODE_NAME || ScriptConstants.script_in_category?(:csf2_draft, name) - end - def get_script_level_by_id(script_level_id) script_levels.find {|sl| sl.id == script_level_id.to_i} end @@ -551,6 +509,24 @@ def get_bonus_script_levels(current_stage) @all_bonus_script_levels.select {|stage| stage[:stageNumber] <= current_stage.absolute_position} end + def beta? + Script.beta? name + end + + def self.beta?(name) + name == 'edit-code' || name == 'coursea-draft' || name == 'courseb-draft' || name == 'coursec-draft' || name == 'coursed-draft' || name == 'coursee-draft' || name == 'coursef-draft' + end + + def k1? + [ + Script::COURSEA_DRAFT_NAME, + Script::COURSEB_DRAFT_NAME, + Script::COURSEA_NAME, + Script::COURSEB_NAME, + Script::COURSE1_NAME + ].include?(name) + end + private def csf_tts_level? k5_course? end @@ -601,31 +577,37 @@ def banner_image end end + def k5_course? + %w(course1 course2 course3 course4 coursea-2017 courseb-2017 coursec-2017 coursed-2017 coursee-2017 coursef-2017 express-2017 pre-express-2017).include? name + end + + def k5_draft_course? + %w(coursea-draft courseb-draft coursec-draft coursed-draft coursee-draft coursef-draft).include? name + end + + def csf? + k5_course? || twenty_hour? + end + + def csf_international? + %w(course1 course2 course3 course4).include? name + end + + def cs_in_a? + name.match(Regexp.union('algebra', 'Algebra')) + end + def has_lesson_pdf? - return false if ScriptConstants.script_in_category?(:csf, name) || ScriptConstants.script_in_category?(:csf_2018, name) + return false if %w(coursea-2017 courseb-2017 coursec-2017 coursed-2017 coursee-2017 coursef-2017 express-2017 pre-express-2017).include?(name) has_lesson_plan? end def has_banner? # Temporarily remove Course A-F banner (wrong size) - Josh L. - return false if ScriptConstants.script_in_category?(:csf, name) || ScriptConstants.script_in_category?(:csf_2018, name) - - k5_course? || [ - Script::CSP17_UNIT1_NAME, - Script::CSP17_UNIT2_NAME, - Script::CSP17_UNIT3_NAME, - Script::CSP_UNIT1_NAME, - Script::CSP_UNIT2_NAME, - Script::CSP_UNIT3_NAME, - ].include?(name) - end + return false if %w(coursea-2017 courseb-2017 coursec-2017 coursed-2017 coursee-2017 coursef-2017 express-2017 pre-express-2017).include?(name) - def self.has_congrats_page?(name) - name == Script::ACCELERATED_NAME || - ScriptConstants.script_in_category?(:csf_international, name) || - ScriptConstants.script_in_category?(:csf, name) || - ScriptConstants.script_in_category?(:csf_2018, name) + k5_course? || %w(csp1-2017 csp2-2017 csp3-2017 cspunit1 cspunit2 cspunit3).include?(name) end def freeplay_links diff --git a/lib/cdo/script_constants.rb b/lib/cdo/script_constants.rb index 6b75ac5c216ca..0f375c2ae900c 100644 --- a/lib/cdo/script_constants.rb +++ b/lib/cdo/script_constants.rb @@ -76,6 +76,7 @@ module ScriptConstants COURSE2_NAME = 'course2'.freeze, COURSE3_NAME = 'course3'.freeze, COURSE4_NAME = 'course4'.freeze, + TWENTY_HOUR_NAME = '20-hour'.freeze, ], math: [ ALGEBRA_NAME = 'algebra'.freeze, @@ -145,9 +146,7 @@ module ScriptConstants CSP_EXAM1_NAME = 'cspexam1-mWU7ilDYM9'.freeze, CSP_EXAM2_NAME = 'cspexam2-AKwgAh1ac5'.freeze, ], - twenty_hour: [ - TWENTY_HOUR_NAME = '20-hour'.freeze, - ], + twenty_hour: [TWENTY_HOUR_NAME], flappy: [FLAPPY_NAME], minecraft: [ MINECRAFT_NAME, diff --git a/pegasus/sites.v3/code.org/public/congrats/splat.haml b/pegasus/sites.v3/code.org/public/congrats/splat.haml index c0541160a1bc2..b420033142c92 100644 --- a/pegasus/sites.v3/code.org/public/congrats/splat.haml +++ b/pegasus/sites.v3/code.org/public/congrats/splat.haml @@ -17,7 +17,17 @@ max_age: 60 - course = File.basename(request.path_info) -- if Script.has_congrats_page?(course) +- if [ ScriptConstants::COURSE1_NAME, + ScriptConstants::COURSE2_NAME, + ScriptConstants::COURSE3_NAME, + ScriptConstants::COURSE4_NAME, + ScriptConstants::COURSEA_NAME, + ScriptConstants::COURSEB_NAME, + ScriptConstants::COURSEC_NAME, + ScriptConstants::COURSED_NAME, + ScriptConstants::COURSEE_NAME, + ScriptConstants::COURSEF_NAME, + ScriptConstants::ACCELERATED_NAME ].include? course = view :csf_congrats, course: course - else - redirect "/congrats" diff --git a/pegasus/sites.v3/code.org/views/csf_congrats.haml b/pegasus/sites.v3/code.org/views/csf_congrats.haml index 01fd6e186aa85..5a48bc39e151e 100644 --- a/pegasus/sites.v3/code.org/views/csf_congrats.haml +++ b/pegasus/sites.v3/code.org/views/csf_congrats.haml @@ -26,13 +26,6 @@ ScriptConstants::COURSED_NAME => ScriptConstants::COURSEE_NAME, ScriptConstants::COURSEE_NAME => ScriptConstants::COURSEF_NAME, ScriptConstants::COURSEF_NAME => "applab", - - ScriptConstants::COURSEA_2018_NAME => ScriptConstants::COURSEB_2018_NAME, - ScriptConstants::COURSEB_2018_NAME => ScriptConstants::COURSEC_2018_NAME, - ScriptConstants::COURSEC_2018_NAME => ScriptConstants::COURSED_2018_NAME, - ScriptConstants::COURSED_2018_NAME => ScriptConstants::COURSEE_2018_NAME, - ScriptConstants::COURSEE_2018_NAME => ScriptConstants::COURSEF_2018_NAME, - ScriptConstants::COURSEF_2018_NAME => "applab", } rec_third_party = { @@ -49,13 +42,6 @@ ScriptConstants::COURSED_NAME => "csf_next", ScriptConstants::COURSEE_NAME => "csf_next", ScriptConstants::COURSEF_NAME => "csf_next", - - ScriptConstants::COURSEA_2018_NAME => "csf_next", - ScriptConstants::COURSEB_2018_NAME => "csf_next", - ScriptConstants::COURSEC_2018_NAME => "csf_next", - ScriptConstants::COURSED_2018_NAME => "csf_next", - ScriptConstants::COURSEE_2018_NAME => "csf_next", - ScriptConstants::COURSEF_2018_NAME => "csf_next", } #congrats.mobile-pad{:style=>'margin: 0 auto;'} From d1abafcccad9788f0867b964bfdd8fca8cad55e2 Mon Sep 17 00:00:00 2001 From: Mehal Shah Date: Tue, 3 Jul 2018 17:38:11 -0700 Subject: [PATCH 29/89] Fix bad merge --- dashboard/lib/pd/jot_form/matrix_question.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/dashboard/lib/pd/jot_form/matrix_question.rb b/dashboard/lib/pd/jot_form/matrix_question.rb index 6eea23bb25306..1ce71daf3ba5a 100644 --- a/dashboard/lib/pd/jot_form/matrix_question.rb +++ b/dashboard/lib/pd/jot_form/matrix_question.rb @@ -66,8 +66,6 @@ def summarize [ generate_sub_question_key(i), { - text: "#{text} #{sub_question}", - answer_type: ANSWER_SELECT_VALUE, parent: name, max_value: options.length, text: "#{text} #{sub_question}", From 000fa06f1c781f282cd6b69046253f8b82238564 Mon Sep 17 00:00:00 2001 From: nkiru onwuneme Date: Wed, 4 Jul 2018 14:23:04 -0700 Subject: [PATCH 30/89] display message when there is no answer text entered for a free response question --- apps/i18n/common/en_us.json | 1 + .../templates/sectionAssessments/sectionAssessmentsRedux.js | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/apps/i18n/common/en_us.json b/apps/i18n/common/en_us.json index 5e21622ec1c1a..8eabc7c42e569 100644 --- a/apps/i18n/common/en_us.json +++ b/apps/i18n/common/en_us.json @@ -479,6 +479,7 @@ "emptyBlockInVariable": "The variable {name} has an unfilled input.", "emptyBlocksErrorMsg": "The \"Repeat\" or \"If\" block needs to have other blocks inside it to work. Make sure the inner block fits properly inside the containing block.", "emptyExampleBlockErrorMsg": "You need at least two examples in function {functionName}. Make sure each example has a call and a result.", + "emptyFreeResponse": "No response given for this question", "emptyFunctionBlocksErrorMsg": "The function block needs to have other blocks inside it to work.", "emptyFunctionalBlock": "You have a block with an unfilled input.", "emptySurveyOverviewTable": "Because this survey is anonymous, we can only show aggregated results once there are at least 5 submissions.", diff --git a/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js b/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js index 2c486676cb356..3c87fd412b927 100644 --- a/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js +++ b/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js @@ -43,6 +43,8 @@ const MultiAnswerStatus = { const ANSWER_LETTERS = ['A', 'B', 'C', 'D', 'E', 'F', 'G', 'H']; +const EMPTY_FREE_RESPONSE_MESSAGE = i18n.emptyFreeResponse(); + // Action type constants const SET_ASSESSMENT_RESPONSES = 'sectionAssessments/SET_ASSESSMENT_RESPONSES'; const SET_ASSESSMENTS_QUESTIONS = 'sectionAssessments/SET_ASSESSMENTS_QUESTIONS'; @@ -283,7 +285,7 @@ export const getAssessmentsFreeResponseResults = (state) => { questionsAndResults[index].responses.push({ id: studentId, name: studentObject.student_name, - response: response.student_result, + response: response.student_result || EMPTY_FREE_RESPONSE_MESSAGE, }); }); }); @@ -309,7 +311,7 @@ export const getSurveyFreeResponseQuestions = (state) => { questionText: question.question, questionNumber: question.question_index + 1, answers: question.results.map((response, index) => { - return {index: index, response: response.result}; + return {index: index, response: response.result || EMPTY_FREE_RESPONSE_MESSAGE}; }), }; }); From 22d5e6a03179e1d0b5ed0989c6182671465046a0 Mon Sep 17 00:00:00 2001 From: Suresh Chanmugam Date: Wed, 4 Jul 2018 19:40:19 -0700 Subject: [PATCH 31/89] Find and iterate over production front end servers, logging their uptime and memory utilization. --- .../restart_high_memory_frontend_services | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100755 bin/cron/restart_high_memory_frontend_services diff --git a/bin/cron/restart_high_memory_frontend_services b/bin/cron/restart_high_memory_frontend_services new file mode 100755 index 0000000000000..9112d51c4362a --- /dev/null +++ b/bin/cron/restart_high_memory_frontend_services @@ -0,0 +1,63 @@ +#!/usr/bin/env ruby + +require_relative '../../dashboard/config/environment' +require_relative '../../lib/cdo/only_one' +require 'aws-sdk' +require 'cdo/chat_client' + +def main + ChatClient.message 'infra-production', 'Beginning to find and restart front end services with high memory utilization' + ec2_resource = Aws::EC2::Resource.new + cloudwatch_resource = Aws::CloudWatch::Resource.new + production_frontends = ec2_resource.instances( + { + filters: [ + { + name: 'tag:environment', + values: [ + 'production' + ] + }, + { + name: 'tag:aws:cloudformation:logical-id', + values: [ + 'Frontends' + ] + }, + { + name: 'instance-state-name', + values: [ + 'running' + ] + } + ] + } + ) + ChatClient.message 'infra-production', "Found #{production_frontends.count} production front end webservers" + production_frontends.each do |instance| + # A new may not have reported MemoryUtilization metrics yet. + memory_utilization = cloudwatch_resource.metric('System/Linux', 'MemoryUtilization').get_statistics( + { + dimensions: [ + { + name: 'InstanceId', + value: instance.instance_id, + } + ], + start_time: Time.now - 5.minutes, + end_time: Time.now, + period: 5.minutes, + statistics: ['Average'], + unit: 'Percent' + } + ).datapoints[0]&.average + uptime_days = (Time.now - instance.launch_time) / (60 * 60 * 24) + ChatClient.message 'infra-production', "Instance ID - #{instance.instance_id} / Memory Utilization - #{memory_utilization&.round}% / Uptime (Days) - #{uptime_days.round(2)}" + if uptime_days > 1 + ChatClient.message 'infra-production', "Restarting Dashboard and Pegasus Unicorn processes on Instance #{instance.instance_id} because it has been up for more than 1 day." + end + end + ChatClient.message 'infra-production', 'Done finding and restarting front end services with high memory utilization' +end + +main if only_one_running?(__FILE__) From 49b9f49a1f4ca4184c1a785ee692ca528948ffe8 Mon Sep 17 00:00:00 2001 From: Suresh Chanmugam Date: Wed, 4 Jul 2018 20:57:45 -0700 Subject: [PATCH 32/89] Add logic to SSH into each front end server and restart pegasus/dashboard. --- bin/cron/restart_high_memory_frontend_services | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/bin/cron/restart_high_memory_frontend_services b/bin/cron/restart_high_memory_frontend_services index 9112d51c4362a..cd3cac2dd7925 100755 --- a/bin/cron/restart_high_memory_frontend_services +++ b/bin/cron/restart_high_memory_frontend_services @@ -4,11 +4,13 @@ require_relative '../../dashboard/config/environment' require_relative '../../lib/cdo/only_one' require 'aws-sdk' require 'cdo/chat_client' +require 'sshkit' def main ChatClient.message 'infra-production', 'Beginning to find and restart front end services with high memory utilization' ec2_resource = Aws::EC2::Resource.new cloudwatch_resource = Aws::CloudWatch::Resource.new + frontend_servers_to_restart = Array.new production_frontends = ec2_resource.instances( { filters: [ @@ -53,10 +55,20 @@ def main ).datapoints[0]&.average uptime_days = (Time.now - instance.launch_time) / (60 * 60 * 24) ChatClient.message 'infra-production', "Instance ID - #{instance.instance_id} / Memory Utilization - #{memory_utilization&.round}% / Uptime (Days) - #{uptime_days.round(2)}" - if uptime_days > 1 - ChatClient.message 'infra-production', "Restarting Dashboard and Pegasus Unicorn processes on Instance #{instance.instance_id} because it has been up for more than 1 day." - end + frontend_servers_to_restart << instance.private_dns_name if uptime_days > 1 end + + delay_per_group = 10.minutes + group_size = 1 + # restart_command = 'sudo service dashboard upgrade && sudo service pegasus upgrade' + # Temporarily use a dummy command. + restart_command = 'ls -l' + + SSHKit::Backend::Netssh.configure {|ssh| ssh.ssh_options = {paranoid: false}} + SSHKit::Coordinator.new(frontend_servers_to_restart).each(in: :sequence, wait: delay_per_group, limit: group_size) do + ChatClient.message 'infra-production', capture(restart_command, raise_on_non_zero_exit: false) + end + ChatClient.message 'infra-production', 'Done finding and restarting front end services with high memory utilization' end From ff308cef7450ff1e5e732336bb91aa1a7d2a5da2 Mon Sep 17 00:00:00 2001 From: nkiru onwuneme Date: Wed, 4 Jul 2018 21:11:39 -0700 Subject: [PATCH 33/89] Add function to calculate opacity for the answer cells --- .../MultipleChoiceAssessmentsOverviewTable.jsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/apps/src/templates/sectionAssessments/MultipleChoiceAssessmentsOverviewTable.jsx b/apps/src/templates/sectionAssessments/MultipleChoiceAssessmentsOverviewTable.jsx index 7a96cc6646e65..50d3920524508 100644 --- a/apps/src/templates/sectionAssessments/MultipleChoiceAssessmentsOverviewTable.jsx +++ b/apps/src/templates/sectionAssessments/MultipleChoiceAssessmentsOverviewTable.jsx @@ -32,6 +32,11 @@ const styles = { const NOT_ANSWERED = 'notAnswered'; +function calculateOpacity(answered) { + let result = ((answered + 10) * (9 / 10))/100; + return result; +} + const answerColumnsFormatter = (percentAnswered, {rowData, columnIndex, rowIndex, property}) => { let percentValue = 0; const answerResults = rowData.answers[columnIndex - 1] || {}; @@ -45,6 +50,7 @@ const answerColumnsFormatter = (percentAnswered, {rowData, columnIndex, rowIndex return ( From 09f33c3a05624f4f0c7a1857d29c48ad09333f33 Mon Sep 17 00:00:00 2001 From: Brad Buchanan Date: Wed, 4 Jul 2018 21:59:20 -0700 Subject: [PATCH 34/89] ES6ify CodeWorkspace --- apps/src/templates/CodeWorkspace.jsx | 48 +++++++++++++--------------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/apps/src/templates/CodeWorkspace.jsx b/apps/src/templates/CodeWorkspace.jsx index 877aebd1aceaf..8b2f480f41d89 100644 --- a/apps/src/templates/CodeWorkspace.jsx +++ b/apps/src/templates/CodeWorkspace.jsx @@ -1,21 +1,21 @@ import $ from 'jquery'; import React, {PropTypes} from 'react'; -var Radium = require('radium'); -var connect = require('react-redux').connect; -var ProtectedStatefulDiv = require('./ProtectedStatefulDiv'); +import Radium from 'radium'; +import {connect} from 'react-redux'; +import ProtectedStatefulDiv from './ProtectedStatefulDiv'; import JsDebugger from '@cdo/apps/lib/tools/jsdebugger/JsDebugger'; import PaneHeader, {PaneSection, PaneButton} from './PaneHeader'; -var msg = require('@cdo/locale'); -var commonStyles = require('../commonStyles'); -var color = require("../util/color"); -var utils = require('@cdo/apps/utils'); +import msg from '@cdo/locale'; +import commonStyles from '../commonStyles'; +import color from "../util/color"; +import * as utils from '@cdo/apps/utils'; import {shouldUseRunModeIndicators} from '../redux/selectors'; import SettingsCog from '../lib/ui/SettingsCog'; import ShowCodeToggle from './ShowCodeToggle'; import {singleton as studioApp} from '../StudioApp'; import ProjectTemplateWorkspaceIcon from './ProjectTemplateWorkspaceIcon'; -var styles = { +const styles = { headerIcon: { fontSize: 18 }, @@ -30,8 +30,8 @@ var styles = { }, }; -var CodeWorkspace = React.createClass({ - propTypes: { +class CodeWorkspace extends React.Component { + static propTypes = { isRtl: PropTypes.bool.isRequired, editCode: PropTypes.bool.isRequired, readonlyWorkspace: PropTypes.bool.isRequired, @@ -44,9 +44,9 @@ var CodeWorkspace = React.createClass({ runModeIndicators: PropTypes.bool.isRequired, withSettingsCog: PropTypes.bool, showMakerToggle: PropTypes.bool, - }, + }; - shouldComponentUpdate: function (nextProps) { + shouldComponentUpdate(nextProps) { // This component is current semi-protected. We don't want to completely // disallow rerendering, since that would prevent us from being able to // update styles. However, we do want to prevent property changes that would @@ -63,9 +63,9 @@ var CodeWorkspace = React.createClass({ }.bind(this)); return true; - }, + } - onDebuggerSlide(debuggerHeight) { + onDebuggerSlide = (debuggerHeight) => { const textbox = this.codeTextbox.getRoot(); if (textbox.style.bottom) { $(textbox).animate( @@ -84,7 +84,7 @@ var CodeWorkspace = React.createClass({ textbox.style.bottom = debuggerHeight + 'px'; utils.fireResizeEvent(); } - }, + }; renderToolboxHeaders() { const { @@ -137,23 +137,19 @@ var CodeWorkspace = React.createClass({ {settingsCog} ]; - }, + } - onToggleShowCode(usingBlocks) { + onToggleShowCode = (usingBlocks) => { this.blockCounterEl.style.display = (usingBlocks && studioApp.enableShowBlockCount) ? 'inline-block' : 'none'; - }, + }; render() { - var props = this.props; + const props = this.props; // By default, continue to show header as focused. When runModeIndicators // is enabled, remove focus while running. - var hasFocus = true; - if (props.runModeIndicators && props.isRunning) { - hasFocus = false; - } - + const hasFocus = !(props.runModeIndicators && props.isRunning); const isRtl = this.props.isRtl; return ( @@ -197,7 +193,7 @@ var CodeWorkspace = React.createClass({
    this.blockCounterEl = el}> / - + {" " + msg.blocks()}
    @@ -219,7 +215,7 @@ var CodeWorkspace = React.createClass({ ); } -}); +} module.exports = connect(state => ({ editCode: state.pageConstants.isDroplet, From a429cb2634f6488ae72cc290ddd67487e665a6cd Mon Sep 17 00:00:00 2001 From: nkiru onwuneme Date: Wed, 4 Jul 2018 22:12:13 -0700 Subject: [PATCH 35/89] answer column styles updated and the render function of multiple choice answer cell component returns rgba value based on opacity --- .../sectionAssessments/MultipleChoiceAnswerCell.jsx | 9 +++++++-- .../MultipleChoiceAssessmentsOverviewTable.jsx | 2 ++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/apps/src/templates/sectionAssessments/MultipleChoiceAnswerCell.jsx b/apps/src/templates/sectionAssessments/MultipleChoiceAnswerCell.jsx index 01893b0fad6b8..fa974922a6152 100644 --- a/apps/src/templates/sectionAssessments/MultipleChoiceAnswerCell.jsx +++ b/apps/src/templates/sectionAssessments/MultipleChoiceAnswerCell.jsx @@ -25,10 +25,15 @@ class MultipleChoiceAnswerCell extends Component { percentValue: PropTypes.number.isRequired, isCorrectAnswer: PropTypes.bool, displayAnswer: PropTypes.string, + opacity: PropTypes.number, }; render() { - const {percentValue, isCorrectAnswer, displayAnswer} = this.props; + const {percentValue, isCorrectAnswer, displayAnswer, opacity} = this.props; + + const rgbaValue = (isCorrectAnswer) ? { backgroundColor: `rgba(14, 190, 14, ${opacity})` } : + { backgroundColor: `rgba(255, 99, 71, ${opacity})`}; + if (displayAnswer) { return (
    @@ -45,7 +50,7 @@ class MultipleChoiceAnswerCell extends Component { } return ( -
    +
    {(percentValue >= 0) && {`${percentValue}%`} diff --git a/apps/src/templates/sectionAssessments/MultipleChoiceAssessmentsOverviewTable.jsx b/apps/src/templates/sectionAssessments/MultipleChoiceAssessmentsOverviewTable.jsx index 50d3920524508..c685b09cb2bcd 100644 --- a/apps/src/templates/sectionAssessments/MultipleChoiceAssessmentsOverviewTable.jsx +++ b/apps/src/templates/sectionAssessments/MultipleChoiceAssessmentsOverviewTable.jsx @@ -22,6 +22,8 @@ const styles = { }, answerColumnCell: { width: ANSWER_COLUMN_WIDTH, + padding: 0, + height: 40, }, questionCell: { overflow: 'hidden', From e4b57e88a8511670aa480fc9147e458215ed77fc Mon Sep 17 00:00:00 2001 From: Suresh Chanmugam Date: Wed, 4 Jul 2018 22:23:40 -0700 Subject: [PATCH 36/89] Set the real restart command. --- bin/cron/restart_high_memory_frontend_services | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/cron/restart_high_memory_frontend_services b/bin/cron/restart_high_memory_frontend_services index cd3cac2dd7925..a9785ab883058 100755 --- a/bin/cron/restart_high_memory_frontend_services +++ b/bin/cron/restart_high_memory_frontend_services @@ -60,9 +60,9 @@ def main delay_per_group = 10.minutes group_size = 1 - # restart_command = 'sudo service dashboard upgrade && sudo service pegasus upgrade' - # Temporarily use a dummy command. - restart_command = 'ls -l' + restart_command = 'sudo service dashboard upgrade && sudo service pegasus upgrade' + + # TODO: (suresh) Add system call to copy the ssh config files needed for SSHKit to work? SSHKit::Backend::Netssh.configure {|ssh| ssh.ssh_options = {paranoid: false}} SSHKit::Coordinator.new(frontend_servers_to_restart).each(in: :sequence, wait: delay_per_group, limit: group_size) do From a51d70e49606f620207a3541bba2901025e583e9 Mon Sep 17 00:00:00 2001 From: nkiru onwuneme Date: Thu, 5 Jul 2018 00:02:13 -0700 Subject: [PATCH 37/89] style answer and not answered cells --- .../sectionAssessments/MultipleChoiceAnswerCell.jsx | 5 +++-- .../MultipleChoiceAssessmentsOverviewTable.jsx | 11 ++++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/apps/src/templates/sectionAssessments/MultipleChoiceAnswerCell.jsx b/apps/src/templates/sectionAssessments/MultipleChoiceAnswerCell.jsx index fa974922a6152..ff321e5aab9b8 100644 --- a/apps/src/templates/sectionAssessments/MultipleChoiceAnswerCell.jsx +++ b/apps/src/templates/sectionAssessments/MultipleChoiceAnswerCell.jsx @@ -9,6 +9,7 @@ const styles = { justifyContent: 'space-between', flexDirection: 'row', alignItems: 'center', + height: '100%', }, icon: { color: color.level_perfect, @@ -31,8 +32,8 @@ class MultipleChoiceAnswerCell extends Component { render() { const {percentValue, isCorrectAnswer, displayAnswer, opacity} = this.props; - const rgbaValue = (isCorrectAnswer) ? { backgroundColor: `rgba(14, 190, 14, ${opacity})` } : - { backgroundColor: `rgba(255, 99, 71, ${opacity})`}; + const rgbaValue = (isCorrectAnswer) ? {backgroundColor: `rgba(14, 190, 14, ${opacity})`} : + {backgroundColor: `rgba(255, 99, 71, ${opacity})`}; if (displayAnswer) { return ( diff --git a/apps/src/templates/sectionAssessments/MultipleChoiceAssessmentsOverviewTable.jsx b/apps/src/templates/sectionAssessments/MultipleChoiceAssessmentsOverviewTable.jsx index c685b09cb2bcd..77882c8edac9f 100644 --- a/apps/src/templates/sectionAssessments/MultipleChoiceAssessmentsOverviewTable.jsx +++ b/apps/src/templates/sectionAssessments/MultipleChoiceAssessmentsOverviewTable.jsx @@ -25,6 +25,10 @@ const styles = { padding: 0, height: 40, }, + notAnsweredCell: { + padding: 0, + height: 35, + }, questionCell: { overflow: 'hidden', textOverflow: 'ellipsis', @@ -115,7 +119,12 @@ class MultipleChoiceAssessmentsOverviewTable extends Component { }, cell: { format: answerColumnsFormatter, - props: {style: tableLayoutStyles.cell}, + props: { + style: { + ...tableLayoutStyles.cell, + ...styles.notAnsweredCell, + } + }, } } ); From 9d3e9276acda5f44fff5e5ac18899319421a2e46 Mon Sep 17 00:00:00 2001 From: Brad Buchanan Date: Thu, 5 Jul 2018 08:20:16 -0700 Subject: [PATCH 38/89] /users/set_age returns 403 unless signed in --- .../controllers/registrations_controller.rb | 1 + .../registrations_controller/set_age_test.rb | 36 +++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 dashboard/test/controllers/registrations_controller/set_age_test.rb diff --git a/dashboard/app/controllers/registrations_controller.rb b/dashboard/app/controllers/registrations_controller.rb index 97bc8cc651ea7..f1ba046f2ec73 100644 --- a/dashboard/app/controllers/registrations_controller.rb +++ b/dashboard/app/controllers/registrations_controller.rb @@ -74,6 +74,7 @@ def sign_up_params # Set age for the current user if empty - skips CSRF verification because this can be called # from cached pages which will not populate the CSRF token def set_age + return head(:forbidden) unless current_user current_user.update(age: params[:user][:age]) unless current_user.age.present? end diff --git a/dashboard/test/controllers/registrations_controller/set_age_test.rb b/dashboard/test/controllers/registrations_controller/set_age_test.rb new file mode 100644 index 0000000000000..77b6bf2bba0c1 --- /dev/null +++ b/dashboard/test/controllers/registrations_controller/set_age_test.rb @@ -0,0 +1,36 @@ +require 'test_helper' + +module RegistrationsControllerTests + # + # Tests over PATCH /users/set_age + # + class SetAgeTest < ActionDispatch::IntegrationTest + test "set_age does nothing if user is not signed in" do + User.any_instance.expects(:update).never + patch '/users/set_age', params: {user: {age: '20'}} + assert_response :forbidden + end + + test "set_age does nothing if user age is already set" do + User.any_instance.expects(:update).never + student = create :student + refute student.age.blank? + + sign_in student + patch '/users/set_age', params: {user: {age: '20'}} + assert_response :success + end + + test "set_age sets age if user is signed in and age is blank" do + student = create :student_in_picture_section, birthday: nil + assert student.age.blank? + + sign_in student + patch '/users/set_age', params: {user: {age: '20'}} + assert_response :success + + student.reload + assert_equal 20, student.age + end + end +end From 8a64b3754420f54d6ac66546b74744db8861ebc7 Mon Sep 17 00:00:00 2001 From: Maddie Kasula Date: Thu, 5 Jul 2018 09:05:56 -0700 Subject: [PATCH 39/89] Summarize authentication_options to avoid passing oauth tokens to client --- dashboard/app/models/authentication_option.rb | 9 +++++++++ dashboard/app/views/devise/registrations/edit.html.haml | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/dashboard/app/models/authentication_option.rb b/dashboard/app/models/authentication_option.rb index 13ecb00733501..459e29a659538 100644 --- a/dashboard/app/models/authentication_option.rb +++ b/dashboard/app/models/authentication_option.rb @@ -83,6 +83,15 @@ def data_hash end end + def summarize + { + id: id, + credential_type: credential_type, + email: email, + hashed_email: hashed_email + } + end + private def email_must_be_unique # skip the db lookup if possible return unless email_changed? && email.present? && errors.blank? diff --git a/dashboard/app/views/devise/registrations/edit.html.haml b/dashboard/app/views/devise/registrations/edit.html.haml index 163ff39a3059d..0e0647eec5d2b 100644 --- a/dashboard/app/views/devise/registrations/edit.html.haml +++ b/dashboard/app/views/devise/registrations/edit.html.haml @@ -210,7 +210,7 @@ userType: current_user.user_type, isOauth: current_user.oauth?, isPasswordRequired: current_user.encrypted_password.present?, - authenticationOptions: current_user.authentication_options, + authenticationOptions: current_user.authentication_options.map(&:summarize), }.to_json } %script{src: minifiable_asset_path('js/devise/registrations/edit.js'), data: script_data} From b2595de6c18877018284cd863f5c66d66f08e22b Mon Sep 17 00:00:00 2001 From: Maddie Kasula Date: Thu, 5 Jul 2018 09:17:27 -0700 Subject: [PATCH 40/89] Fix storybook errors, add more stories --- .../accounts/ManageLinkedAccounts.story.jsx | 46 ++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/apps/src/lib/ui/accounts/ManageLinkedAccounts.story.jsx b/apps/src/lib/ui/accounts/ManageLinkedAccounts.story.jsx index 032262e7dbfc9..9a7b6fd8f545a 100644 --- a/apps/src/lib/ui/accounts/ManageLinkedAccounts.story.jsx +++ b/apps/src/lib/ui/accounts/ManageLinkedAccounts.story.jsx @@ -1,6 +1,50 @@ import React from 'react'; +import {action} from '@storybook/addon-actions'; import ManageLinkedAccounts from './ManageLinkedAccounts'; +const DEFAULT_PROPS = { + userType: 'student', + authenticationOptions: [], + connect: action('connect'), + disconnect: action('disconnect'), +}; + +const mockAuthenticationOptions = [ + {id: 1, credential_type: 'google_oauth2', email: 'google@email.com'}, + {id: 2, credential_type: 'facebook', email: 'facebook@email.com'}, + {id: 3, credential_type: 'clever', email: 'clever@email.com'}, + {id: 4, credential_type: 'windowslive', email: 'windowslive@email.com'}, +]; + export default storybook => storybook .storiesOf('ManageLinkedAccounts', module) - .add('default table', () => ()); + .addStoryTable([ + { + name: 'default table', + story: () => ( + + ) + }, + { + name: 'table for teacher with all authentication options', + story: () => ( + + ) + }, + { + name: 'table for student with all authentication options', + story: () => ( + + ) + }, + ]); From fbb569bea3671876ced8b68dc0e555b83f0171e9 Mon Sep 17 00:00:00 2001 From: Caley Brock Date: Thu, 5 Jul 2018 10:29:56 -0700 Subject: [PATCH 41/89] Set student Id action in section assessments redux --- .../sectionAssessments/sectionAssessmentsRedux.js | 13 +++++++++++++ .../sectionAssessmentsReduxTest.js | 9 +++++++++ 2 files changed, 22 insertions(+) diff --git a/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js b/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js index 2c486676cb356..e3237d64f79f7 100644 --- a/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js +++ b/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js @@ -1,6 +1,8 @@ import {SET_SECTION} from '@cdo/apps/redux/sectionDataRedux'; import i18n from '@cdo/locale'; +const ALL_STUDENT_FILTER = 0; + /** * Initial state of sectionAssessmentsRedux * The redux state matches the structure of our API calls and our views don't @@ -15,6 +17,7 @@ import i18n from '@cdo/locale'; * isLoading - boolean - indicates that requests for assessments and surveys have been * sent to the server but the client has not yet received a response * assessmentId - int - the level_group id of the assessment currently in view + * studentId - int - the studentId of the current student being filtered for. */ const initialState = { assessmentResponsesByScript: {}, @@ -22,6 +25,7 @@ const initialState = { surveysByScript: {}, isLoading: false, assessmentId: 0, + studentId: ALL_STUDENT_FILTER, }; // Question types for assessments. @@ -50,6 +54,7 @@ const SET_SURVEYS = 'sectionAssessments/SET_SURVEYS'; const START_LOADING_ASSESSMENTS = 'sectionAssessments/START_LOADING_ASSESSMENTS'; const FINISH_LOADING_ASSESSMENTS = 'sectionAssessments/FINISH_LOADING_ASSESSMENTS'; const SET_ASSESSMENT_ID = 'sectionAssessments/SET_ASSESSMENT_ID'; +const SET_STUDENT_ID = 'sectionAssessments/SET_STUDENT_ID'; // Action creators export const setAssessmentResponses = (scriptId, assessments) => @@ -59,6 +64,7 @@ export const setAssessmentQuestions = (scriptId, assessments) => export const startLoadingAssessments = () => ({ type: START_LOADING_ASSESSMENTS }); export const finishLoadingAssessments = () => ({ type: FINISH_LOADING_ASSESSMENTS }); export const setAssessmentId = (assessmentId) => ({ type: SET_ASSESSMENT_ID, assessmentId: assessmentId }); +export const setStudentId = (studentId) => ({ type: SET_STUDENT_ID, studentId: studentId }); export const setSurveys = (scriptId, surveys) => ({ type: SET_SURVEYS, scriptId, surveys }); export const asyncLoadAssessments = (sectionId, scriptId) => { @@ -100,6 +106,13 @@ export default function sectionAssessments(state=initialState, action) { return { ...state, assessmentId: action.assessmentId, + studentId: ALL_STUDENT_FILTER, + }; + } + if (action.type === SET_STUDENT_ID) { + return { + ...state, + studentId: action.studentId, }; } if (action.type === SET_ASSESSMENT_RESPONSES) { diff --git a/apps/test/unit/templates/sectionAssessments/sectionAssessmentsReduxTest.js b/apps/test/unit/templates/sectionAssessments/sectionAssessmentsReduxTest.js index 9c2332c2738d0..de451a3d23072 100644 --- a/apps/test/unit/templates/sectionAssessments/sectionAssessmentsReduxTest.js +++ b/apps/test/unit/templates/sectionAssessments/sectionAssessmentsReduxTest.js @@ -20,6 +20,7 @@ import sectionAssessments, { isCurrentAssessmentSurvey, getExportableSurveyData, getExportableAssessmentData, + setStudentId, } from '@cdo/apps/templates/sectionAssessments/sectionAssessmentsRedux'; import {setSection} from '@cdo/apps/redux/sectionDataRedux'; @@ -89,6 +90,14 @@ describe('sectionAssessmentsRedux', () => { }); }); + describe('setStudentId', () => { + it('sets the id of the current student in view', () => { + const action = setStudentId(777); + const nextState = sectionAssessments(initialState, action); + assert.deepEqual(nextState.studentId, 777); + }); + }); + describe('startLoadingAssessments', () => { it('sets isLoading to true', () => { const action = startLoadingAssessments(); From b86acedd339e2c8a77195a626cad1254620d486f Mon Sep 17 00:00:00 2001 From: nkiru onwuneme Date: Thu, 5 Jul 2018 10:48:37 -0700 Subject: [PATCH 42/89] Adjust the margins on the div that contains the percent value and checkmark icon to match product spec --- .../templates/sectionAssessments/MultipleChoiceAnswerCell.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/src/templates/sectionAssessments/MultipleChoiceAnswerCell.jsx b/apps/src/templates/sectionAssessments/MultipleChoiceAnswerCell.jsx index ff321e5aab9b8..a0f70d0aff024 100644 --- a/apps/src/templates/sectionAssessments/MultipleChoiceAnswerCell.jsx +++ b/apps/src/templates/sectionAssessments/MultipleChoiceAnswerCell.jsx @@ -6,7 +6,6 @@ const styles = { main: { border: 'none', display: 'flex', - justifyContent: 'space-between', flexDirection: 'row', alignItems: 'center', height: '100%', @@ -18,6 +17,7 @@ const styles = { color: color.charcoal, fontFamily: '"Gotham 5r", sans-serif', marginRight: 10, + marginLeft: 10, }, }; From c0772363b8a0e5c08e59d5fb96048c353d8f6024 Mon Sep 17 00:00:00 2001 From: Caley Brock Date: Thu, 5 Jul 2018 10:57:09 -0700 Subject: [PATCH 43/89] Add student filter dropdown --- apps/i18n/common/en_us.json | 1 + apps/src/redux/sectionDataRedux.js | 4 ++ .../sectionAssessments/SectionAssessments.jsx | 22 +++++++++- .../sectionAssessments/StudentSelector.jsx | 41 +++++++++++++++++++ .../sectionAssessmentsRedux.js | 2 +- 5 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 apps/src/templates/sectionAssessments/StudentSelector.jsx diff --git a/apps/i18n/common/en_us.json b/apps/i18n/common/en_us.json index 0d00aea1069d1..7c00f0827f55f 100644 --- a/apps/i18n/common/en_us.json +++ b/apps/i18n/common/en_us.json @@ -1020,6 +1020,7 @@ "seeAllTutorials": "See all tutorials", "selectACourse": "Select a course or unit", "selectAssessment": "Select an assessment or survey", + "selectStudent": "Filter by student", "selectGoogleClassroom": "Select a Google Classroom", "selectCleverSection": "Select a Clever section", "selectSection": "Select Section", diff --git a/apps/src/redux/sectionDataRedux.js b/apps/src/redux/sectionDataRedux.js index d5f8252eafb22..b2d0bfabd7fa9 100644 --- a/apps/src/redux/sectionDataRedux.js +++ b/apps/src/redux/sectionDataRedux.js @@ -60,3 +60,7 @@ export default function sectionData(state=initialState, action) { export const getTotalStudentCount = (state) => { return state.sectionData.section.students.length; }; + +export const getStudentList = (state) => { + return state.sectionData.section.students; +}; diff --git a/apps/src/templates/sectionAssessments/SectionAssessments.jsx b/apps/src/templates/sectionAssessments/SectionAssessments.jsx index 661d802fb11b7..515d586862c3c 100644 --- a/apps/src/templates/sectionAssessments/SectionAssessments.jsx +++ b/apps/src/templates/sectionAssessments/SectionAssessments.jsx @@ -7,7 +7,9 @@ import { isCurrentAssessmentSurvey, countSubmissionsForCurrentAssessment, getExportableData, + setStudentId, } from '@cdo/apps/templates/sectionAssessments/sectionAssessmentsRedux'; +import { getStudentList } from '@cdo/apps/redux/sectionDataRedux'; import {connect} from 'react-redux'; import {h3Style} from "../../lib/ui/Headings"; import i18n from '@cdo/locale'; @@ -19,6 +21,7 @@ import FreeResponsesAssessmentsContainer from './FreeResponsesAssessmentsContain import FreeResponseBySurveyQuestionContainer from './FreeResponseBySurveyQuestionContainer'; import MCSurveyOverviewContainer from './MCSurveyOverviewContainer'; import AssessmentSelector from './AssessmentSelector'; +import StudentSelector from './StudentSelector'; import FontAwesome from '@cdo/apps/templates/FontAwesome'; import {CSVLink} from 'react-csv'; @@ -64,6 +67,9 @@ class SectionAssessments extends Component { isCurrentAssessmentSurvey: PropTypes.bool, totalStudentSubmissions: PropTypes.number, exportableData: PropTypes.array, + studentId: PropTypes.number, + setStudentId: PropTypes.func, + studentList: PropTypes.array, }; onChangeScript = scriptId => { @@ -74,7 +80,8 @@ class SectionAssessments extends Component { render() { const {validScripts, scriptId, assessmentList, assessmentId, - isLoading, isCurrentAssessmentSurvey, totalStudentSubmissions, exportableData} = this.props; + isLoading, isCurrentAssessmentSurvey, totalStudentSubmissions, + exportableData, studentId, studentList} = this.props; return (
    @@ -105,6 +112,14 @@ class SectionAssessments extends Component { {/* Assessments */} {!isCurrentAssessmentSurvey &&
    +
    + {i18n.selectStudent()} +
    + {totalStudentSubmissions > 0 && ({ isCurrentAssessmentSurvey: isCurrentAssessmentSurvey(state), totalStudentSubmissions: countSubmissionsForCurrentAssessment(state), exportableData: getExportableData(state), + studentId: state.sectionAssessments.studentId, + studentList: getStudentList(state), }), dispatch => ({ setScriptId(scriptId) { dispatch(setScriptId(scriptId)); @@ -180,4 +197,7 @@ export default connect(state => ({ setAssessmentId(assessmentId) { dispatch(setAssessmentId(assessmentId)); }, + setStudentId(studentId) { + dispatch(setStudentId(studentId)); + }, }))(SectionAssessments); diff --git a/apps/src/templates/sectionAssessments/StudentSelector.jsx b/apps/src/templates/sectionAssessments/StudentSelector.jsx new file mode 100644 index 0000000000000..598950944b1a7 --- /dev/null +++ b/apps/src/templates/sectionAssessments/StudentSelector.jsx @@ -0,0 +1,41 @@ +import React, { Component, PropTypes } from 'react'; +import { dropdownStyles } from '@cdo/apps/templates/sectionProgress/ScriptSelector'; +import { ALL_STUDENT_FILTER } from './sectionAssessmentsRedux'; + +export default class StudentSelector extends Component { + static propTypes = { + studentList: PropTypes.array.isRequired, + studentId: PropTypes.number, + onChange: PropTypes.func.isRequired, + }; + + render() { + const { studentList, studentId, onChange } = this.props; + + return ( +
    + +
    + ); + } +} diff --git a/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js b/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js index e3237d64f79f7..15dbb3e654bd2 100644 --- a/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js +++ b/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js @@ -1,7 +1,7 @@ import {SET_SECTION} from '@cdo/apps/redux/sectionDataRedux'; import i18n from '@cdo/locale'; -const ALL_STUDENT_FILTER = 0; +export const ALL_STUDENT_FILTER = 0; /** * Initial state of sectionAssessmentsRedux From 2cc6da521de8a626e8f2b4dea6ec64abdf2bfe90 Mon Sep 17 00:00:00 2001 From: David Bailey Date: Thu, 5 Jul 2018 11:06:05 -0700 Subject: [PATCH 44/89] stub commit hash out of sources_s3_directory in ruby unit tests --- shared/test/common_test_helper.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/shared/test/common_test_helper.rb b/shared/test/common_test_helper.rb index b18e09b56f95f..bde43e73d82a2 100644 --- a/shared/test/common_test_helper.rb +++ b/shared/test/common_test_helper.rb @@ -69,6 +69,15 @@ def around(&block) any_instance. stubs(:static_credentials). returns(credentials) + + # CDO.sources_s3_directory contains the commit hash when running in the test + # environment, so new projects created during UI tests will not already + # contain source code generated from previous test runs. However, this is + # not compatible with our unit tests which use VCR to stub out network + # requests to url paths which must be consistent across test runs. + # Therefore, remove the commit-specific part of this path only in unit tests. + CDO.stubs(:sources_s3_directory).returns('sources_test') + VCR.use_cassette(cassette_name, record: record_mode) do PEGASUS_DB.transaction(rollback: :always) do AWS::S3.stub(:random, proc {random.bytes(16).unpack('H*')[0]}, &block) From ceaf60ef390fdca91cfd61204abccb78ee0cb551 Mon Sep 17 00:00:00 2001 From: Maddie Kasula Date: Thu, 5 Jul 2018 11:08:01 -0700 Subject: [PATCH 45/89] Restore sinon.fakeServer after tests --- .../unit/lib/ui/accounts/ManageLinkedAccountsControllerTest.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/test/unit/lib/ui/accounts/ManageLinkedAccountsControllerTest.js b/apps/test/unit/lib/ui/accounts/ManageLinkedAccountsControllerTest.js index f7bf7be759615..b738de896ae22 100644 --- a/apps/test/unit/lib/ui/accounts/ManageLinkedAccountsControllerTest.js +++ b/apps/test/unit/lib/ui/accounts/ManageLinkedAccountsControllerTest.js @@ -56,6 +56,8 @@ describe('ManageLinkedAccountsController', () => { server = sinon.fakeServer.create(); }); + afterEach(() => server.restore()); + describe('onSuccess', () => { beforeEach(() => { controller = new ManageLinkedAccountsController(mockMountPoint, userType, mockAuthenticationOptions); From c5e1ea4b9026533a07739376684c99bd76aad335 Mon Sep 17 00:00:00 2001 From: Caley Brock Date: Thu, 5 Jul 2018 11:10:29 -0700 Subject: [PATCH 46/89] Filter status table by student --- .../sectionAssessments/sectionAssessmentsRedux.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js b/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js index 15dbb3e654bd2..cb492dbcc231a 100644 --- a/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js +++ b/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js @@ -420,7 +420,13 @@ export const getStudentsMCSummaryForCurrentAssessment = (state) => { ...studentResponses, }; - const studentsSummaryArray = Object.keys(allStudentsByIds).map(studentId => { + let currentStudentsIds = Object.keys(allStudentsByIds); + // Filter by current selected student. + if (state.sectionAssessments.studentId !== ALL_STUDENT_FILTER) { + currentStudentsIds = [state.sectionAssessments.studentId]; + } + + const studentsSummaryArray = currentStudentsIds.map(studentId => { studentId = (parseInt(studentId, 10)); const studentsObject = allStudentsByIds[studentId]; const currentAssessmentId = state.sectionAssessments.assessmentId; From 11298b4d258474f158b75cd0864de7a34e00ed56 Mon Sep 17 00:00:00 2001 From: Caley Brock Date: Thu, 5 Jul 2018 11:14:14 -0700 Subject: [PATCH 47/89] Hide mc overview if not 'all student' filter --- .../sectionAssessments/MCAssessmentsOverviewContainer.jsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/apps/src/templates/sectionAssessments/MCAssessmentsOverviewContainer.jsx b/apps/src/templates/sectionAssessments/MCAssessmentsOverviewContainer.jsx index ce0cf4afabdf5..003d93dce40d2 100644 --- a/apps/src/templates/sectionAssessments/MCAssessmentsOverviewContainer.jsx +++ b/apps/src/templates/sectionAssessments/MCAssessmentsOverviewContainer.jsx @@ -3,6 +3,7 @@ import MultipleChoiceAssessmentsOverviewTable from './MultipleChoiceAssessmentsO import { getMultipleChoiceSectionSummary, countSubmissionsForCurrentAssessment, + ALL_STUDENT_FILTER, } from './sectionAssessmentsRedux'; import { connect } from 'react-redux'; import { multipleChoiceDataPropType } from './assessmentDataShapes'; @@ -14,13 +15,14 @@ class MCAssessmentsOverviewContainer extends Component { questionAnswerData: PropTypes.arrayOf(multipleChoiceDataPropType), totalStudentCount: PropTypes.number, totalStudentSubmissions: PropTypes.number, + studentId: PropTypes.number, }; render() { - const {questionAnswerData, totalStudentCount, totalStudentSubmissions} = this.props; + const {questionAnswerData, totalStudentCount, totalStudentSubmissions, studentId} = this.props; return (
    - {questionAnswerData.length > 0 && + {(questionAnswerData.length > 0 && studentId === ALL_STUDENT_FILTER) &&

    {i18n.multipleChoiceQuestionsOverview({ @@ -44,4 +46,5 @@ export default connect(state => ({ questionAnswerData: getMultipleChoiceSectionSummary(state), totalStudentSubmissions: countSubmissionsForCurrentAssessment(state), totalStudentCount: getTotalStudentCount(state), + studentId: state.sectionAssessments.studentId, }))(MCAssessmentsOverviewContainer); From d9beaebb78bc1e544cb6f138b7826f8ca54a2a64 Mon Sep 17 00:00:00 2001 From: Caley Brock Date: Thu, 5 Jul 2018 11:17:29 -0700 Subject: [PATCH 48/89] Filter free response qs by student id --- .../sectionAssessments/sectionAssessmentsRedux.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js b/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js index cb492dbcc231a..cedda60db9f4a 100644 --- a/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js +++ b/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js @@ -284,8 +284,14 @@ export const getAssessmentsFreeResponseResults = (state) => { const studentResponses = getAssessmentResponsesForCurrentScript(state); + let currentStudentsIds = Object.keys(studentResponses); + // Filter by current selected student. + if (state.sectionAssessments.studentId !== ALL_STUDENT_FILTER) { + currentStudentsIds = [state.sectionAssessments.studentId]; + } + // For each student, look up their responses to the currently selected assessment. - Object.keys(studentResponses).forEach(studentId => { + currentStudentsIds.forEach(studentId => { studentId = (parseInt(studentId, 10)); const studentObject = studentResponses[studentId]; const currentAssessmentId = state.sectionAssessments.assessmentId; From 6b779947211995f309de9982206e59d3fe5030cd Mon Sep 17 00:00:00 2001 From: Suresh Chanmugam Date: Thu, 5 Jul 2018 11:25:42 -0700 Subject: [PATCH 49/89] Add to crontab, add error handling, and add logic to copy ssh config to support fanout. --- .../restart_high_memory_frontend_services | 114 +++++++++--------- .../cdo-apps/templates/default/crontab.erb | 4 + 2 files changed, 64 insertions(+), 54 deletions(-) diff --git a/bin/cron/restart_high_memory_frontend_services b/bin/cron/restart_high_memory_frontend_services index a9785ab883058..71b6d3803f1e7 100755 --- a/bin/cron/restart_high_memory_frontend_services +++ b/bin/cron/restart_high_memory_frontend_services @@ -4,72 +4,78 @@ require_relative '../../dashboard/config/environment' require_relative '../../lib/cdo/only_one' require 'aws-sdk' require 'cdo/chat_client' +require 'fileutils' require 'sshkit' def main ChatClient.message 'infra-production', 'Beginning to find and restart front end services with high memory utilization' - ec2_resource = Aws::EC2::Resource.new - cloudwatch_resource = Aws::CloudWatch::Resource.new - frontend_servers_to_restart = Array.new - production_frontends = ec2_resource.instances( - { - filters: [ - { - name: 'tag:environment', - values: [ - 'production' - ] - }, - { - name: 'tag:aws:cloudformation:logical-id', - values: [ - 'Frontends' - ] - }, - { - name: 'instance-state-name', - values: [ - 'running' - ] - } - ] - } - ) - ChatClient.message 'infra-production', "Found #{production_frontends.count} production front end webservers" - production_frontends.each do |instance| - # A new may not have reported MemoryUtilization metrics yet. - memory_utilization = cloudwatch_resource.metric('System/Linux', 'MemoryUtilization').get_statistics( + begin + ec2_resource = Aws::EC2::Resource.new + cloudwatch_resource = Aws::CloudWatch::Resource.new + frontend_servers_to_restart = Array.new + + production_frontends = ec2_resource.instances( { - dimensions: [ + filters: [ + { + name: 'tag:environment', + values: [ + 'production' + ] + }, { - name: 'InstanceId', - value: instance.instance_id, + name: 'tag:aws:cloudformation:logical-id', + values: [ + 'Frontends' + ] + }, + { + name: 'instance-state-name', + values: [ + 'running' + ] } - ], - start_time: Time.now - 5.minutes, - end_time: Time.now, - period: 5.minutes, - statistics: ['Average'], - unit: 'Percent' + ] } - ).datapoints[0]&.average - uptime_days = (Time.now - instance.launch_time) / (60 * 60 * 24) - ChatClient.message 'infra-production', "Instance ID - #{instance.instance_id} / Memory Utilization - #{memory_utilization&.round}% / Uptime (Days) - #{uptime_days.round(2)}" - frontend_servers_to_restart << instance.private_dns_name if uptime_days > 1 - end + ) + ChatClient.message 'infra-production', "Found #{production_frontends.count} production front end webservers" + production_frontends.each do |instance| + memory_utilization = cloudwatch_resource.metric('System/Linux', 'MemoryUtilization').get_statistics( + { + dimensions: [ + { + name: 'InstanceId', + value: instance.instance_id, + } + ], + start_time: Time.now - 5.minutes, + end_time: Time.now, + period: 5.minutes, + statistics: ['Average'], + unit: 'Percent' + } + # Use safe navigation operator because a new instance may not have reported this metric yet. + ).datapoints[0]&.average + uptime_days = (Time.now - instance.launch_time) / (60 * 60 * 24) + ChatClient.message 'infra-production', "Instance ID - #{instance.instance_id} / Memory Utilization - #{memory_utilization&.round}% / Uptime (Days) - #{uptime_days.round(2)}" + frontend_servers_to_restart << instance.private_dns_name if uptime_days > 1 + end - delay_per_group = 10.minutes - group_size = 1 - restart_command = 'sudo service dashboard upgrade && sudo service pegasus upgrade' + delay_per_group = 10.minutes + group_size = 1 + restart_command = 'sudo service dashboard upgrade && sudo service pegasus upgrade' - # TODO: (suresh) Add system call to copy the ssh config files needed for SSHKit to work? + # Copy SSH configuration needed to "fanout" to each front end server. + FileUtils.cp('~/.ssh/config_fanout', '~/.ssh/config') - SSHKit::Backend::Netssh.configure {|ssh| ssh.ssh_options = {paranoid: false}} - SSHKit::Coordinator.new(frontend_servers_to_restart).each(in: :sequence, wait: delay_per_group, limit: group_size) do - ChatClient.message 'infra-production', capture(restart_command, raise_on_non_zero_exit: false) + SSHKit::Backend::Netssh.configure {|ssh| ssh.ssh_options = {paranoid: false}} + SSHKit::Coordinator.new(frontend_servers_to_restart).each(in: :sequence, wait: delay_per_group, limit: group_size) do + ChatClient.message 'infra-production', capture(restart_command, raise_on_non_zero_exit: false) + end + ChatClient.message 'infra-production', 'Done finding and restarting front end services with high memory utilization' + rescue StandardError => error + ChatClient.message 'infra-production', "Error finding/restarting frond end services with high memory utilization - #{error.message}" end - - ChatClient.message 'infra-production', 'Done finding and restarting front end services with high memory utilization' end main if only_one_running?(__FILE__) diff --git a/cookbooks/cdo-apps/templates/default/crontab.erb b/cookbooks/cdo-apps/templates/default/crontab.erb index 77f05e7336019..d93cae2686cc1 100644 --- a/cookbooks/cdo-apps/templates/default/crontab.erb +++ b/cookbooks/cdo-apps/templates/default/crontab.erb @@ -99,6 +99,10 @@ cronjob at:'0 10 * * *', do:deploy_dir('bin', 'cron', 'regional_partner_reporting') cronjob at:'1 7 * * 6', do:deploy_dir('bin', 'cron', 'cleanup_workshop_attendance_codes') + # 4AM UTC is 8 or 9PM PT, a low traffic time when engineers are still awake to monitor any issues that occur + # during the automated restarts. + cronjob at:'00 4 * * *', do:deploy_dir('bin', 'cron', 'restart_high_memory_frontend_services') + # RDS backup window is 08:50-09:20, so by 11:50 backups should definitely be ready cronjob at:'50 11 * * *', do:deploy_dir('bin', 'cron', 'push_latest_rds_backup_to_secondary_account') end From f6f4e8bca7ae5e8303444621d92760d636f6f9d6 Mon Sep 17 00:00:00 2001 From: Caley Brock Date: Thu, 5 Jul 2018 11:27:59 -0700 Subject: [PATCH 50/89] Filter individual MC responses by student id --- .../MultipleChoiceByStudentContainer.jsx | 25 ++++++++++++------- .../sectionAssessments/SectionAssessments.jsx | 2 +- .../sectionAssessmentsRedux.js | 8 +++++- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/apps/src/templates/sectionAssessments/MultipleChoiceByStudentContainer.jsx b/apps/src/templates/sectionAssessments/MultipleChoiceByStudentContainer.jsx index c65da8defc97b..d3ac5508a70cd 100644 --- a/apps/src/templates/sectionAssessments/MultipleChoiceByStudentContainer.jsx +++ b/apps/src/templates/sectionAssessments/MultipleChoiceByStudentContainer.jsx @@ -4,6 +4,7 @@ import { studentWithResponsesPropType, multipleChoiceQuestionPropType } from './ import { getMultipleChoiceStructureForCurrentAssessment, getStudentMCResponsesForCurrentAssessment, + ALL_STUDENT_FILTER, } from './sectionAssessmentsRedux'; import i18n from "@cdo/locale"; import { connect } from 'react-redux'; @@ -12,21 +13,26 @@ class MultipleChoiceByStudentContainer extends Component { static propTypes = { multipleChoiceStructure: PropTypes.arrayOf(multipleChoiceQuestionPropType), studentAnswerData: PropTypes.arrayOf(studentWithResponsesPropType), + studentId: PropTypes.number, }; render() { - const {multipleChoiceStructure, studentAnswerData} = this.props; + const {multipleChoiceStructure, studentAnswerData, studentId} = this.props; return (
    - {studentAnswerData.map((studentResponse, index) => ( -
    -

    {i18n.multipleChoiceStudentOverview({studentName: studentResponse.name})}

    - + {studentId !== ALL_STUDENT_FILTER && +
    + {studentAnswerData.map((studentResponse, index) => ( +
    +

    {i18n.multipleChoiceStudentOverview({studentName: studentResponse.name})}

    + +
    + ))}
    - ))} + }
    ); } @@ -37,4 +43,5 @@ export const UnconnectedMultipleChoiceByStudentContainer = MultipleChoiceByStude export default connect(state => ({ multipleChoiceStructure: getMultipleChoiceStructureForCurrentAssessment(state), studentAnswerData: getStudentMCResponsesForCurrentAssessment(state), + studentId: state.sectionAssessments.studentId, }))(MultipleChoiceByStudentContainer); diff --git a/apps/src/templates/sectionAssessments/SectionAssessments.jsx b/apps/src/templates/sectionAssessments/SectionAssessments.jsx index 515d586862c3c..c72a37e62a754 100644 --- a/apps/src/templates/sectionAssessments/SectionAssessments.jsx +++ b/apps/src/templates/sectionAssessments/SectionAssessments.jsx @@ -136,8 +136,8 @@ class SectionAssessments extends Component { {totalStudentSubmissions > 0 &&
    - +
    }
    diff --git a/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js b/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js index cedda60db9f4a..a7c27644433fe 100644 --- a/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js +++ b/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js @@ -232,7 +232,13 @@ export const getStudentMCResponsesForCurrentAssessment = (state) => { return []; } - const studentResponsesArray = Object.keys(studentResponses).map(studentId => { + let currentStudentsIds = Object.keys(studentResponses); + // Filter by current selected student. + if (state.sectionAssessments.studentId !== ALL_STUDENT_FILTER) { + currentStudentsIds = [state.sectionAssessments.studentId]; + } + + const studentResponsesArray = currentStudentsIds.map(studentId => { studentId = (parseInt(studentId, 10)); const studentObject = studentResponses[studentId]; const currentAssessmentId = state.sectionAssessments.assessmentId; From 91c7d1b6db4d6da116198de95d5ed249e49e908b Mon Sep 17 00:00:00 2001 From: Mehal Shah Date: Thu, 5 Jul 2018 11:31:40 -0700 Subject: [PATCH 51/89] Discard environment change --- dashboard/app/models/concerns/pd/jot_form_backed_form.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dashboard/app/models/concerns/pd/jot_form_backed_form.rb b/dashboard/app/models/concerns/pd/jot_form_backed_form.rb index 6a03601cda929..65e16d2a7e0db 100644 --- a/dashboard/app/models/concerns/pd/jot_form_backed_form.rb +++ b/dashboard/app/models/concerns/pd/jot_form_backed_form.rb @@ -128,8 +128,7 @@ def skip_submission?(form_id, processed_answers) # Skip other environments, and anything without an environment value. # Only keep this environment. - #return true if environment != Rails.env - return true unless environment + return true if environment != Rails.env # Is it a duplicate? These will be prevented in the future, but for now log and skip # TODO(Andrew): prevent duplicates and remove this code. From 14cae046e4978aec5a97540783b22d2b9f848144 Mon Sep 17 00:00:00 2001 From: Mehal Shah Date: Thu, 5 Jul 2018 11:40:26 -0700 Subject: [PATCH 52/89] Remove some debugging code --- .../local_summer_workshop_daily_survey/results.jsx | 8 -------- 1 file changed, 8 deletions(-) diff --git a/apps/src/code-studio/pd/workshop_dashboard/reports/local_summer_workshop_daily_survey/results.jsx b/apps/src/code-studio/pd/workshop_dashboard/reports/local_summer_workshop_daily_survey/results.jsx index becd284a6080a..b3ba2d9c09cae 100644 --- a/apps/src/code-studio/pd/workshop_dashboard/reports/local_summer_workshop_daily_survey/results.jsx +++ b/apps/src/code-studio/pd/workshop_dashboard/reports/local_summer_workshop_daily_survey/results.jsx @@ -45,14 +45,6 @@ export default class Results extends React.Component { key={i} /> ); - } else { - // DO NOT CHECK IN. Debugging item right now - return ( -

    - {question['text']} - {question['answer_type']} -

    - ); } })); } From 4a0c47b3d4042de5c76263a02bb944c6630a55c1 Mon Sep 17 00:00:00 2001 From: Maddie Kasula Date: Thu, 5 Jul 2018 12:37:32 -0700 Subject: [PATCH 53/89] Restore navigateToHref stub to avoid breaking other tests --- .../unit/lib/ui/accounts/ManageLinkedAccountsControllerTest.js | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/test/unit/lib/ui/accounts/ManageLinkedAccountsControllerTest.js b/apps/test/unit/lib/ui/accounts/ManageLinkedAccountsControllerTest.js index b738de896ae22..e7e6ae52cc549 100644 --- a/apps/test/unit/lib/ui/accounts/ManageLinkedAccountsControllerTest.js +++ b/apps/test/unit/lib/ui/accounts/ManageLinkedAccountsControllerTest.js @@ -44,6 +44,7 @@ describe('ManageLinkedAccountsController', () => { let arg = navigateToHrefStub.getCall(0).args[0]; expect(navigateToHrefStub).to.have.been.calledOnce; expect(arg).to.equal('/users/auth/google_oauth2/connect'); + utils.navigateToHref.restore(); }); }); From fc86a279c6fed741bd786c291e430943729f071c Mon Sep 17 00:00:00 2001 From: nkiru onwuneme Date: Thu, 5 Jul 2018 12:52:45 -0700 Subject: [PATCH 54/89] Add padding to the checkmark icon --- .../sectionAssessments/MultipleChoiceAnswerCell.jsx | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/apps/src/templates/sectionAssessments/MultipleChoiceAnswerCell.jsx b/apps/src/templates/sectionAssessments/MultipleChoiceAnswerCell.jsx index a0f70d0aff024..e0d9909e94e32 100644 --- a/apps/src/templates/sectionAssessments/MultipleChoiceAnswerCell.jsx +++ b/apps/src/templates/sectionAssessments/MultipleChoiceAnswerCell.jsx @@ -12,11 +12,13 @@ const styles = { }, icon: { color: color.level_perfect, + paddingRight: 5, + fontSize: 20, }, - text: { + value: { color: color.charcoal, fontFamily: '"Gotham 5r", sans-serif', - marginRight: 10, + marginRight: 15, marginLeft: 10, }, }; @@ -32,13 +34,13 @@ class MultipleChoiceAnswerCell extends Component { render() { const {percentValue, isCorrectAnswer, displayAnswer, opacity} = this.props; - const rgbaValue = (isCorrectAnswer) ? {backgroundColor: `rgba(14, 190, 14, ${opacity})`} : + const rgbaValue = (isCorrectAnswer) ? {backgroundColor: `rgba(159, 212, 159, ${opacity})`} : {backgroundColor: `rgba(255, 99, 71, ${opacity})`}; if (displayAnswer) { return (
    -
    +
    {displayAnswer}
    @@ -52,7 +54,7 @@ class MultipleChoiceAnswerCell extends Component { return (
    -
    +
    {(percentValue >= 0) && {`${percentValue}%`} } From f2ce39a228513994b1ee3ac0999164d8dff96d22 Mon Sep 17 00:00:00 2001 From: eric Date: Thu, 5 Jul 2018 13:16:40 -0700 Subject: [PATCH 55/89] PR feedback --- .../controllers/omniauth_callbacks_controller.rb | 16 ++++++++++++---- dashboard/app/models/authentication_option.rb | 6 ++++++ .../omniauth_callbacks_controller_test.rb | 8 ++++---- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/dashboard/app/controllers/omniauth_callbacks_controller.rb b/dashboard/app/controllers/omniauth_callbacks_controller.rb index dd6478749c69f..f9874983d0b1e 100644 --- a/dashboard/app/controllers/omniauth_callbacks_controller.rb +++ b/dashboard/app/controllers/omniauth_callbacks_controller.rb @@ -1,4 +1,5 @@ require 'cdo/shared_cache' +require 'honeybadger' class OmniauthCallbacksController < Devise::OmniauthCallbacksController include UsersHelper @@ -178,7 +179,7 @@ def silent_takeover(oauth_user, auth_hash) # Copy oauth details to primary account @user = User.find_by_email_or_hashed_email(oauth_user.email) if @user.migrated? - AuthenticationOption.new( + success = AuthenticationOption.create( user: @user, email: oauth_user.email, hashed_email: oauth_user.hashed_email, @@ -189,7 +190,14 @@ def silent_takeover(oauth_user, auth_hash) oauth_token_expiration: auth_hash.credentials&.expires_at, oauth_refresh_token: auth_hash.credentials&.refresh_token } - ).save! + ) + unless success + # This should never happen if other logic is working correctly, so notify + Honeybadger.notify( + error_class: 'Failed to create AuthenticationOption during silent takeover', + error_message: "Could not create AuthenticationOption during silent takeover for user with email #{oauth_user.email}" + ) + end else @user.provider = oauth_user.provider @user.uid = oauth_user.uid @@ -206,8 +214,8 @@ def sign_in_user end def allows_silent_takeover(oauth_user, auth_hash) - allow_takeover = oauth_user.provider.present? - allow_takeover &= %w(facebook google_oauth2 windowslive).include?(auth_hash.provider.to_s) + allow_takeover = auth_hash.provider.present? + allow_takeover &= AuthenticationOption::SILENT_TAKEOVER_CREDENTIAL_TYPES.include?(auth_hash.provider.to_s) lookup_user = User.find_by_email_or_hashed_email(oauth_user.email) allow_takeover && lookup_user end diff --git a/dashboard/app/models/authentication_option.rb b/dashboard/app/models/authentication_option.rb index 13ecb00733501..8fd548903f62f 100644 --- a/dashboard/app/models/authentication_option.rb +++ b/dashboard/app/models/authentication_option.rb @@ -48,6 +48,12 @@ class AuthenticationOption < ApplicationRecord OAUTH_CREDENTIAL_TYPES, ].flatten + SILENT_TAKEOVER_CREDENTIAL_TYPES = [ + FACEBOOK, + GOOGLE, + WINDOWS_LIVE + ] + def oauth? OAUTH_CREDENTIAL_TYPES.include? credential_type end diff --git a/dashboard/test/controllers/omniauth_callbacks_controller_test.rb b/dashboard/test/controllers/omniauth_callbacks_controller_test.rb index ab0be458aa7fa..1e0e176a7569d 100644 --- a/dashboard/test/controllers/omniauth_callbacks_controller_test.rb +++ b/dashboard/test/controllers/omniauth_callbacks_controller_test.rb @@ -496,7 +496,7 @@ class OmniauthCallbacksControllerTest < ActionController::TestCase test 'connect_provider: can connect multiple auth options with the same email to the same user' do email = 'test@xyz.foo' user = create :user, :multi_auth_migrated, uid: 'some-uid' - AuthenticationOption.new( + AuthenticationOption.create!( { user: user, email: email, @@ -509,7 +509,7 @@ class OmniauthCallbacksControllerTest < ActionController::TestCase oauth_refresh_token: 'fake_refresh_token' } } - ).save! + ) auth = generate_auth_user_hash(provider: 'facebook', uid: user.uid, refresh_token: '65432', email: email) @request.env['omniauth.auth'] = auth @@ -529,7 +529,7 @@ class OmniauthCallbacksControllerTest < ActionController::TestCase test 'connect_provider: cannot connect multiple auth options with the same email to a different user' do email = 'test@xyz.foo' user_a = create :user, :multi_auth_migrated - AuthenticationOption.new( + AuthenticationOption.create!( { user: user_a, email: email, @@ -542,7 +542,7 @@ class OmniauthCallbacksControllerTest < ActionController::TestCase oauth_refresh_token: 'fake_refresh_token' } } - ).save! + ) user_b = create :user, :multi_auth_migrated auth = generate_auth_user_hash(provider: 'facebook', uid: 'some-other-uid', refresh_token: '65432', email: email) From 77cc5e3d44959b6a9f421ec4a1a99649ae6e0d7c Mon Sep 17 00:00:00 2001 From: Caley Brock Date: Thu, 5 Jul 2018 13:20:12 -0700 Subject: [PATCH 56/89] No need to loop through an array of one object --- .../MultipleChoiceByStudentContainer.jsx | 16 +++--- .../sectionAssessmentsRedux.js | 54 +++++++++---------- 2 files changed, 30 insertions(+), 40 deletions(-) diff --git a/apps/src/templates/sectionAssessments/MultipleChoiceByStudentContainer.jsx b/apps/src/templates/sectionAssessments/MultipleChoiceByStudentContainer.jsx index d3ac5508a70cd..2f751dc1afa5b 100644 --- a/apps/src/templates/sectionAssessments/MultipleChoiceByStudentContainer.jsx +++ b/apps/src/templates/sectionAssessments/MultipleChoiceByStudentContainer.jsx @@ -12,7 +12,7 @@ import { connect } from 'react-redux'; class MultipleChoiceByStudentContainer extends Component { static propTypes = { multipleChoiceStructure: PropTypes.arrayOf(multipleChoiceQuestionPropType), - studentAnswerData: PropTypes.arrayOf(studentWithResponsesPropType), + studentAnswerData: studentWithResponsesPropType, studentId: PropTypes.number, }; @@ -22,15 +22,11 @@ class MultipleChoiceByStudentContainer extends Component {
    {studentId !== ALL_STUDENT_FILTER &&
    - {studentAnswerData.map((studentResponse, index) => ( -
    -

    {i18n.multipleChoiceStudentOverview({studentName: studentResponse.name})}

    - -
    - ))} +

    {i18n.multipleChoiceStudentOverview({studentName: studentAnswerData.name})}

    +
    }
    diff --git a/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js b/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js index a7c27644433fe..cede3896afe76 100644 --- a/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js +++ b/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js @@ -229,42 +229,36 @@ export const getMultipleChoiceStructureForCurrentAssessment = (state) => { export const getStudentMCResponsesForCurrentAssessment = (state) => { const studentResponses = getAssessmentResponsesForCurrentScript(state); if (!studentResponses) { - return []; + return {}; } - - let currentStudentsIds = Object.keys(studentResponses); - // Filter by current selected student. - if (state.sectionAssessments.studentId !== ALL_STUDENT_FILTER) { - currentStudentsIds = [state.sectionAssessments.studentId]; + const studentId = state.sectionAssessments.studentId; + const studentObject = studentResponses[studentId]; + if (!studentObject) { + return {}; } - const studentResponsesArray = currentStudentsIds.map(studentId => { - studentId = (parseInt(studentId, 10)); - const studentObject = studentResponses[studentId]; - const currentAssessmentId = state.sectionAssessments.assessmentId; - const studentAssessment = studentObject.responses_by_assessment[currentAssessmentId]; + const currentAssessmentId = state.sectionAssessments.assessmentId; + const studentAssessment = studentObject.responses_by_assessment[currentAssessmentId]; - // If the student has not submitted this assessment, don't display results. - if (!studentAssessment) { - return; - } + // If the student has not submitted this assessment, don't display results. + if (!studentAssessment) { + return {}; + } - // Transform that data into what we need for this particular table, in this case - // is the structure studentAnswerDataPropType - return { - id: studentId, - name: studentObject.student_name, - studentResponses: studentAssessment.level_results.filter(answer => answer.type === QuestionType.MULTI) - .map(answer => { - return { - responses: indexesToAnswerString(answer.student_result), - isCorrect: answer.status === MultiAnswerStatus.CORRECT, - }; - }) - }; - }).filter(studentData => studentData); + // Transform that data into what we need for this particular table, in this case + // is the structure studentAnswerDataPropType + return { + id: studentId, + name: studentObject.student_name, + studentResponses: studentAssessment.level_results.filter(answer => answer.type === QuestionType.MULTI) + .map(answer => { + return { + responses: indexesToAnswerString(answer.student_result), + isCorrect: answer.status === MultiAnswerStatus.CORRECT, + }; + }) + }; - return studentResponsesArray; }; /** From 783833837adfbd20c3b74454934bcb33bc921f41 Mon Sep 17 00:00:00 2001 From: Caley Brock Date: Thu, 5 Jul 2018 13:26:45 -0700 Subject: [PATCH 57/89] Only show mc responses when current student hass responded --- .../MultipleChoiceByStudentContainer.jsx | 7 +++++-- .../sectionAssessments/sectionAssessmentsRedux.js | 5 +++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/apps/src/templates/sectionAssessments/MultipleChoiceByStudentContainer.jsx b/apps/src/templates/sectionAssessments/MultipleChoiceByStudentContainer.jsx index 2f751dc1afa5b..be1b3d64e48f5 100644 --- a/apps/src/templates/sectionAssessments/MultipleChoiceByStudentContainer.jsx +++ b/apps/src/templates/sectionAssessments/MultipleChoiceByStudentContainer.jsx @@ -5,6 +5,7 @@ import { getMultipleChoiceStructureForCurrentAssessment, getStudentMCResponsesForCurrentAssessment, ALL_STUDENT_FILTER, + currentStudentHasResponses, } from './sectionAssessmentsRedux'; import i18n from "@cdo/locale"; import { connect } from 'react-redux'; @@ -14,13 +15,14 @@ class MultipleChoiceByStudentContainer extends Component { multipleChoiceStructure: PropTypes.arrayOf(multipleChoiceQuestionPropType), studentAnswerData: studentWithResponsesPropType, studentId: PropTypes.number, + currentStudentHasResponses: PropTypes.bool, }; render() { - const {multipleChoiceStructure, studentAnswerData, studentId} = this.props; + const {multipleChoiceStructure, studentAnswerData, studentId, currentStudentHasResponses} = this.props; return (
    - {studentId !== ALL_STUDENT_FILTER && + {(studentId !== ALL_STUDENT_FILTER && currentStudentHasResponses) &&

    {i18n.multipleChoiceStudentOverview({studentName: studentAnswerData.name})}

    ({ multipleChoiceStructure: getMultipleChoiceStructureForCurrentAssessment(state), studentAnswerData: getStudentMCResponsesForCurrentAssessment(state), studentId: state.sectionAssessments.studentId, + currentStudentHasResponses: currentStudentHasResponses(state), }))(MultipleChoiceByStudentContainer); diff --git a/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js b/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js index cede3896afe76..ee03fdfab9005 100644 --- a/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js +++ b/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js @@ -636,6 +636,11 @@ export const getExportableAssessmentData = (state) => { return responses; }; +// TODO(write comment) +export const currentStudentHasResponses = (state) => { + return !!getAssessmentResponsesForCurrentScript(state).hasOwnProperty(state.sectionAssessments.studentId); +}; + // Helpers /** From 662da5145388ab5e5b418f55916ddc1b79d55f10 Mon Sep 17 00:00:00 2001 From: Mehal Shah Date: Thu, 5 Jul 2018 13:45:42 -0700 Subject: [PATCH 58/89] PR feedback --- .../reports/local_summer_workshop_daily_survey/results.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/src/code-studio/pd/workshop_dashboard/reports/local_summer_workshop_daily_survey/results.jsx b/apps/src/code-studio/pd/workshop_dashboard/reports/local_summer_workshop_daily_survey/results.jsx index b3ba2d9c09cae..efa59d623a13a 100644 --- a/apps/src/code-studio/pd/workshop_dashboard/reports/local_summer_workshop_daily_survey/results.jsx +++ b/apps/src/code-studio/pd/workshop_dashboard/reports/local_summer_workshop_daily_survey/results.jsx @@ -27,7 +27,7 @@ export default class Results extends React.Component { return null; } - if (question['answer_type'] === 'scale'|| question['answer_type'] === 'singleSelect') { + if (['scale', 'singleSelect'].includes(question['answer_type'])) { return ( Date: Thu, 5 Jul 2018 13:54:55 -0700 Subject: [PATCH 59/89] Better assertion that student age is unchanged in second test --- .../controllers/registrations_controller/set_age_test.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/dashboard/test/controllers/registrations_controller/set_age_test.rb b/dashboard/test/controllers/registrations_controller/set_age_test.rb index 77b6bf2bba0c1..03473206bf4ba 100644 --- a/dashboard/test/controllers/registrations_controller/set_age_test.rb +++ b/dashboard/test/controllers/registrations_controller/set_age_test.rb @@ -13,12 +13,15 @@ class SetAgeTest < ActionDispatch::IntegrationTest test "set_age does nothing if user age is already set" do User.any_instance.expects(:update).never - student = create :student - refute student.age.blank? + student = create :student, age: 18 + assert_equal 18, student.age sign_in student patch '/users/set_age', params: {user: {age: '20'}} assert_response :success + + student.reload + assert_equal 18, student.age end test "set_age sets age if user is signed in and age is blank" do From 8c60d904a56831fd6791f45e184a426e7803e625 Mon Sep 17 00:00:00 2001 From: Maddie Kasula Date: Thu, 5 Jul 2018 14:05:43 -0700 Subject: [PATCH 60/89] Add google_classroom_student? and clever_student? --- dashboard/app/models/user.rb | 10 ++++++++++ .../app/views/devise/registrations/edit.html.haml | 2 ++ 2 files changed, 12 insertions(+) diff --git a/dashboard/app/models/user.rb b/dashboard/app/models/user.rb index 5db8b7ad00295..287de393bf8a5 100644 --- a/dashboard/app/models/user.rb +++ b/dashboard/app/models/user.rb @@ -842,6 +842,16 @@ def secret_picture_account_only? any_sections && sections_as_student.all? {|section| section.login_type == Section::LOGIN_TYPE_PICTURE} end + # True if user is a student in a section that has Google Classroom login type + def google_classroom_student? + sections_as_student.find_by_login_type(Section::LOGIN_TYPE_GOOGLE_CLASSROOM).present? + end + + # True if user is a student in a section that has Clever login type + def clever_student? + sections_as_student.find_by_login_type(Section::LOGIN_TYPE_CLEVER).present? + end + # overrides Devise::Authenticatable#find_first_by_auth_conditions # see https://github.com/plataformatec/devise/blob/master/lib/devise/models/authenticatable.rb#L245 def self.find_for_authentication(tainted_conditions) diff --git a/dashboard/app/views/devise/registrations/edit.html.haml b/dashboard/app/views/devise/registrations/edit.html.haml index 0e0647eec5d2b..0125861adc5b6 100644 --- a/dashboard/app/views/devise/registrations/edit.html.haml +++ b/dashboard/app/views/devise/registrations/edit.html.haml @@ -211,6 +211,8 @@ isOauth: current_user.oauth?, isPasswordRequired: current_user.encrypted_password.present?, authenticationOptions: current_user.authentication_options.map(&:summarize), + isGoogleClassroomStudent: current_user.google_classroom_student?, + isCleverStudent: current_user.clever_student?, }.to_json } %script{src: minifiable_asset_path('js/devise/registrations/edit.js'), data: script_data} From fcea978407a523d17b3384c327ccf7611e738f2f Mon Sep 17 00:00:00 2001 From: Maddie Kasula Date: Thu, 5 Jul 2018 14:07:21 -0700 Subject: [PATCH 61/89] Implement cannotDisconnect for all providers --- .../lib/ui/accounts/ManageLinkedAccounts.jsx | 38 +++++++++++++++++++ .../ManageLinkedAccountsController.js | 8 +++- .../studio/pages/devise/registrations/edit.js | 12 +++++- 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx b/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx index 4813f679e16e9..1ff2cc8396961 100644 --- a/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx +++ b/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx @@ -25,6 +25,9 @@ export default class ManageLinkedAccounts extends React.Component { })).isRequired, connect: PropTypes.func.isRequired, disconnect: PropTypes.func.isRequired, + userHasPassword: PropTypes.bool.isRequired, + isGoogleClassroomStudent: PropTypes.bool.isRequired, + isCleverStudent: PropTypes.bool.isRequired, }; getAuthenticationOption = (provider) => { @@ -57,6 +60,37 @@ export default class ManageLinkedAccounts extends React.Component { console.log(error.message); }; + cannotDisconnectGoogle = () => { + // Cannot disconnect from Google if student is in a Google Classroom section + return (this.getAuthenticationOption(OAUTH_PROVIDERS.GOOGLE) && this.props.isGoogleClassroomStudent) || + this.cannotDisconnect(OAUTH_PROVIDERS.GOOGLE); + }; + + cannotDisconnectClever = () => { + // Cannot disconnect from Clever if student is in a Clever section + return (this.getAuthenticationOption(OAUTH_PROVIDERS.CLEVER) && this.props.isCleverStudent) || + this.cannotDisconnect(OAUTH_PROVIDERS.CLEVER); + }; + + cannotDisconnect = (provider) => { + const {authenticationOptions, userHasPassword} = this.props; + const otherAuthOptions = _.reject(authenticationOptions, option => option.credential_type === provider); + + // If it's the user's last authentication option + if (otherAuthOptions.length === 0) { + return true; + } + + // If the user's only other authentication option is an email address, + // a password is required to disconnect + const otherOptionIsEmail = otherAuthOptions.length === 1 && otherAuthOptions[0].credential_type === 'email'; + if (otherOptionIsEmail && !userHasPassword) { + return true; + } + + return false; + }; + render() { return (
    @@ -76,24 +110,28 @@ export default class ManageLinkedAccounts extends React.Component { displayName={i18n.manageLinkedAccounts_google_oauth2()} email={this.getEmailForProvider(OAUTH_PROVIDERS.GOOGLE)} onClick={() => this.toggleProvider(OAUTH_PROVIDERS.GOOGLE)} + cannotDisconnect={this.cannotDisconnectGoogle()} /> this.toggleProvider(OAUTH_PROVIDERS.MICROSOFT)} + cannotDisconnect={this.cannotDisconnect(OAUTH_PROVIDERS.MICROSOFT)} /> this.toggleProvider(OAUTH_PROVIDERS.CLEVER)} + cannotDisconnect={this.cannotDisconnectClever()} /> this.toggleProvider(OAUTH_PROVIDERS.FACEBOOK)} + cannotDisconnect={this.cannotDisconnect(OAUTH_PROVIDERS.FACEBOOK)} /> diff --git a/apps/src/lib/ui/accounts/ManageLinkedAccountsController.js b/apps/src/lib/ui/accounts/ManageLinkedAccountsController.js index ded0cbbe3ed13..e89b87baca976 100644 --- a/apps/src/lib/ui/accounts/ManageLinkedAccountsController.js +++ b/apps/src/lib/ui/accounts/ManageLinkedAccountsController.js @@ -5,10 +5,13 @@ import {navigateToHref} from '@cdo/apps/utils'; import ManageLinkedAccounts from './ManageLinkedAccounts'; export default class ManageLinkedAccountsController { - constructor(mountPoint, userType, authenticationOptions) { + constructor(mountPoint, userType, authenticationOptions, userHasPassword, isGoogleClassroomStudent, isCleverStudent) { this.mountPoint = mountPoint; this.userType = userType; this.authenticationOptions = authenticationOptions; + this.userHasPassword = userHasPassword; + this.isGoogleClassroomStudent = isGoogleClassroomStudent; + this.isCleverStudent = isCleverStudent; this.renderManageLinkedAccounts(); } @@ -19,6 +22,9 @@ export default class ManageLinkedAccountsController { authenticationOptions={this.authenticationOptions} connect={this.connect} disconnect={this.disconnect} + userHasPassword={this.userHasPassword} + isGoogleClassroomStudent={this.isGoogleClassroomStudent} + isCleverStudent={this.isCleverStudent} />, this.mountPoint ); diff --git a/apps/src/sites/studio/pages/devise/registrations/edit.js b/apps/src/sites/studio/pages/devise/registrations/edit.js index 5afcd4e7643d8..5899dea6e647f 100644 --- a/apps/src/sites/studio/pages/devise/registrations/edit.js +++ b/apps/src/sites/studio/pages/devise/registrations/edit.js @@ -8,7 +8,14 @@ import getScriptData from '@cdo/apps/util/getScriptData'; // Values loaded from scriptData are always initial values, not the latest // (possibly unsaved) user-edited values on the form. const scriptData = getScriptData('edit'); -const {userAge, userType, isPasswordRequired, authenticationOptions} = scriptData; +const { + userAge, + userType, + isPasswordRequired, + authenticationOptions, + isGoogleClassroomStudent, + isCleverStudent, +} = scriptData; $(document).ready(() => { new ChangeEmailController({ @@ -37,6 +44,9 @@ $(document).ready(() => { manageLinkedAccountsMountPoint, userType, authenticationOptions, + isPasswordRequired, + isGoogleClassroomStudent, + isCleverStudent, ); } From 09f9c9145303a67a2395fb45ec2723bb65ee15a6 Mon Sep 17 00:00:00 2001 From: Caley Brock Date: Thu, 5 Jul 2018 15:19:15 -0700 Subject: [PATCH 62/89] Force mc tables to re-render when new props passed --- .../SingleStudentAssessmentsMCTable.jsx | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/apps/src/templates/sectionAssessments/SingleStudentAssessmentsMCTable.jsx b/apps/src/templates/sectionAssessments/SingleStudentAssessmentsMCTable.jsx index 25cea6b852671..3d6bfe99e8da4 100644 --- a/apps/src/templates/sectionAssessments/SingleStudentAssessmentsMCTable.jsx +++ b/apps/src/templates/sectionAssessments/SingleStudentAssessmentsMCTable.jsx @@ -85,13 +85,12 @@ class SingleStudentAssessmentsMCTable extends Component { ); }; - studentAnswerColumnFormatter = (studentResponses, {rowData, rowIndex}) => { - const answerData = this.props.studentAnswerData.studentResponses[rowIndex]; + studentAnswerColumnFormatter = (studentAnswer, {rowData, rowIndex}) => { return ( ); }; @@ -166,11 +165,18 @@ class SingleStudentAssessmentsMCTable extends Component { const columns = this.getColumns(sortable); const sortingColumns = this.getSortingColumns(); + const rowData = this.props.questionAnswerData.map((question, index) => { + return { + ...question, + studentAnswer: this.props.studentAnswerData.studentResponses[index], + }; + }); + const sortedRows = sort.sorter({ columns, sortingColumns, sort: orderBy, - })(this.props.questionAnswerData); + })(rowData); return ( Date: Thu, 5 Jul 2018 15:22:26 -0700 Subject: [PATCH 63/89] Remove requires that are no longer required --- apps/src/templates/sectionAssessments/assessmentDataShapes.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/src/templates/sectionAssessments/assessmentDataShapes.js b/apps/src/templates/sectionAssessments/assessmentDataShapes.js index 78bf9ec376342..e6561bb52de6c 100644 --- a/apps/src/templates/sectionAssessments/assessmentDataShapes.js +++ b/apps/src/templates/sectionAssessments/assessmentDataShapes.js @@ -47,9 +47,9 @@ export const studentResponsePropType = PropTypes.shape({ // Represents a single student and a set of the student's answers for // a single assessment's multiple choice questions export const studentWithResponsesPropType = PropTypes.shape({ - id: PropTypes.number.isRequired, + id: PropTypes.number, name: PropTypes.string, - studentResponses: PropTypes.arrayOf(studentResponsePropType).isRequired, + studentResponses: PropTypes.arrayOf(studentResponsePropType), }); // Represents a single multiple choice question structure From 1fd17cdc46509a45893561e5c213fff38a8ab689 Mon Sep 17 00:00:00 2001 From: Brad Buchanan Date: Thu, 5 Jul 2018 15:32:12 -0700 Subject: [PATCH 64/89] New connection tests expect to redirect to edit --- .../test/controllers/omniauth_callbacks_controller_test.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dashboard/test/controllers/omniauth_callbacks_controller_test.rb b/dashboard/test/controllers/omniauth_callbacks_controller_test.rb index 8a5de41412cb8..056e9411ea6ef 100644 --- a/dashboard/test/controllers/omniauth_callbacks_controller_test.rb +++ b/dashboard/test/controllers/omniauth_callbacks_controller_test.rb @@ -521,7 +521,7 @@ class OmniauthCallbacksControllerTest < ActionController::TestCase end user.reload - assert_response :success + assert_redirected_to 'http://test.host/users/edit' assert_equal 2, user.authentication_options.length end end @@ -554,7 +554,8 @@ class OmniauthCallbacksControllerTest < ActionController::TestCase get :facebook end - assert_response 422 + assert_redirected_to 'http://test.host/users/edit' + assert_equal 'Email has already been taken', flash.alert end user_a.reload user_b.reload From 6b84e7cc0f7d33d15bc30f2f3413adbc541b7357 Mon Sep 17 00:00:00 2001 From: Caley Brock Date: Thu, 5 Jul 2018 15:36:46 -0700 Subject: [PATCH 65/89] Don't show free responses when current student has not started --- .../FreeResponsesAssessmentsContainer.jsx | 33 ++++++++++++------- .../sectionAssessmentsRedux.js | 6 +++- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/apps/src/templates/sectionAssessments/FreeResponsesAssessmentsContainer.jsx b/apps/src/templates/sectionAssessments/FreeResponsesAssessmentsContainer.jsx index a915eb141cbf3..e9acf25c87c59 100644 --- a/apps/src/templates/sectionAssessments/FreeResponsesAssessmentsContainer.jsx +++ b/apps/src/templates/sectionAssessments/FreeResponsesAssessmentsContainer.jsx @@ -3,6 +3,8 @@ import FreeResponsesAssessmentsTable from './FreeResponsesAssessmentsTable'; import {freeResponsesDataPropType} from './assessmentDataShapes'; import { getAssessmentsFreeResponseResults, + ALL_STUDENT_FILTER, + currentStudentHasResponses, } from './sectionAssessmentsRedux'; import { connect } from 'react-redux'; import i18n from "@cdo/locale"; @@ -15,24 +17,29 @@ export const freeResponseSummaryPropType = PropTypes.shape({ class FreeResponsesAssessmentsContainer extends Component { static propTypes= { freeResponseQuestions: PropTypes.arrayOf(freeResponseSummaryPropType), + studentId: PropTypes.number, + currentStudentHasResponses: PropTypes.bool, }; render() { - const {freeResponseQuestions} = this.props; - + const {freeResponseQuestions, studentId, currentStudentHasResponses} = this.props; return (
    - {freeResponseQuestions.length > 0 && -

    {i18n.studentFreeResponseAnswers()}

    - } - {freeResponseQuestions.map((question, index) => ( -
    -

    {`${question.questionNumber}. ${question.questionText}`}

    - + {(studentId === ALL_STUDENT_FILTER || currentStudentHasResponses) && +
    + {freeResponseQuestions.length > 0 && +

    {i18n.studentFreeResponseAnswers()}

    + } + {freeResponseQuestions.map((question, index) => ( +
    +

    {`${question.questionNumber}. ${question.questionText}`}

    + +
    + ))}
    - ))} + }
    ); } @@ -42,5 +49,7 @@ export const UnconnectedFreeResponsesAssessmentsContainer = FreeResponsesAssessm export default connect(state => ({ freeResponseQuestions: getAssessmentsFreeResponseResults(state), + studentId: state.sectionAssessments.studentId, + currentStudentHasResponses: currentStudentHasResponses(state), }))(FreeResponsesAssessmentsContainer); diff --git a/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js b/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js index ee03fdfab9005..0c07b4a78d6d7 100644 --- a/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js +++ b/apps/src/templates/sectionAssessments/sectionAssessmentsRedux.js @@ -287,7 +287,11 @@ export const getAssessmentsFreeResponseResults = (state) => { let currentStudentsIds = Object.keys(studentResponses); // Filter by current selected student. if (state.sectionAssessments.studentId !== ALL_STUDENT_FILTER) { - currentStudentsIds = [state.sectionAssessments.studentId]; + if (!currentStudentHasResponses(state)) { + return []; + } else { + currentStudentsIds = [state.sectionAssessments.studentId]; + } } // For each student, look up their responses to the currently selected assessment. From 5d60f7c0a4e3fa693fd0f3d3fb2d3ec7e03f45cf Mon Sep 17 00:00:00 2001 From: Maddie Kasula Date: Thu, 5 Jul 2018 15:44:04 -0700 Subject: [PATCH 66/89] Test google_classroom_student? and clever_student? helpers --- dashboard/test/models/user_test.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/dashboard/test/models/user_test.rb b/dashboard/test/models/user_test.rb index 54de4f2c82126..4b1ac04ceec8b 100644 --- a/dashboard/test/models/user_test.rb +++ b/dashboard/test/models/user_test.rb @@ -1764,6 +1764,28 @@ def update_primary_contact_info_fails_safely_for(user, *params) assert_equal original_primary_contact_info, user.primary_contact_info end + test 'google_classroom_student? is true if user belongs to a google classroom section as a student' do + section = create(:section, login_type: Section::LOGIN_TYPE_GOOGLE_CLASSROOM) + user = create(:follower, section: section).student_user + assert user.google_classroom_student? + end + + test 'google_classroom_student? is false if user does not belong to any google classroom sections as a student' do + user = create(:user) + refute user.google_classroom_student? + end + + test 'clever_student? is true if user belongs to a clever section as a student' do + section = create(:section, login_type: Section::LOGIN_TYPE_CLEVER) + user = create(:follower, section: section).student_user + assert user.clever_student? + end + + test 'clever_student? is false if user does not belong to any clever sections as a student' do + user = create(:user) + refute user.clever_student? + end + test 'track_proficiency adds proficiency if necessary and no hint used' do level_concept_difficulty = create :level_concept_difficulty # Defaults with repeat_loops_{d1,d2,d3,d4,d5}_count = {0,2,0,3,0}. From a77c099309ab346bdb260c2c167b2f4c24c6bbd4 Mon Sep 17 00:00:00 2001 From: Maddie Kasula Date: Thu, 5 Jul 2018 15:48:18 -0700 Subject: [PATCH 67/89] Move all disconnect logic into cannotDisconnect method --- .../lib/ui/accounts/ManageLinkedAccounts.jsx | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx b/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx index 1ff2cc8396961..66a24464fa895 100644 --- a/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx +++ b/apps/src/lib/ui/accounts/ManageLinkedAccounts.jsx @@ -62,20 +62,26 @@ export default class ManageLinkedAccounts extends React.Component { cannotDisconnectGoogle = () => { // Cannot disconnect from Google if student is in a Google Classroom section - return (this.getAuthenticationOption(OAUTH_PROVIDERS.GOOGLE) && this.props.isGoogleClassroomStudent) || - this.cannotDisconnect(OAUTH_PROVIDERS.GOOGLE); + return this.getAuthenticationOption(OAUTH_PROVIDERS.GOOGLE) && this.props.isGoogleClassroomStudent; }; cannotDisconnectClever = () => { // Cannot disconnect from Clever if student is in a Clever section - return (this.getAuthenticationOption(OAUTH_PROVIDERS.CLEVER) && this.props.isCleverStudent) || - this.cannotDisconnect(OAUTH_PROVIDERS.CLEVER); + return this.getAuthenticationOption(OAUTH_PROVIDERS.CLEVER) && this.props.isCleverStudent; }; cannotDisconnect = (provider) => { const {authenticationOptions, userHasPassword} = this.props; const otherAuthOptions = _.reject(authenticationOptions, option => option.credential_type === provider); + if (provider === OAUTH_PROVIDERS.GOOGLE && this.cannotDisconnectGoogle()) { + return true; + } + + if (provider === OAUTH_PROVIDERS.CLEVER && this.cannotDisconnectClever()) { + return true; + } + // If it's the user's last authentication option if (otherAuthOptions.length === 0) { return true; @@ -110,7 +116,7 @@ export default class ManageLinkedAccounts extends React.Component { displayName={i18n.manageLinkedAccounts_google_oauth2()} email={this.getEmailForProvider(OAUTH_PROVIDERS.GOOGLE)} onClick={() => this.toggleProvider(OAUTH_PROVIDERS.GOOGLE)} - cannotDisconnect={this.cannotDisconnectGoogle()} + cannotDisconnect={this.cannotDisconnect(OAUTH_PROVIDERS.GOOGLE)} /> this.toggleProvider(OAUTH_PROVIDERS.CLEVER)} - cannotDisconnect={this.cannotDisconnectClever()} + cannotDisconnect={this.cannotDisconnect(OAUTH_PROVIDERS.CLEVER)} /> Date: Thu, 5 Jul 2018 15:52:03 -0700 Subject: [PATCH 68/89] Update selector styles --- .../sectionAssessments/AssessmentSelector.jsx | 2 +- .../sectionAssessments/SectionAssessments.jsx | 54 +++++++++++++------ 2 files changed, 38 insertions(+), 18 deletions(-) diff --git a/apps/src/templates/sectionAssessments/AssessmentSelector.jsx b/apps/src/templates/sectionAssessments/AssessmentSelector.jsx index 94dfc182e989f..1d7d6b8651664 100644 --- a/apps/src/templates/sectionAssessments/AssessmentSelector.jsx +++ b/apps/src/templates/sectionAssessments/AssessmentSelector.jsx @@ -16,7 +16,7 @@ export default class AssessmentSelector extends Component {