This repository has been archived by the owner on Sep 19, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 22
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge remote-tracking branch 'origin/develop' into sr-webpack-resolve…
…-aliases
- Loading branch information
Showing
14 changed files
with
223 additions
and
72 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,7 @@ | |
"@validators": "./src/validators", | ||
"@views": "./src/views" | ||
} | ||
}] | ||
}], | ||
"transform-class-properties" | ||
] | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.