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

Case insensitive admin search, testing updates and manager setting of notes and report status #281

Merged
merged 60 commits into from
Feb 5, 2021
Merged
Show file tree
Hide file tree
Changes from 50 commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
3524e9e
Remove --runInBand
dcloud Jan 27, 2021
60b68e0
Add some docs around testing
dcloud Feb 1, 2021
bb1a516
Merge branch 'main' into dcloud/test-failure-fixes
dcloud Feb 1, 2021
8484d6e
Allow search to be case-insensitive
Feb 1, 2021
4ef5971
Fix Eslint issue
Feb 1, 2021
3f43234
Fix example
dcloud Feb 1, 2021
deac317
Rename docker-compose file used for dynamic security scans
dcloud Jan 26, 2021
f3f9022
Create bin/docker script, which will make command composition easier
dcloud Jan 27, 2021
44af291
Remove --runInBand
dcloud Jan 27, 2021
9b422f3
Add option parsing and rename docker script
dcloud Jan 27, 2021
84b24b8
Multiple docker-compose configs for multiple environments
dcloud Jan 27, 2021
92a41c4
bin/docker-run fixes, improvements
dcloud Jan 28, 2021
32f38c8
Minor docker-run improvements
dcloud Jan 28, 2021
de13b1f
Change 'conf' to be a string, since that's how it gets used. Echo cmd…
dcloud Jan 29, 2021
a232a91
docker-run: 'test' uses test env by default
dcloud Jan 29, 2021
ac43405
Undo the composed compose files; docker-compose wasn't handling diffe…
dcloud Jan 29, 2021
188b05f
Remove 2-liners in favor of echo_exec function
dcloud Jan 29, 2021
ece7bb4
docker-run: Make default config a readonly var
dcloud Jan 29, 2021
c8886f0
Add simpler run-tests script. Improve docker-compose.test.yml with na…
dcloud Feb 1, 2021
8b0bece
Remove bin/docker-run since we are no longer composing multiple
dcloud Feb 1, 2021
43e18c9
One last rename
dcloud Feb 1, 2021
90c4e50
Managers can add notes and set status of reports
jasalisbury Feb 2, 2021
975711e
Add API docs
jasalisbury Feb 2, 2021
fe19276
Update DB diagram
jasalisbury Feb 2, 2021
58b12ac
Merge branch 'main' into dcloud/test-failure-fixes
dcloud Feb 2, 2021
13ff30f
Simplify networking, remove unused containers per @dcmcand
dcloud Feb 2, 2021
e56cf18
update failing tests
Feb 2, 2021
43ed721
Merge branch 'main' into js-137-manager-approves-report
jasalisbury Feb 2, 2021
486acd4
Add section on caveats with creating/deleting databas records
dcloud Feb 2, 2021
6dee177
Merge branch 'main' into dcloud/test-failure-fixes
dcloud Feb 2, 2021
b14eeb9
Split run-test into multiple steps, per @dcmcand
dcloud Feb 2, 2021
11b92b9
Fix API docs. Split review page into components. Add DECIMAL_BASE
jasalisbury Feb 2, 2021
f698be6
Merge branch 'js-137-manager-approves-report' of github.com:adhocteam…
jasalisbury Feb 2, 2021
25e5f1f
Switch shortcut, also per @dcmcand
dcloud Feb 2, 2021
d227006
Merge branch 'main' into dcloud/split-docker-local-test
dcloud Feb 2, 2021
0a785fd
Merge branch 'main' into 274-allow-case-insensitive-search
gopar Feb 3, 2021
4eadc38
Merge branch 'main' into dcloud/test-failure-fixes
dcloud Feb 3, 2021
b3fb374
Merge branch 'main' into dcloud/test-failure-fixes
dcloud Feb 3, 2021
9e05993
Merge pull request #134 from adhocteam/dcloud/test-failure-fixes
dcloud Feb 3, 2021
b80d852
Merge branch 'main' into dcloud/split-docker-local-test
dcloud Feb 3, 2021
e976f8a
Merge branch 'main' into 274-allow-case-insensitive-search
gopar Feb 3, 2021
db9cb58
Merge pull request #138 from adhocteam/dcloud/split-docker-local-test
dcloud Feb 3, 2021
2956987
Merge branch 'main' into 274-allow-case-insensitive-search
gopar Feb 3, 2021
eb867ac
Merge pull request #137 from adhocteam/274-allow-case-insensitive-search
gopar Feb 3, 2021
75e4df0
Merge branch 'main' into js-137-manager-approves-report
jasalisbury Feb 4, 2021
d31f14f
Merge pull request #139 from adhocteam/js-137-manager-approves-report
jasalisbury Feb 4, 2021
a50285f
Increase wait time for the server to spin up in cucummber ci tests
jasalisbury Feb 4, 2021
ba18d84
Merge pull request #145 from adhocteam/js-increase-ci-cucumber-wait-time
jasalisbury Feb 4, 2021
d29a75d
Document bin/run-tests, make sure frontend tests run in frontend inst…
dcloud Feb 4, 2021
3f47788
Feedback from PR
jasalisbury Feb 4, 2021
c99ba9a
Add frontend tests
jasalisbury Feb 4, 2021
e027cdf
Remove unused import
jasalisbury Feb 4, 2021
c6f62d6
Merge pull request #147 from adhocteam/dcloud/docker-tests-changes-pe…
dcloud Feb 4, 2021
4680a12
Merge branch 'main' into js-manager-notes-feedback
jasalisbury Feb 5, 2021
5a31b94
Merge pull request #148 from adhocteam/js-manager-notes-feedback
jasalisbury Feb 5, 2021
c08eb4c
Somehow service == container_name for docker-compose exec
dcloud Feb 5, 2021
6cf1025
Add param to run-tests to choose to run only frontend or backend test…
dcloud Feb 5, 2021
c4c7bd4
Check exit codes of docker cmds, and 'log' errors. Return exit_code e…
dcloud Feb 5, 2021
68a76de
Merge branch 'main' into dcloud/test-script-improvements
dcloud Feb 5, 2021
f327246
Merge pull request #149 from adhocteam/dcloud/test-script-improvements
dcloud Feb 5, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ jobs:
- run:
name: Run cucumber
command: |
sleep 5
sleep 10
yarn cucumber:ci
- store_artifacts:
path: reports/
Expand All @@ -240,12 +240,12 @@ jobs:
- run:
name: Start up local server
command: | # production style build (single BE server with static FE)
docker-compose -f docker-compose-test.yml run --rm server yarn install --production=false
docker-compose -f docker-compose-test.yml run --rm server yarn --cwd frontend install --production=false
docker-compose -f docker-compose-test.yml run --rm server yarn build
docker-compose -f docker-compose-test.yml run --rm server yarn --cwd frontend run build
docker-compose -f docker-compose-test.yml up -d
docker-compose -f docker-compose-test.yml exec server yarn db:migrate:ci
docker-compose -f docker-compose.dss.yml run --rm server yarn install --production=false
docker-compose -f docker-compose.dss.yml run --rm server yarn --cwd frontend install --production=false
docker-compose -f docker-compose.dss.yml run --rm server yarn build
docker-compose -f docker-compose.dss.yml run --rm server yarn --cwd frontend run build
docker-compose -f docker-compose.dss.yml up -d
docker-compose -f docker-compose.dss.yml exec server yarn db:migrate:ci
- run:
name: Pull OWASP ZAP docker image
command: docker pull owasp/zap2docker-weekly
Expand Down
24 changes: 24 additions & 0 deletions bin/run-tests
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#!/bin/bash

echo "Running tests in using test config 'docker-compose.test.yml'"
# Start containers
docker-compose -f 'docker-compose.test.yml' up -d

# Let postgres initialize
echo "Give postgres a few seconds to start up..."
sleep 5

# Migrate and seed db
docker exec test-backend bash -c "yarn db:migrate"
docker exec test-backend bash -c "yarn db:seed;"

# Test backend
docker exec test-backend bash -c "yarn test:ci"

# Test frontend
docker exec test-frontend bash -c "yarn --cwd frontend run test:ci"
Copy link
Contributor

@rahearn rahearn Feb 5, 2021

Choose a reason for hiding this comment

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

This doesn't work. Looks like a missing container_name for the frontend service. --cwd frontend is also unnecessary (though doesn't break anything) in the frontend container.

Screen Shot 2021-02-05 at 9 35 36 AM

That also exposes that this script papers over any failures if you don't read the output carefully. Since this is not used in CI/CD I think I'm ok with that, but I think it'd be good to collect exit statuses and exit the script with non-zero if any individual step exits with non-zero, or just bail out as soon as we get a non-zero.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm, I've run this a few times and haven't seen this problem. I'll take a look again. Could you get me the output of docker-compose -f docker-compose.test.yml ps --all on your machine?

As for handing failures, it was intentional that we didn't error out if one of the intermediate steps threw an error. We could catch those statuses, but I'm not sure we want to prevent frontend tests from running if the backend tests fail, for example.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure we want to prevent frontend tests from running if the backend tests fail, for example

Yeah, I went back and forth on that. I'm 100% fine with all the steps running as long as I get a nice clear indication that at least one step failed when the script exits.

Here's the ps output:

Screen Shot 2021-02-05 at 10 20 14 AM

Copy link
Contributor

@dcloud dcloud Feb 5, 2021

Choose a reason for hiding this comment

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

Just a quick check of the doc page for docker-compose exec, it sounds like that command takes a service name rather than a container name, so that might be related.

While we're discussing, I had originally considered having the frontend and backend tests run as separate scripts, since it seems like engineers might be focused on either frontend or backend during development. Would you be interested in making this accept an optional frontend/backend param?

Copy link
Contributor

Choose a reason for hiding this comment

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

Would you be interested in making this accept an optional frontend/backend param

If it's quick, sure, but that's pretty low priority to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

It was quick: https://github.com/adhocteam/Head-Start-TTADP/pull/149/files

I also added some logging of errors and have the script return an exit code value equal to the number of script failures.


# Cleanup
docker-compose \
-f 'docker-compose.test.yml' \
down --volumes
File renamed without changes.
38 changes: 38 additions & 0 deletions docker-compose.test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
version: "3.5"
services:
test-backend:
build:
context: .
container_name: test-backend
command: yarn server
user: ${CURRENT_USER:-root}
depends_on:
- test-db
environment:
- POSTGRES_HOST=test-db
volumes:
- ".:/app:rw"
networks:
- ttadp-test
test-frontend:
build:
context: .
command: yarn start
user: ${CURRENT_USER:-root}
stdin_open: true
volumes:
- "./frontend:/app:rw"
- "./scripts:/app/scripts"
environment:
- BACKEND_PROXY=http://test-backend:8080
networks:
- ttadp-test
test-db:
image: postgres:12.4
container_name: test-db
env_file: .env
networks:
- ttadp-test
# Use non-default network so we don't conflict with the developer environment
networks:
ttadp-test:
5 changes: 3 additions & 2 deletions docs/logical_data_model.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Logical Data Model
==================

<img src="http://www.plantuml.com/plantuml/png/nLVRRkCs47tNL_2jRO0OHOkYm29Ox6xgBV9GDqYo7p2M9Z9RYiCEHzaQSVwzffWAeYnd6QkNbmGwSpyyrCZll01BsvZ2bs1GVRluyOUY-4h10-bAEQQrmSQhpres2cnji58bqsUlxX6byBzhwu2XKKPRbAr3HImRAehIWFVgaqTMZuLIPqfNwetILh3UGnSNDLfa4ApUljsRDnvzyBqAJbvOstgFMcXm-EmsP77LDcla8Noci05jXf1eCVX6eMsRa9qQlC5QaxqQDF7rzzzzeswM7vStozNbybQvTcrqkOYEVg6hBdN96BfyB_j0qGrwWSPrbjPL4vQdlqUFUNA6FniDsgUuBSdXAHcKgj2Nh9reiZqwQvnhv4PxdprIwv4ps0w7tdFD9vwH3pnbX7Ly_XCd-qUBNfRCn3w73NBXGPCTdb3BUlZOe6meKJ_7CX-esgQmTsGz7iClgS8UdpaecIdvlIJPEtAwp8_foyIuB6KncPp33bdxivQG_ghwKapNnNo1_4nWyZXGoWj8wuT8I7ZF8pVjFp8rjmsm-0sYt0bS6mY2MXCP13klusxmbzgkzxOCqaEA_ujq1YkOgTVvbOxM6Q-dazzgTHmeX_Ne6GCF1TLj-9z0S3GeskeGdKrCa436HJvbm9IQtBWOTPA6Pt2n8XkxFcM1polzlXJze6LxWuvOSiepUcgvxBx1aXq4LQN3uWAp8i54LL7elzvV0uikjGjC9tUXGgCVaBU_76osgktNOOMbRi0aQWp39RBj8UWdsnVKQELkwt1YPuw_3m_lX6Eg6PeG7t1WXkr5j0P21BX6kIRNcgxJAHz6y6Gd32vbP6IEhyL9Wq2Svy1IaXNyKv81eCPRYUpyvicU_D5bmCkdAAWFst_vY-qvRBwq1wBBkrwGt8cCXjDB51j_rMidXjZ-DP0Hj-gTPnELjdz9ni2Q68Jul-BhjO8C8TFjvw-KkMsVi2cUsmuJgDtkd1vLb7f4dkZyVFSu-F5KnDzqXtYCcZAGwVsxJwMY-wkUTXqKUYYYQdgtVWyNuSy16bCk5Yhy7wISXcBHFji8YON3v7s9vr5IBEKv93ckq4ZToovVofVqUymKAHoPaxCScTB7n2Tv9qLUWdtQt5zqFExp3ctLDkOl">
<img src="http://www.plantuml.com/plantuml/png/nLVRRkCs47tNL_2jRO0OHOkYm29Ox6xgBV9GDqYo7p2M9Z9RYiCEHzaQSVwzefaAeYnd6QkNbmGwSpyylBXuvmLOsiOKlWI3wjl5ZpyKnLSA7aXNoZ6j3JPUVTQoKM1hWvKgcJvxTOyeXVzTMmSCZJBQecmTAcBPK56K1hnNdpooUYoKEbDTjZTAMy6wWoukQhJ885YzVTkPDnvzyBqAJbvOsxf3BJGu_7ORihXgcpNo43vJs80sGqYq67oZqBPDo4uDtc2joLuDclZw----KJVBZykRvUfoUQjSkpQwN4H7lz3LFdN96BfyBrsWw0QzGEFsbjPL4vQdlqUFUNA6FniDsgUuBSdXAHcKgj2Nh9reiZqwQvnhv4PxdprIwv4ps0wBtdFD9wwH3pnbXEtv_2TMze-smvRCn3w73NBXGvCThb3BUlZPe6meKJ_7CX-esgQmTsGz7iClgS8UdpaecIdvlIJPEtAwp8_foyIuB6KncPp53bdxjPQG_ghwKio-Y_a2-Pd0v76Wb1UGrW-HaF2UHsxQVsHgRXjWyHj4kHEuDX04jIOo27PMnt7PNcgxxcmPf8SK_nVf35OmKw_pAnsjCxwUJZxLwZXG3klHCmQE11KHbfz0S3GeskeGdKrCa43EHZvXm9IQtBWOTPA6Pt2n8XkxFkK2drVwVIdwGSls1Xsnv9HdzDHokN-39NSGL9KMJHRCYWGJLKMX_tf_3Iowr2umdUo52er-GDx-Sd2pLMr_7LPOwWPCeiOmN2BR7e9-igwWHYjtMuSJT-RuVNYu9nnJpT24-803CsmleZKG8i0roZMvrkmxdV9X14zsmk0IHiRcU2nE6WJYF0UMagpWdv8C03NUIcJdDq-sWngm7SZfIfty63FWvLCKseVjl_oIUmwRB_c16FEs7IHx9QFXT7F5nbzr7CKGsnyF6N6htdasKMdxbsGCh8b1YFyhlbuho10otNx-LJEtxHDkaEUsmmHgjpkdHnMb7f4dEZ_VFGv-L1NnjzuX7gEc36Hw__qJgeXlwAVT7Wez554rlLj_FnVXpm4QKouMAlmVf9o6Oj4-smY9XSFaVOddKL8ivJaaEQxGIDsBBb_Cb_Hxp1Gf79cJjHoPqi_49pcJ8Y_1FcqUF1gE-hn3MxMD-Gi0">

UML Source
----------
Expand Down Expand Up @@ -150,6 +150,7 @@ class ActivityReport {
ttaType : array<string>
context : string
pageState : json
managerNotes : string
* userId : integer(32) REFERENCES public.Users.id
* lastUpdatedById : integer(32) REFERENCES public.Users.id
* regionId : integer(32) REFERENCES public.Region.id
Expand Down Expand Up @@ -208,7 +209,7 @@ NonGrantee ||-{ ActivityParticipant
Instructions
------------

1. [Edit this diagram with plantuml.com](http://www.plantuml.com/plantuml/uml/nLVRRkCs47tNL_2jRO0OHOkYm29Ox6xgBV9GDqYo7p2M9Z9RYiCEHzaQSVwzffWAeYnd6QkNbmGwSpyyrCZll01BsvZ2bs1GVRluyOUY-4h10-bAEQQrmSQhpres2cnji58bqsUlxX6byBzhwu2XKKPRbAr3HImRAehIWFVgaqTMZuLIPqfNwetILh3UGnSNDLfa4ApUljsRDnvzyBqAJbvOstgFMcXm-EmsP77LDcla8Noci05jXf1eCVX6eMsRa9qQlC5QaxqQDF7rzzzzeswM7vStozNbybQvTcrqkOYEVg6hBdN96BfyB_j0qGrwWSPrbjPL4vQdlqUFUNA6FniDsgUuBSdXAHcKgj2Nh9reiZqwQvnhv4PxdprIwv4ps0w7tdFD9vwH3pnbX7Ly_XCd-qUBNfRCn3w73NBXGPCTdb3BUlZOe6meKJ_7CX-esgQmTsGz7iClgS8UdpaecIdvlIJPEtAwp8_foyIuB6KncPp33bdxivQG_ghwKapNnNo1_4nWyZXGoWj8wuT8I7ZF8pVjFp8rjmsm-0sYt0bS6mY2MXCP13klusxmbzgkzxOCqaEA_ujq1YkOgTVvbOxM6Q-dazzgTHmeX_Ne6GCF1TLj-9z0S3GeskeGdKrCa436HJvbm9IQtBWOTPA6Pt2n8XkxFcM1polzlXJze6LxWuvOSiepUcgvxBx1aXq4LQN3uWAp8i54LL7elzvV0uikjGjC9tUXGgCVaBU_76osgktNOOMbRi0aQWp39RBj8UWdsnVKQELkwt1YPuw_3m_lX6Eg6PeG7t1WXkr5j0P21BX6kIRNcgxJAHz6y6Gd32vbP6IEhyL9Wq2Svy1IaXNyKv81eCPRYUpyvicU_D5bmCkdAAWFst_vY-qvRBwq1wBBkrwGt8cCXjDB51j_rMidXjZ-DP0Hj-gTPnELjdz9ni2Q68Jul-BhjO8C8TFjvw-KkMsVi2cUsmuJgDtkd1vLb7f4dkZyVFSu-F5KnDzqXtYCcZAGwVsxJwMY-wkUTXqKUYYYQdgtVWyNuSy16bCk5Yhy7wISXcBHFji8YON3v7s9vr5IBEKv93ckq4ZToovVofVqUymKAHoPaxCScTB7n2Tv9qLUWdtQt5zqFExp3ctLDkOl)
1. [Edit this diagram with plantuml.com](http://www.plantuml.com/plantuml/png/nLVRRkCs47tNL_2jRO0OHOkYm29Ox6xgBV9GDqYo7p2M9Z9RYiCEHzaQSVwzefaAeYnd6QkNbmGwSpyylBXuvmLOsiOKlWI3wjl5ZpyKnLSA7aXNoZ6j3JPUVTQoKM1hWvKgcJvxTOyeXVzTMmSCZJBQecmTAcBPK56K1hnNdpooUYoKEbDTjZTAMy6wWoukQhJ885YzVTkPDnvzyBqAJbvOsxf3BJGu_7ORihXgcpNo43vJs80sGqYq67oZqBPDo4uDtc2joLuDclZw----KJVBZykRvUfoUQjSkpQwN4H7lz3LFdN96BfyBrsWw0QzGEFsbjPL4vQdlqUFUNA6FniDsgUuBSdXAHcKgj2Nh9reiZqwQvnhv4PxdprIwv4ps0wBtdFD9wwH3pnbXEtv_2TMze-smvRCn3w73NBXGvCThb3BUlZPe6meKJ_7CX-esgQmTsGz7iClgS8UdpaecIdvlIJPEtAwp8_foyIuB6KncPp53bdxjPQG_ghwKio-Y_a2-Pd0v76Wb1UGrW-HaF2UHsxQVsHgRXjWyHj4kHEuDX04jIOo27PMnt7PNcgxxcmPf8SK_nVf35OmKw_pAnsjCxwUJZxLwZXG3klHCmQE11KHbfz0S3GeskeGdKrCa43EHZvXm9IQtBWOTPA6Pt2n8XkxFkK2drVwVIdwGSls1Xsnv9HdzDHokN-39NSGL9KMJHRCYWGJLKMX_tf_3Iowr2umdUo52er-GDx-Sd2pLMr_7LPOwWPCeiOmN2BR7e9-igwWHYjtMuSJT-RuVNYu9nnJpT24-803CsmleZKG8i0roZMvrkmxdV9X14zsmk0IHiRcU2nE6WJYF0UMagpWdv8C03NUIcJdDq-sWngm7SZfIfty63FWvLCKseVjl_oIUmwRB_c16FEs7IHx9QFXT7F5nbzr7CKGsnyF6N6htdasKMdxbsGCh8b1YFyhlbuho10otNx-LJEtxHDkaEUsmmHgjpkdHnMb7f4dEZ_VFGv-L1NnjzuX7gEc36Hw__qJgeXlwAVT7Wez554rlLj_FnVXpm4QKouMAlmVf9o6Oj4-smY9XSFaVOddKL8ivJaaEQxGIDsBBb_Cb_Hxp1Gf79cJjHoPqi_49pcJ8Y_1FcqUF1gE-hn3MxMD-Gi0)
2. Copy and paste the final UML into the UML Source section
3. Update the img src and edit link target to the current values

Expand Down
46 changes: 46 additions & 0 deletions docs/openapi/paths/activity-reports/review.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
put:
tags:
- activity-reports
summary: Review an activity report
description: >
An approving manager reviews an activity report to determine if it requires
any additional updates. If the report needs updates the manager sets the status to
'Needs Action', otherwise to 'Approved'
requestBody:
description: The status and any manager notes
required: true
content:
application/json:
schema:
type: object
properties:
status:
type: string
description: The status of the report after review
enum:
- Approved
- Needs Action
managerNotes:
type: string
description: Any notes the manager needs to relay to the author/collaborators of the report
parameters:
- in: path
name: activityReportId
required: true
schema:
type: number
responses:
200:
description: The new status of the activity report
content:
application/json:
schema:
type: object
properties:
status:
type: string
enum:
- Approved
- Needs Action
managerNotes:
type: string
2 changes: 1 addition & 1 deletion docs/openapi/paths/collaborators.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ get:
schema:
type: array
items:
$ref: '../../index.yaml#/components/schemas/selectableUser'
$ref: '../index.yaml#/components/schemas/selectableUser'
2 changes: 2 additions & 0 deletions docs/openapi/paths/index.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,7 @@
$ref: './activity-reports/activity-reports-id.yaml'
'/activity-reports/{activityReportId}/submit':
$ref: './activity-reports/submit.yaml'
'/activity-reports/{activityReportId}/review':
$ref: './activity-reports/review.yaml'
'/files':
$ref: './files.yaml'
44 changes: 44 additions & 0 deletions docs/testing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Testing

## Tips and Caveats when writing tests


### Handling async/promises

When writing tests that rely on asynchronous operations, such as writing to the database, take care to make sure that those operations are resolved before any tests that rely on them run. If you need to create database records in a setup function such as `beforeAll`, you will want to make sure all async/promise operations resolve before subsequent tests run. You can make sure multiple await (promise) operations resolve by using [`Promise.all()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/all) (which takes an iterable of promises).

Here's how that might look:

```
let a = someAsyncFun();
let b = anotherAsyncFun();

return Promise.all([a, b]);
```

### Creating/deleting database records

Some tests will require interactions with the database, but at present all tests run using the same instance of the database. That means that any records you create or delete can potentially affect tests elsewhere. On top of that, tests run in parallel, so database operations may run in an unexpected order. That can mean tests may pass various times only to fail due to missing or unexpected records in the database when your tests run.

To mitigate issues with missing or unexpected records causing failing tests, you can try a few approaches. One approach is to avoid using the database if your test doesn't actually require it. You may be able to use mock models or responses rather than interact with the database. If your test does require the database, you should create the records you need before your tests run and delete the records you created (and no others) when your tests finish. If you `Model.create`, make sure you `Model.destroy()` the records you created.

When writing tests that create database records, it might also help to use a `try...catch` to catch errors in database transactions and log meaningful output. Sequelize error messages can be vague, and it might help others to see more informative messages.

## Testing in Docker

### Using `./bin/run-tests`

To simplify running tests in Docker, there is a bash script, `./bin/run-tests` that will run the appropriate commands to start `test-` variations of the services used in tests. You should be able to run tests using that command while your development Docker environment is running. The script uses a separate `docker-compose.test.yml` which does not create a user-accessible network and cleans up after itself once tests have run.

### Running tests in your development Docker environment

When running tests in Docker, be aware that there are tests that will modify/delete database records. For tests to run, the 'db' service needs to exist and `db:migrate` and `db:seed` need to have been run (to create the tables and populate certain records).

In the `docker-compose.yml` configuration, the database is set up to persist to a volume, "dbdata", so database records will persist between runs of the 'db' service, unless you remove that volume explicitly (e.g. `docker volume rm` or `docker-compose down --volumes`).

rahearn marked this conversation as resolved.
Show resolved Hide resolved

### Notes on docker-compose and multiple configurations

`docker-compose` has a feature for providing multiple `docker-compose.*.yml` files where subsequent files can override settings in previous files, which sounds like it would suit the use case of running docker for local development and for testing. However, the ability to [override configurations](https://docs.docker.com/compose/extends/#adding-and-overriding-configuration) is limited. While experimenting with overrides, it became clear that doing so would require a minimum of three docker-compose.yml files: one "base", one for local development, one for running tests. Trying to compose docker-compose.yml files would be complicated.

In addition, while experimenting with multiple configuration files, it became clear that docker was unable to differentiate between different versions of the same service. Trying to override the 'db' service for testing would not work as expected: if the local/dev 'db' service had already been created, that one would be used when tests were run.
2 changes: 1 addition & 1 deletion frontend/src/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ function App() {
<Route
path="/activity-reports/:activityReportId/:currentPage?"
render={({ match }) => (
<ActivityReport match={match} />
<ActivityReport match={match} user={user} />
)}
/>
{admin
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/Constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,5 @@ export const REGIONS = [
11,
12,
];

export const DECIMAL_BASE = 10;
5 changes: 4 additions & 1 deletion frontend/src/components/Navigator/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const pages = [
label: 'review page',
path: 'review',
review: true,
render: (hookForm, allComplete, formData, submitted, onSubmit) => (
render: (hookForm, allComplete, formData, onSubmit) => (
<div>
<button type="button" data-testid="review" onClick={onSubmit}>Continue</button>
</div>
Expand All @@ -59,6 +59,9 @@ describe('Navigator', () => {
<Navigator
submitted={false}
initialData={initialData}
status="draft"
onReview={() => {}}
approvingManager={false}
defaultValues={{ first: '', second: '' }}
pages={pages}
currentPage={currentPage}
Expand Down
7 changes: 6 additions & 1 deletion frontend/src/components/Navigator/components/SideNav.css
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,13 @@
color: #f8f8f8;
}

.smart-hub--tag-needs-action {
background-color: #f9e0e4;
color: #d42240;
}

.smart-hub--tag {
width: 84px;
width: 94px;
text-align: center;
font-weight: normal;
display: inline-block;
Expand Down
8 changes: 6 additions & 2 deletions frontend/src/components/Navigator/components/SideNav.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import moment from 'moment';
import Container from '../../Container';
import './SideNav.css';
import {
NOT_STARTED, IN_PROGRESS, COMPLETE, SUBMITTED,
NOT_STARTED, IN_PROGRESS, COMPLETE, SUBMITTED, APPROVED, NEEDS_ACTION,
} from '../constants';

const tagClass = (state) => {
Expand All @@ -26,6 +26,10 @@ const tagClass = (state) => {
return 'smart-hub--tag-complete';
case SUBMITTED:
return 'smart-hub--tag-submitted';
case APPROVED:
return 'smart-hub--tag-submitted';
case NEEDS_ACTION:
return 'smart-hub--tag-needs-action';
default:
return '';
}
Expand All @@ -46,7 +50,7 @@ function SideNav({
>
<span className="margin-left-2">{page.label}</span>
<span className="margin-left-auto margin-right-2">
{page.state
{page.state !== 'draft'
&& (
<Tag className={`smart-hub--tag ${tagClass(page.state)}`}>
{page.state}
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/components/Navigator/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ export const NOT_STARTED = 'Not started';
export const IN_PROGRESS = 'In progress';
export const COMPLETE = 'Complete';
export const SUBMITTED = 'Submitted';
export const APPROVED = 'Approved';
export const NEEDS_ACTION = 'Needs Action';
16 changes: 10 additions & 6 deletions frontend/src/components/Navigator/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import moment from 'moment';
import Container from '../Container';

import {
IN_PROGRESS, COMPLETE, SUBMITTED,
IN_PROGRESS, COMPLETE,
} from './constants';
import SideNav from './components/SideNav';
import NavigatorHeader from './components/NavigatorHeader';
Expand All @@ -24,11 +24,13 @@ function Navigator({
initialData,
pages,
onFormSubmit,
submitted,
onReview,
currentPage,
additionalData,
onSave,
autoSaveInterval,
approvingManager,
status,
reportId,
}) {
const [formData, updateFormData] = useState(initialData);
Expand All @@ -37,7 +39,6 @@ function Navigator({
const { pageState } = formData;

const page = pages.find((p) => p.path === currentPage);
const submittedNavState = submitted ? SUBMITTED : null;
const allComplete = _.every(pageState, (state) => state === COMPLETE);

const hookForm = useForm({
Expand Down Expand Up @@ -102,7 +103,7 @@ function Navigator({
const navigatorPages = pages.map((p) => {
const current = p.position === page.position;
const stateOfPage = current ? IN_PROGRESS : pageState[p.position];
const state = p.review ? submittedNavState : stateOfPage;
const state = p.review ? status : stateOfPage;
return {
label: p.label,
onNavigation: () => onSaveForm(false, p.position),
Expand All @@ -129,9 +130,10 @@ function Navigator({
hookForm,
allComplete,
formData,
submitted,
onFormSubmit,
additionalData,
onReview,
approvingManager,
reportId,
)}
{!page.review
Expand All @@ -158,8 +160,10 @@ function Navigator({
Navigator.propTypes = {
initialData: PropTypes.shape({}),
onFormSubmit: PropTypes.func.isRequired,
submitted: PropTypes.bool.isRequired,
onSave: PropTypes.func.isRequired,
status: PropTypes.string.isRequired,
onReview: PropTypes.func.isRequired,
approvingManager: PropTypes.bool.isRequired,
pages: PropTypes.arrayOf(
PropTypes.shape({
review: PropTypes.bool.isRequired,
Expand Down
3 changes: 2 additions & 1 deletion frontend/src/fetchers/Admin.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import join from 'url-join';
import { get, put } from './index';
import { DECIMAL_BASE } from '../Constants';

export const getUsers = async () => {
const users = await get((join('/', 'api', 'admin', 'users')));
return users.json();
};

export const updateUser = async (userId, data) => {
const user = await put((join('/', 'api', 'admin', 'users', userId.toString(10))), data);
const user = await put((join('/', 'api', 'admin', 'users', userId.toString(DECIMAL_BASE))), data);
return user.json();
};
Loading