Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

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

Already on GitHub? Sign in to your account

PL: Regional Partner mini contact form #27166

Merged
merged 8 commits into from
Feb 23, 2019

Conversation

breville
Copy link
Member

@breville breville commented Feb 21, 2019

This is a "mini" contact form for a regional partner. It works similarly to the regular "Contact a Regional Partner" form at https://studio.code.org/pd/regional_partner_contact/new but is much easier to fill out. It has four fields - name, email, school zip, and notes - but only email and school zip are required.

It's designed to have as many fields filled out as possible. For example, a signed-in teacher should see all four fields filled out, with name and email coming from the user profile, and school zip coming from the user's associated school.

On the backend, there is a new model (Pd::RegionalPartnerMiniContact) to store the results because it has less to validate than the existing model (Pd::RegionalPartnerContact). It uses the same mailer (Pd::RegionalPartnerContactMailer) but some modifications have been made to cope with the fewer fields available. The submitted school zip code is used to look up the matching regional partner.

The form is designed to work on both dashboard and pegasus. For pegasus, a new dashboard route has been added (/api/v1/users/:user_id/contact_details) to retrieve the contact details that will be pre-populated.

The form is not yet shown publicly. It is available to use on two test URLs:

screenshot:

image

Also there are no tests yet.

'tutorialExplorer',
'regionalPartnerSearch',
'regionalPartnerMiniContact'
)
Copy link
Contributor

Choose a reason for hiding this comment

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

@davidsbailey You've got the most recent knowledge on JS build configuration - does this make sense?

Copy link
Member

Choose a reason for hiding this comment

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

no -- this doesn't look right, because only locale-related files should be added here. were you getting an error without adding these here, @breville ?

Copy link
Member

Choose a reason for hiding this comment

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

sorry... I misread. seems like you are just adding 'regionalPartnerMiniContact' which follows the existing pattern, so this looks good to me. I'm not sure what a better pattern would be but at least this way we only have one pattern to clean up later (if that turns out to be needed).

so this line LGTM

Copy link
Contributor

@islemaster islemaster left a comment

Choose a reason for hiding this comment

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

Awesome!

defaultValue={this.state.notes}
/>
{!this.state.submitting && (
<Button id="submit" onClick={this.submit}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Worth adding a disabled={this.state.submitting} or something?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question. The current implementation just replaces the button with a spinner while submitting (the line above hides the button). I suspect we'll do a UI refresh before launching this and I'll make it nice then.

id="notes"
label="Questions or notes for your local Regional Partner"
type="text"
componentClass="textarea"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is where user enters question which could be in form of a short email. Should we make the text area bigger?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question. We can also address this in a UI pass soon. The control is responsive and so it might depend on how big we want it to be overall. The user can also resize it manually already.

@breville breville merged commit e1c4729 into staging Feb 23, 2019
@breville breville deleted the pl-regional-partner-mini-contact-form branch February 23, 2019 05:55
@sureshc
Copy link
Contributor

sureshc commented Feb 26, 2019

@breville Dropped me a note to ask if these email addresses would inadvertently get collected by our Contact Rollups process. I don't see any reference to this new pd_regional_partner_mini_contacts table in Contact Rollups, nor a reference to the original pd_regional_partner_contacts table. I do recommend, however, that we update the controller to invoke EmailPreference.upsert! with opt_in set to false every time we create a mini contact. That won't override if the email address has already opted in nor will it prevent that email address from opting in in the future. Any address in EmailPreference will get sent to Pardot, but with opt_in set to false that address will NOT be sent emails.

@breville
Copy link
Member Author

This PR should have included #27215.

@breville
Copy link
Member Author

Thanks @sureshc. Addressed in #27269.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants