diff --git a/.babelrc b/.babelrc index 249d7e1f9..f73742065 100644 --- a/.babelrc +++ b/.babelrc @@ -15,6 +15,7 @@ "@validators": "./src/validators", "@views": "./src/views" } - }] + }], + "transform-class-properties" ] } diff --git a/.eslintrc.yml b/.eslintrc.yml index 148d0617c..59dc890ae 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -8,6 +8,7 @@ env: es6: true node: true jest: true +parser: babel-eslint parserOptions: ecmaVersion: 9 sourceType: module diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 000000000..6ca7c4bc5 --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,11 @@ +## Description + +\[Replace this with your description, include Fixes #123 to connect to the github issue\] + +## Checklist for Reveiwer + +- [ ] Review code changes +- [ ] Review changes for Database effects +- [ ] Verify change works in IE browser + +More details about this can be found in [docs/review.md](docs/review.md) diff --git a/api/templates/telephone.xml b/api/templates/telephone.xml index 341e876f1..0e2dad0c8 100644 --- a/api/templates/telephone.xml +++ b/api/templates/telephone.xml @@ -22,4 +22,6 @@ Also, here are some additional findings: {{- if ne .props.extension ""}} {{.props.extension}} {{- end}} +{{- if ne .props.timeOfDay "NA"}} +{{- end}} diff --git a/docs/advanced.md b/docs/advanced.md index 31cd52db8..c079508c6 100644 --- a/docs/advanced.md +++ b/docs/advanced.md @@ -8,6 +8,10 @@ See [documentation](../docs/multi-host.md). See [documentation](../docs/saml.md). +## Changes requiring API and database updates + +See [documentation](../docs/updating-api-db.md). + ## Reset locked app submission In the terminal run the following: @@ -67,10 +71,10 @@ make docs All of the documentation may then be found in the respective directories under `doc/`. - ## Docker The current build process generates a lot of orphaned docker volumes in the host environment over time. Developers must periodically run `docker volume prune` in order to reclaim disk space. For example: + ``` # docker volume prune WARNING! This will remove all volumes not used by at least one container. @@ -94,21 +98,25 @@ Renaming app eqip-prototype-dev to eqip-prototype-dev-venerable in org gsa-acq-e error: Server error, status code: 400, error code: 100002, message: The app name is taken: eqip-prototype-dev-venerable Exited with code 1 ``` + The resolution requires a cloud.gov user, with admin rights on the eApp space (i.e., `gsa-acq-eqip`) to manually delete the app name with the `venerable` suffix, using the [Cloud Foundry command line interface](https://docs.cloudfoundry.org/cf-cli/install-go-cli.html): + ``` cf login -a api.fr.cloud.gov -u INSERT-USERNAME-HERE -o gsa-acq-eqip -s production --sso cf target -s dev cf delete eqip-prototype-dev-venerable ``` + Make sure that the `venerable` application displayed in the error log is the application that is being deleted; it will be a variant of the frontend (`eqip-prototype`) or the backend (`eqip-prototype-api`). The presence of `dev`, `staging`, or no moniker in the application name will indicate whether your target space should be `dev`, `staging`, or `production` respectively. -The application should re-deploy correctly at this point, either on the next commit to the associated GitHub branch, or through a manual rerun of the failed CircleCI job. +The application should re-deploy correctly at this point, either on the next commit to the associated GitHub branch, or through a manual rerun of the failed CircleCI job. ### Purging the database To execute the [database purge script](https://github.com/18F/e-QIP-prototype/blob/develop/docs/test-scenarios.md) against the cloud.gov PostgreSQL, the following approach may be used. It requires `cf`, the [`cf-service-connect`](cf-service-connect) plug-in and the PostgreSQL `psql` client. Additional details can be found in the [cloud.gov database documentation](https://cloud.gov/docs/services/relational-database/#manually-access-a-database). This example purges the `dev` database, which is a service used by the api backend. In one terminal window: + ``` cf login -a api.fr.cloud.gov -u INSERT-USERNAME-HERE -o gsa-acq-eqip -s production --sso cf target -s dev @@ -116,6 +124,7 @@ cf connect-to-service -no-client eqip-prototype-api-dev eqip-postgres ``` `cf connect-to-service` will print out something similar to: + ``` Host: localhost Port: GENERATED-PORT @@ -125,6 +134,7 @@ Name: GENERATED-DB ``` In a second terminal window, you can connect `psql` to the local port, and `cf` will proxy those commands to the remote cloud.gov instance. When prompted for a password, supply `GENERATED-PASS` from the `cf connect-to-service` output: + ``` -psql -h localhost -p GENERATED-PORT -U GENERATED-USER -W GENERATED-DB -f purge-all-user-data.sql +psql -h localhost -p GENERATED-PORT -U GENERATED-USER -W GENERATED-DB -f purge-all-user-data.sql ``` diff --git a/docs/review.md b/docs/review.md new file mode 100644 index 000000000..0c9af1fe9 --- /dev/null +++ b/docs/review.md @@ -0,0 +1,36 @@ +# Review + + Overview of the general eApp pull request review process + +## Requester + + Create your pull request via GitHub UI. Assign one to two members of the eApp team to review your code. Include as much detail as you feel is needed for reviewers to understand the fix. Be sure to include the note `Fixes #123` with the GitHub Issue id to make sure GitHub auto links the issue to the PR, and when the PR is merged it will close the linked ticket. + +## Reviewer +### A) Review code changes + + Review code changes for style, naming, bugs, etc. + +### B) Review change for DB effects + + Data model and database changes need to be handled well. Changes to data field names, structure, and other types of changes can cause data previously saved to fail to load in the UI resulting in blank screens. In light of this changes to the data model that is saved to the database or the database structure need to be supported in a backwards compatible fashion. Two choices are available. + +#### Ensure old structure is migrated to new by a script + + To handle the data changes a migration should be done to update any old data in the database to the new format or column. This migration should be intended to be run once during the deploy process, and preferably have a script to undo the change if we need to roll back. + +#### Ensure old structure is supported by code + + If the above is not possible then the code should be made to detect that the data loading is an older version, perhaps by checking for a data version field that contains the version of eApp that was used to save. If the data is old then the code can update the format and version and sent on. Ideally this would be good to do in the backend api before the frontend sees the issue. + +### C) Verify in IE + + Download an instance of [Modern IE](https://developer.microsoft.com/en-us/microsoft-edge/tools/vms/) for the vm platform you have available. Currently eApp needs to be verified in at least IE 11 or newer. + +#### VM Setup + + Once the vm is downloaded and started edit the `C:\Windows\system32\drivers\etc\hosts` file and add the following. Note the IP may change and can be verified by checking what the **Default Gateway** is when running `ipconfig` in the cmd prompt. Also you will need to replace **web** and **api** with what values you have in `.env` for **API\_REDIRECT** and **API\_BASE\_URL** + + > 10.0.2.2 api web + + Check the PR feature / bug fix works as expected in IE 11 is important as many government employees, e.g. our army pilot users, use IE 11 as their primary browser where as developers are commonly developing in Chrome or Firefox. diff --git a/docs/updating-api-db.md b/docs/updating-api-db.md new file mode 100644 index 000000000..20ffc59a7 --- /dev/null +++ b/docs/updating-api-db.md @@ -0,0 +1,49 @@ +# Changes requiring API and database updates + +On occasion, there will be data persistence issues within eApp that cause data not to survive a log in & log out. Most of the time these issues are caused by the data not being properly saved to the database. + +## Check for the column in database + +If you suspect this to be the case, first start out by checking for the existence of a column associated with the data in the PostgreSQL database by either browsing the database directly or looking through past migrations. If you find the corresponding table and there is no column associated with the data, then move on to the next step. + +## Update frontend code + +Before updating the database and API, make sure that the frontend is handling the data properly in the Redux store. You can append `?reduxLogger=true` (on development) to the application's URL to see the content of the store at various points. Note the exact name of the field, and the type of input that is used (`DateRange`, `Location`, `Email`, etc) within the React code. Add necessary tests to support the addition of the new field. + +## Update schema + +The frontend has a schema that is used to translate the data coming from, and going into, the database. Without properly representing the data in the schema, it will not populate properly when the app is initially loaded. Find the section in question within `/src/schema/section` and add the new input's name, and input type. Input type will correspond with the component used within React (`daterange`, `text`, `location`, etc). Update corresponding schema tests to account for the new addition. See here for an example of this change: https://github.com/18F/e-QIP-prototype/pull/675/files#diff-553dcd0197b047cac66e517b87577db6 + +## Update database + +If the column for the data does not currently exist in the database, it needs to be added with a migration. As of right now, these migrations are being added by hand but in the future it would be beneficial to begin using a tool such as [goose](https://github.com/pressly/goose). In the interim, duplicate a previous migration file from `/api/migrations` and give it a descriptive file name. The timestamp in the filename can be generated by running: + +``` +date +%Y%m%d%H%M%S +``` + +Following the pattern shown in previous migrations, alter the existing table to add the new column (using snake case for the name of the column). See here for an example of adding a new column to an existing table: https://github.com/18F/e-QIP-prototype/pull/675/files#diff-9fe41306637d1dbea2da5c055ea410b5 + +## Add API boilerplate code + +Next, update the API code to include the new data. In most cases this will involve modifying the corresponding section's Go file in the root `/api` folder. Following the existing patterns, copy and paste the boilerplate code within the various functions in the file (code will be added in roughly ~10 different locations). Take care to follow proper naming styles and reference the proper input type (`DateControl`, `Location`, `Email`, etc). See here for an example of adding a new field to an existing section: https://github.com/18F/e-QIP-prototype/pull/675/files#diff-89cd2ccde37e50062861c1ebdc3b539e + +## Update XML template and test data + +After finding the corresponding XML file in `/api/templates`, make sure that the data is properly represented within the XML. It is also important to make sure that the new input is added to the corresponding `.json` file found within `/api/testdata` which is used to test the generated XML. + +Lastly, if the new data is not already found in at least one of the complete scenarios (in `/api/testdata/complete-scenarios`), then update test case 6 to include the data. This will allow the data to be pre-loaded and tested using the [load-scenario](https://github.com/18F/e-QIP-prototype/blob/develop/docs/test-scenarios.md#loading-existing-test-json-files) script. + +## Test the change + +Upon completing the above steps, restart the server(s) and: + +- Run the Go test suite `make test-go` +- Test that the data is saved within Redux and is persisted when navigating between pages +- Log out and log back in, return to the input in question and verify that the data is still present. + +If the data is still not present after logging out and back in, check the server console for errors relating to the new input - more than likely it is a misspelled variable name. Double check all of the boilerplate API code, as this seems to be where most naming errors can occur. + +## Example + +For reference, the following pull request is a good representation of most of the steps documented above: https://github.com/18F/e-QIP-prototype/pull/675 diff --git a/package.json b/package.json index 51d49ffd5..46c39a309 100644 --- a/package.json +++ b/package.json @@ -69,8 +69,10 @@ }, "devDependencies": { "babel-core": "^6.26.3", + "babel-eslint": "^10.0.1", "babel-loader": "^7.1.4", "babel-plugin-module-resolver": "^3.1.3", + "babel-plugin-transform-class-properties": "^6.24.1", "babel-preset-es2015": "^6.18.0", "babel-preset-react": "^6.16.0", "babel-preset-stage-2": "^6.24.1", diff --git a/src/components/Form/Telephone/Telephone.jsx b/src/components/Form/Telephone/Telephone.jsx index c6ee06af0..46c0153bd 100644 --- a/src/components/Form/Telephone/Telephone.jsx +++ b/src/components/Form/Telephone/Telephone.jsx @@ -116,7 +116,7 @@ export default class Telephone extends ValidationElement { update(queue) { this.props.onUpdate({ name: this.props.name, - timeOfDay: this.props.timeOfDay, + timeOfDay: this.props.showTimeOfDay ? this.props.timeOfDay : 'NA', type: this.props.type || 'Domestic', numberType: this.props.numberType, number: this.getFormattedNumber(), @@ -734,43 +734,45 @@ export default class Telephone extends ValidationElement { {this.international()} -
- - - - - -
+ +
+ + + + + +
+
{ input.focus() diff --git a/src/components/Form/Telephone/Telephone.test.jsx b/src/components/Form/Telephone/Telephone.test.jsx index 1e07bd64e..29475070c 100644 --- a/src/components/Form/Telephone/Telephone.test.jsx +++ b/src/components/Form/Telephone/Telephone.test.jsx @@ -312,6 +312,14 @@ describe('The Telephone component', () => { expect(component.find('.phonetype').length).toBe(0) }) + it('can hide time of day', () => { + const props = { + showTimeOfDay: false + } + const component = mount() + expect(component.find('.time.day input').length).toBe(0) + }) + it('can disable not applicable on on telephone', () => { const props = { allowNotApplicable: false diff --git a/src/components/Section/History/Residence/ResidenceItem.jsx b/src/components/Section/History/Residence/ResidenceItem.jsx index cb639dfb8..4c5d47ff8 100644 --- a/src/components/Section/History/Residence/ResidenceItem.jsx +++ b/src/components/Section/History/Residence/ResidenceItem.jsx @@ -414,36 +414,36 @@ export default class ResidenceItem extends ValidationElement { className="reference-relationship-neighbor" label={i18n.t('reference.label.relationship.neighbor')} value="Neighbor" - onUpdate={this.updateReferenceRelationship}> - + onUpdate={this.updateReferenceRelationship} + /> - + onUpdate={this.updateReferenceRelationship} + /> - + onUpdate={this.updateReferenceRelationship} + /> - + onUpdate={this.updateReferenceRelationship} + /> - + onUpdate={this.updateReferenceRelationship} + /> @@ -504,6 +505,7 @@ export default class ResidenceItem extends ValidationElement { onUpdate={this.updateReferencePhoneDay} onError={this.props.onError} required={this.props.required} + showTimeOfDay={false} /> @@ -521,6 +523,7 @@ export default class ResidenceItem extends ValidationElement { onUpdate={this.updateReferencePhoneMobile} onError={this.props.onError} required={this.props.required} + showTimeOfDay={false} /> diff --git a/src/components/Section/Package/ValidForm.jsx b/src/components/Section/Package/ValidForm.jsx index c3288e581..504dbbef1 100644 --- a/src/components/Section/Package/ValidForm.jsx +++ b/src/components/Section/Package/ValidForm.jsx @@ -18,20 +18,12 @@ export default class ValidForm extends ValidationElement { constructor(props) { super(props) - this.update = this.update.bind(this) - this.updateComments = this.updateComments.bind(this) - this.updateGeneral = this.updateGeneral.bind(this) - this.updateMedical = this.updateMedical.bind(this) - this.updateCredit = this.updateCredit.bind(this) - this.accordionItems = this.accordionItems.bind(this) - this.submit = this.submit.bind(this) - this.state = { accordionItems: this.accordionItems() } } - update(queue) { + update = (queue) => { this.props.onUpdate({ AdditionalComments: this.props.AdditionalComments, General: this.props.General, @@ -42,31 +34,31 @@ export default class ValidForm extends ValidationElement { }) } - updateComments(values) { + updateComments = (values) => { this.update({ AdditionalComments: values }) } - updateGeneral(values) { + updateGeneral = (values) => { this.update({ General: values }) } - updateMedical(values) { + updateMedical = (values) => { this.update({ Medical: values }) } - updateCredit(values) { + updateCredit = (values) => { this.update({ Credit: values }) } - submit() { + submit = () => { if (window.confirm('Are you sure you want to submit this application?')) { this.props.onSubmit() } @@ -91,7 +83,7 @@ export default class ValidForm extends ValidationElement { } } - accordionItems() { + accordionItems = () => { const self = this return [ { diff --git a/src/schema/section/identification-ssn.test.js b/src/schema/section/identification-ssn.test.js index 0cfd6ead1..c45cb7b70 100644 --- a/src/schema/section/identification-ssn.test.js +++ b/src/schema/section/identification-ssn.test.js @@ -3,11 +3,33 @@ import { identificationSSN } from './identification-ssn' describe('Schema for financial taxes', () => { it('can wrap in schema', () => { - const data = { - ssn: {}, - verified: false - } + const testData = [ + { + ssn: {}, + verified: false + }, + { + ssn: { + first: "111", + middle: "11", + last: "1111", + notApplicable: false + }, + verified: true + }, + { + ssn: { + first: "", + middle: "", + last: "", + notApplicable: true + }, + verified: false + } + ] - expect(unschema(identificationSSN(data))).toEqual(data) + testData.forEach(data => { + expect(unschema(identificationSSN(data))).toEqual(data) + }) }) }) diff --git a/src/validators/helpers.test.js b/src/validators/helpers.test.js index f6464192f..ec3ba4587 100644 --- a/src/validators/helpers.test.js +++ b/src/validators/helpers.test.js @@ -309,6 +309,17 @@ describe('Helpers for validators', function() { { phone: null, expected: false + }, + { + phone: { + noNumber: '', + number: '7031112222', + numberType: 'Home', + timeOfDay: '', + type: 'Domestic', + extension: '' + }, + expected: false } ]