Merge pull request #812 from 18F/develop

Merge in mid-point of 2018-09-25 sprint

api/identification.go

@@ -602,16 +602,19 @@ func (entity *IdentificationSSN) Find(context DatabaseService) error {
 
 // IdentificationContacts represents the payload for the identification contact information section.
 type IdentificationContacts struct {
-	PayloadEmails       Payload `json:"Emails" sql:"-"`
+	PayloadHomeEmail       Payload `json:"HomeEmail" sql:"-"`
+	PayloadWorkEmail       Payload `json:"WorkEmail" sql:"-"`
 	PayloadPhoneNumbers Payload `json:"PhoneNumbers" sql:"-"`
 
 	// Validator specific fields
-	Emails       *Collection `json:"-"`
+	HomeEmail       *Email `json:"-"`
+	WorkEmail       *Email `json:"-"`
 	PhoneNumbers *Collection `json:"-"`
 
 	// Persister specific fields
 	ID             int `json:"-"`
-	EmailsID       int `json:"-" pg:", fk:Emails"`
+	HomeEmailID       int `json:"-" pg:", fk:HomeEmail"`
+	WorkEmailID       int `json:"-" pg:", fk:WorkEmail"`
 	PhoneNumbersID int `json:"-" pg:", fk:PhoneNumbers"`
 }
 
@@ -622,11 +625,17 @@ func (entity *IdentificationContacts) Unmarshal(raw []byte) error {
 		return err
 	}
 
-	emails, err := entity.PayloadEmails.Entity()
+	homeEmail, err := entity.PayloadHomeEmail.Entity()
 	if err != nil {
 		return err
 	}
-	entity.Emails = emails.(*Collection)
+	entity.HomeEmail = homeEmail.(*Email)
+
+	workEmail, err := entity.PayloadWorkEmail.Entity()
+	if err != nil {
+		return err
+	}
+	entity.WorkEmail = workEmail.(*Email)
 
 	phoneNumbers, err := entity.PayloadPhoneNumbers.Entity()
 	if err != nil {
@@ -639,8 +648,11 @@ func (entity *IdentificationContacts) Unmarshal(raw []byte) error {
 
 // Marshal to payload structure
 func (entity *IdentificationContacts) Marshal() Payload {
-	if entity.Emails != nil {
-		entity.PayloadEmails = entity.Emails.Marshal()
+	if entity.HomeEmail != nil {
+		entity.PayloadHomeEmail = entity.HomeEmail.Marshal()
+	}
+	if entity.WorkEmail != nil {
+		entity.PayloadWorkEmail = entity.WorkEmail.Marshal()
 	}
 	if entity.PhoneNumbers != nil {
 		entity.PayloadPhoneNumbers = entity.PhoneNumbers.Marshal()
@@ -652,8 +664,12 @@ func (entity *IdentificationContacts) Marshal() Payload {
 func (entity *IdentificationContacts) Valid() (bool, error) {
 	var stack ErrorStack
 
-	if ok, err := entity.Emails.Valid(); !ok {
-		stack.Append("Emails", err)
+	if ok, err := entity.HomeEmail.Valid(); !ok {
+		stack.Append("HomeEmail", err)
+	}
+
+	if ok, err := entity.WorkEmail.Valid(); !ok {
+		stack.Append("WorkEmail", err)
 	}
 
 	if ok, err := entity.PhoneNumbers.Valid(); !ok {
@@ -675,11 +691,17 @@ func (entity *IdentificationContacts) Save(context DatabaseService, account int)
 		return entity.ID, err
 	}
 
-	emailsID, err := entity.Emails.Save(context, account)
+	homeEmailID, err := entity.HomeEmail.Save(context, account)
+	if err != nil {
+		return homeEmailID, err
+	}
+	entity.HomeEmailID = homeEmailID
+
+	workEmailID, err := entity.WorkEmail.Save(context, account)
 	if err != nil {
-		return emailsID, err
+		return workEmailID, err
 	}
-	entity.EmailsID = emailsID
+	entity.WorkEmailID = workEmailID
 
 	phoneNumbersID, err := entity.PhoneNumbers.Save(context, account)
 	if err != nil {
@@ -712,7 +734,11 @@ func (entity *IdentificationContacts) Delete(context DatabaseService, account in
 		}
 	}
 
-	if _, err := entity.Emails.Delete(context, account); err != nil {
+	if _, err := entity.HomeEmail.Delete(context, account); err != nil {
+		return entity.ID, err
+	}
+
+	if _, err := entity.WorkEmail.Delete(context, account); err != nil {
 		return entity.ID, err
 	}
 
@@ -737,9 +763,16 @@ func (entity *IdentificationContacts) Get(context DatabaseService, account int)
 		}
 	}
 
-	if entity.EmailsID != 0 {
-		entity.Emails = &Collection{ID: entity.EmailsID}
-		if _, err := entity.Emails.Get(context, account); err != nil {
+	if entity.HomeEmailID != 0 {
+		entity.HomeEmail = &Email{ID: entity.HomeEmailID}
+		if _, err := entity.HomeEmail.Get(context, account); err != nil {
+			return entity.ID, err
+		}
+	}
+
+	if entity.WorkEmailID != 0 {
+		entity.WorkEmail = &Email{ID: entity.WorkEmailID}
+		if _, err := entity.WorkEmail.Get(context, account); err != nil {
 			return entity.ID, err
 		}
 	}
@@ -768,11 +801,16 @@ func (entity *IdentificationContacts) SetID(id int) {
 func (entity *IdentificationContacts) Find(context DatabaseService) error {
 	context.Find(&IdentificationContacts{ID: entity.ID}, func(result interface{}) {
 		previous := result.(*IdentificationContacts)
-		if entity.Emails == nil {
-			entity.Emails = &Collection{}
+		if entity.HomeEmail == nil {
+			entity.HomeEmail = &Email{}
+		}
+		entity.HomeEmail.ID = previous.HomeEmailID
+		entity.HomeEmailID = previous.HomeEmailID
+		if entity.WorkEmail == nil {
+			entity.WorkEmail = &Email{}
 		}
-		entity.EmailsID = previous.EmailsID
-		entity.Emails.ID = previous.EmailsID
+		entity.WorkEmail.ID = previous.WorkEmailID
+		entity.WorkEmailID = previous.WorkEmailID
 		if entity.PhoneNumbers == nil {
 			entity.PhoneNumbers = &Collection{}
 		}

api/migrations/20180914153510_add_contact_emails.sql

@@ -0,0 +1,16 @@
+
+-- +goose Up
+-- SQL in section 'Up' is executed when this migration is applied
+-- +goose StatementBegin
+ALTER TABLE identification_contacts DROP COLUMN IF EXISTS emails_id;
+ALTER TABLE identification_contacts ADD COLUMN home_email_id bigint REFERENCES emails(id);
+ALTER TABLE identification_contacts ADD COLUMN work_email_id bigint REFERENCES emails(id);
+-- +goose StatementEnd
+
+-- +goose Down
+-- SQL section 'Down' is executed when this migration is rolled back
+-- +goose StatementBegin
+ALTER TABLE identification_contacts ADD COLUMN emails_id bigint REFERENCES collections(id);
+ALTER TABLE identification_contacts DROP COLUMN IF EXISTS home_email_id;
+ALTER TABLE identification_contacts DROP COLUMN IF EXISTS work_email_id;
+-- +goose StatementEnd

api/pdf/pdf_test.go

@@ -14,14 +14,17 @@ import (
 )
 
 const (
-	packageDir  = "pdf"
-	testdataDir = packageDir + "/testdata"
+	packageDir    = "pdf"
+	testdataDir   = packageDir + "/testdata"
+	fileExtension = ".pdf"
 )
 
 func TestPackage(t *testing.T) {
 	// Change working dir to parent so code under test is
 	// executed in same working directory as in production.
 	os.Chdir("..")
+	// Restore working dir
+	defer os.Chdir(packageDir)
 
 	application := applicationData(t)
 	logger := &mock.LogService{}
@@ -39,25 +42,28 @@ func TestPackage(t *testing.T) {
 			t.Fatalf("Error creating PDF from %s: %s", p.Template, err.Error())
 		}
 
-		rpath := path.Join(testdataDir, p.Name+".pdf")
+		rpath := path.Join(testdataDir, p.Name+fileExtension)
 		reference, err := ioutil.ReadFile(rpath)
 		if err != nil {
 			t.Fatalf("Error reading reference PDF: %s", err.Error())
 		}
 
 		if !bytes.Equal(created, reference) {
-			tmpfile, err := ioutil.TempFile("", p.Name)
+			tmpfile, err := ioutil.TempFile(testdataDir, p.Name+fileExtension)
+			if err != nil {
+				t.Fatalf("Error creating generated PDF: %s", err.Error())
+			}
+			defer tmpfile.Close()
+
+			_, err = tmpfile.Write(created)
 			if err != nil {
-				t.Fatalf("Error saving generated PDF: %s", err.Error())
+				t.Fatalf("Error writing generated PDF: %s", err.Error())
 			}
 
 			t.Errorf("Generated PDF (%s) does not match reference PDF (%s)",
 				tmpfile.Name(), rpath)
 		}
 	}
-
-	// Restore working dir
-	os.Chdir(packageDir)
 }
 
 func applicationData(t *testing.T) map[string]interface{} {

api/pdf/templates/README.md

@@ -2,16 +2,52 @@
 
 No convenient open-source library existed for Go that would allow easy customization of PDFs from a template. A work-around doing basic text substitution was devised, with PDF templates generated from the following process:
 
+Tools:
+* macOS and [OmniGraffle](https://itunes.apple.com/us/app/omnigraffle-7/id1142578753?mt=12)
+* RedHat Enterprise Linux 7 (RHEL7) and [qpdf](https://github.com/qpdf/qpdf). On RHEL7, `qpdf` can be installed via `yum install qpdf`
+
 1. Extract single-page PDFs from multi-page SF-86 PDF.
-2. Using OmniGraffle on macOS, create look-alike forms, starting off with result of PDF import.
+2. Using OmniGraffle, create look-alike forms, starting off with result of PDF import.
 3. Adjust layout of form fields as appropriate to accommodate e-signature legalese, etc.
 4. Add placeholder text with appropriate font and size and length in the area of the form where text will be substituted.
-5. To minimize PDF size, only use Helvetica or Courier. Use Courier for the non-signature auto-populated fields, as a monospace type simplifies layout. Use Helvetica to provide visual contrast for the e-signature fields (full name as signature and date). Tab through all the text in the document and make sure no other fonts are in use.
-5. By default, when exporting to PDF, OmniGraffle will structure the PDF objects in such a way to make basic text substitution difficult (i.e., it positions every character separately). To avoid this, place holder text should be a number string (e.g., `1111111111`) of the same length as the desired fixed width field. OmniGraffle will position the string as a single text object surrounded by parentheses (e.g., `(111111111111)` and will make simple substitution possible. In a later stage, the number string will be replaced with a self-describing field name (e.g., `(SSN         )`)
-6. Export each OmniGraffle file to PDF.
-7. Use `qpdf --qdf` on each PDF to get a text editable file.
-8. OmniGraffle will embed a subset of the TrueType font instead of relying on the default PDF fonts. Replace those with equivalent Type1 directives for `Helvetica`, `Helvetica-Bold`, `Helvetica-Oblique`, and `Courier`.
-9. Replace number strings field placeholders with more self-documenting equivalent (e.g., `SSN`, `FIRST_MIDDLE_LAST`, `DOB`, etc.). Pad the field with the space character to preserve the object and xref byte offsets in the PDF, so we don't have to parse and re-calculate these values at run-time.
-10. Run `fix-qdf` on each file to renumber PDF object.
-11. Run `qpdf --qdf` on each file again to remove dangling PDF object references (e.g., the embedded fonts that are no longer used).
-12. Validate templates with the [3-Heights PDF validator](https://www.pdf-online.com/osa/validate.aspx) or similar.
+5. To minimize PDF size, only use Helvetica or Courier fonts. They are part of the PDF standard. Use Courier for the non-signature auto-populated fields, as a monospace type simplifies layout. Use Helvetica to provide visual contrast for the e-signature fields (full name as signature and date). Tab through all the text in the document and make sure no other fonts are in use.
+6. By default, when exporting to PDF, OmniGraffle will structure the PDF objects in such a way to make basic text substitution difficult (i.e., it positions every character separately). To avoid this, place holder text should be a number string (e.g., `1111111111`) of the same length as the desired fixed width field. OmniGraffle will position the string as a single text object surrounded by parentheses (e.g., `(111111111111)` and will make simple substitution possible. In a later stage, the number string will be replaced with a self-describing field name (e.g., `(SSN         )`)
+7. Export each OmniGraffle file to PDF.
+8. Use `qpdf --qdf` on each PDF to get a text editable file.
+9. OmniGraffle will embed a subset of the TrueType font instead of relying on the default PDF fonts. Replace those with equivalent Type1 directives for `Helvetica`, `Helvetica-Bold`, `Helvetica-Oblique`, and `Courier`, adjusting the object IDs in the below example to match your existing document:
+```
+%% Original object ID: 17 0
+17 0 obj
+<<
+  /BaseFont /Helvetica-Bold
+  /Encoding /MacRomanEncoding
+  /Subtype /Type1
+  /Type /Font
+>>
+endobj
+
+%% Original object ID: 18 0
+18 0 obj
+<<
+  /BaseFont /Helvetica
+  /Encoding /MacRomanEncoding
+  /Subtype /Type1
+  /Type /Font
+>>
+endobj
+
+%% Original object ID: 19 0
+19 0 obj
+<<
+  /BaseFont /Helvetica-Oblique
+  /Encoding /MacRomanEncoding
+  /Subtype /Type1
+  /Type /Font
+>>
+endobj
+```
+10. Replace number strings field placeholders with more self-documenting equivalent (e.g., `SSN`, `FIRST_MIDDLE_LAST`, `DOB`, etc.). Pad the field with the space character to preserve the object and xref byte offsets in the PDF, so we don't have to parse and re-calculate these values at run-time (i.e., field placeholder character length plus trailing space characters should match original character length of number string in OmniGraffle). See `pdf.go` for current list of supported field names and existing template PDFs for how padding works.
+11. Run `fix-qdf` on each file to renumber the internal PDF objects.
+12. Run `qpdf --qdf` on each file again to remove dangling PDF object references (e.g., the embedded fonts that are no longer used).
+13. Validate templates with the [3-Heights PDF validator](https://www.pdf-online.com/osa/validate.aspx) or similar.
+14. When complete, archive the `.graffle` in `api/pdf/graffle` and place the new template `.pdf` in `api/pdf/templates` respectively. Note: if the OmniGraffle file contains an image it will be in the macOS package format (e.g., directory), otherwise it will be a single file.

api/telephone.go

@@ -10,6 +10,7 @@ var (
 	formatTelephoneDomestic      = regexp.MustCompile("\\d{3}-?\\d{3}-?\\d{4}")
 	formatTelephoneInternational = regexp.MustCompile("\\d{3}-?\\d{4}-?\\d{4}")
 	formatTelephoneDSN           = regexp.MustCompile("\\d{3}-?\\d{4}")
+	formatNumberType             = regexp.MustCompile("(Home|Work|Cell|NA|^$)")
 )
 
 // Telephone is a basic input.
@@ -64,8 +65,8 @@ func (entity *Telephone) Valid() (bool, error) {
 		}
 	}
 
-	if strings.TrimSpace(entity.NumberType) == "" {
-		stack.Append("Telephone", ErrFieldRequired{"Telephone number type is required"})
+	if ok := formatNumberType.MatchString(entity.NumberType); !ok {
+		stack.Append("Telephone", ErrFieldInvalid{"Number type is not properly formatted"})
 	}
 
 	return !stack.HasErrors(), stack

api/templates/foreign-business-government-contacts.xml

@@ -41,11 +41,13 @@
         <FutureContactPlans>{{text $Contact.Future}}</FutureContactPlans>
         <Purpose>{{text $Contact.Subsequent}}</Purpose>
       </Contact>
-      <HaveAdditionalEntryAnswer>No</HaveAdditionalEntryAnswer>
-      <SummaryComment></SummaryComment>
       {{end}}
       {{end}}
       {{end}}
+      {{if branchcollectionHas $Item.SubsequentContacts | eq "Yes"}}
+      <HaveAdditionalEntryAnswer>No</HaveAdditionalEntryAnswer>
+      {{end}}
+      <SummaryComment></SummaryComment>
     </SubsequentForeignContacts>
   </GovernmentContact>
   {{end}}

api/templates/foreign-business-job-offers.xml

@@ -31,8 +31,4 @@
   </ForeignJobOffer>
   {{end}}
   {{end}}
-  {{- if branch .props.HasForeignEmployment | eq "Yes"}}
-  <HaveAdditionalEntryAnswer>{{branch .props.List.props.branch}}</HaveAdditionalEntryAnswer>
-  <SummaryComment></SummaryComment>
-  {{end}}
 </ForeignJobOffers>

api/templates/foreign-business-other-employment.xml

@@ -38,8 +38,4 @@
   </OtherForeignEmployment>
   {{end}}
   {{end}}
-  {{- if branch .props.HasForeignVentures | eq "Yes"}}
-  <HaveAdditionalEntryAnswer>{{branch .props.List.props.branch}}</HaveAdditionalEntryAnswer>
-  <SummaryComment></SummaryComment>
-  {{end}}
 </OtherForeignEmployments>

api/templates/foreign-business-sponsored-visits.xml

@@ -43,10 +43,18 @@
       {{name $Item.Name}}
     </Name>
     <Organization>
-      <Address NotApplicable="{{notApplicable $Item.OrganizationAddressNotApplicable}}">
+      {{if notApplicable $Item.OrganizationAddressNotApplicable | eq "True"}}
+      <Address NotApplicable="True"/>
+      {{else}}
+      <Address>
         {{location $Item.OrganizationAddress}}
       </Address>
-      <Name NotApplicable="{{notApplicable $Item.OrganizationNotApplicable}}">{{text $Item.Organization}}</Name>
+      {{end}}
+      {{if notApplicable $Item.OrganizationNotApplicable | eq "True"}}
+      <Name NotApplicable="True"></Name>
+      {{else}}
+      <Name>{{text $Item.Organization}}</Name>
+      {{end}}
     </Organization>
     <Purpose>{{text $Item.Stay}}</Purpose>
     <SponsorshipPurpose>{{text $Item.Sponsorship}}</SponsorshipPurpose>

api/templates/foreign-business-voted.xml

@@ -20,8 +20,4 @@
   </ForeignElection>
   {{end}}
   {{end}}
-  {{- if branch .props.HasForeignVoting | eq "Yes"}}
-  <HaveAdditionalEntryAnswer>{{branch .props.List.props.branch}}</HaveAdditionalEntryAnswer>
-  <SummaryComment></SummaryComment>
-  {{end}}
 </VotedInForeignElections>

api/templates/foreign-contacts.xml

@@ -108,7 +108,7 @@
           {{name $Item.Name}}
         </LegalName>
       </FullName>
-      {{- if $Item.Aliases.props.branch.type}}
+      {{if branchcollectionHas $Item.Aliases | eq "Yes"}}
       <OtherNamesUsed>
         {{range $oindex, $othername := $Item.Aliases.props.items}}
         {{with $Alias := $othername.Item}}