Fix migrations/Add URL routing to activity report #234

jasalisbury · 2020-12-17T23:28:06Z

Issue 202

Description of change

adds migration to deployment command
cleans up other environment variables that are passed to the application
adds step by step instructions for developers to add their own env variables to the application.

How to test

Add staging postgres db vars to circle ci as env variable. @rahearn Use these variable names: STAGING_POSTGRES_DB, STAGING_POSTGRES_HOST, STAGING_POSTGRES_PASSWORD, STAGING_POSTGRES_USERNAME
After deploy completes, check that the postgres var were written as cloud.gov env vars and verify that the migrations were run by reading the log stream. Failed migrations will not be visible in circle ci since the migration is run as a task on the application instance

Issue(s)

closes CD: Run DB Migrations #202

Checklist

Meets issue criteria
Code tested
[n/a] Meets accessibility standards (WCAG 2.1 Levels A, AA)
Documentation updated
- API methods
- Boundary and Data Flow Diagrams
- Architectural Decision Records written for major infrastructure decisions with the Nygard template
- OSCAL templates completed when security controls are implemented or modified

Issue 135

Description of change
The current activity reports page shown is determined by the url. Being able to browse to specific report pages via URL has two main advantages:

A user can share/bookmark a specific page
We are setup to directly link to specific sections of the pages

Also I added the report pages to the axe testing, and added a file with the urls axe should test. Note the axe:ci command no longer works on windows. Also fixed a color contrast accessibility issue in the activity report side nav when a page is selected and has a state of Complete.

How to test

Browse to dev. Browsing to different pages will change the url. You can also directly go to specific pages of the report (https://tta-smarthub-dev.app.cloud.gov/activity-reports/review)

Issue(s)

Review and submit a report for approval #135

Checklist

Meets issue criteria
Code tested
Meets accessibility standards (WCAG 2.1 Levels A, AA)
[n/a] Documentation updated
- API methods
- Boundary and Data Flow Diagrams
- Architectural Decision Records written for major infrastructure decisions with the Nygard template
- OSCAL templates completed when security controls are implemented or modified

Issue 230

Description of change
Updates nodejs version to 12.20.0 to work with current cloud.gov buildpack

Issue

closes Update nodejs versions #230

Checklist
Checklist

Meets issue criteria
Code tested

Being able to browse to specific report pages via URL has two main advantages: * A user can share/bookmark a specific page * We are setup to directly link to specific sections of the pages

* check if env vars can be referenced directly in deploy config * try as deployments vars, and out of the manifest * add back to manifest * was it proactively checking other branches? * check that we really need vars in manifest * try migrate as cf run-task in single command * ignore that failing fe test for now * omg, this takes forever * try wo explicit wait * add back sleep, up to 120; add db connect directions * move sequelize-cli to production deps * change migrations path to build dir * move production sequalizerc to file * update migration command * add back all the jobs * rename prod sequalize config * missing ' * don't restart app Co-authored-by: Sarah-Jaine Szekeresh <[email protected]>

* adhoc-main: CD: Run database migrations (#90)

Nodejs & yarn updates

* add fake postgres vars to deploy config * move to manifest * use var substitute * update vars and documentation * add back jobs * make all env vars uppercase * whoops, missing ) * call out crazy param syntax Co-authored-by: Sarah-Jaine Szekeresh <[email protected]>

Conflicts: .circleci/config.yml

Activity report uses urls

rahearn

I love the documentation updates for env vars, but in the particular use case of the database, I don't want to have to update the variables by hand.

Since we're using a managed service, the deployed environment already contains DATABASE_URL. Let's tell sequelize to configure itself with that in production rather than having to manually set each individual part.

rahearn · 2020-12-18T14:20:38Z

.circleci/config.yml

@@ -368,6 +416,7 @@ workflows:
            - build
      - deploy:
          requires:
+            - build


Why did we add build explicitly here? If build fails all the rest of the steps wont run either.

Whoops- definitely a copy paste error there. I keep removing and adding back the required jobs for deploy so I can test out deploys faster on my feature branches. I'm working on a PR to merge into josh's adhocteam#78 (I'm correcting the seeding that runs on CD). I can fix this then, or just open a new simple PR if that's better. Let me know.

Can we remove it here and also switch over to using DATABASE_URL instead of all of the CircleCI configured POSTGRES variables?

rahearn · 2020-12-18T14:22:47Z

README.md

+	  - name: tta-smarthub-((env))
+	    env:
+	      SECRET_FRUIT: ((SECRET_FRUIT))
+	      PUBLIC_VEGGIE: ((public_veggie))


Freaking love SECRET_FRUIT and PUBLIC_VEGGIE as the example vars

rahearn · 2020-12-18T14:28:47Z

manifest.yml

@@ -6,12 +6,16 @@ applications:
    buildpacks:
      - nodejs_buildpack
    env:
-      AUTH_BASE: ((AUTH_BASE))
+      AUTH_BASE: https://uat.hsesinfo.org


I suppose we can change this later (I don't know exactly what the value will be anyway), but this will not be the AUTH_BASE for production

rahearn · 2020-12-18T14:41:32Z

frontend/src/components/Navigator/components/SideNav.js

@@ -71,8 +72,8 @@ SideNav.propTypes = {
  pages: PropTypes.arrayOf(
    PropTypes.shape({
      label: PropTypes.string.isRequired,
-      current: PropTypes.bool.isRequired,


🎉 for using the URL to determine the current page

rahearn · 2020-12-18T14:56:52Z

Some follow up information:

Documentation that cloud.gov adds DATABASE_URL to bound applications. I verified that our apps do already have that environment variable set, due to the services block of manifest.yml

A blog post that includes information on using DATABASE_URL to configure sequelize. I have not verified that this post is still relevant to sequelize today, since it is over 2 years old.

CD: Use DATABASE_URL

rahearn

🎉

…not-work add region to s3 config

jasalisbury and others added 10 commits December 15, 2020 16:22

Activity report pages are navigable via url

fdc2df9

Being able to browse to specific report pages via URL has two main advantages: * A user can share/bookmark a specific page * We are setup to directly link to specific sections of the pages

Set sandbox branch

66e7d6b

Upgrade to node 12.20.0

49a15ac

Upgrade js dependencies

8c81f12

Merge branch 'adhoc-main' into nodejs-update

d80ad52

* adhoc-main: CD: Run database migrations (#90)

Merge pull request #93 from HHS/nodejs-update

e7c3619

Nodejs & yarn updates

Merge branch 'main' into js-135-activity-report-uses-urls

e1bb335

Conflicts: .circleci/config.yml

Merge pull request #89 from adhocteam/js-135-activity-report-uses-urls

3d25fa3

Activity report uses urls

jasalisbury requested a review from rahearn December 17, 2020 23:28

rahearn requested changes Dec 18, 2020

View reviewed changes

SarahJaine added 2 commits December 21, 2020 17:56

database_url

d1d3a3b

explain var in .env.example

50b8f98

SarahJaine mentioned this pull request Dec 22, 2020

CD: Use DATABASE_URL adhocteam/Head-Start-TTADP#97

Merged

3 tasks

SarahJaine and others added 12 commits December 22, 2020 11:45

try supplying both

f751b4f

remove pg vars from manifest

693714d

updatedirections

f681786

without config options

2c38854

add dialect

b625899

linting, skip to deploy

32f3a8b

use variable name

2761340

add back tests to workflow, whitespace

2801bcc

add database_url to docker service

123cb0f

host same as container_name?

471bf13

add back other jobs

1d3bf88

Merge pull request #97 from adhocteam/sj-cd-use-db-url

9f661a5

CD: Use DATABASE_URL

rahearn approved these changes Dec 23, 2020

View reviewed changes

rahearn merged commit 87eea27 into HHS:main Dec 23, 2020

rahearn added a commit that referenced this pull request Mar 13, 2021

Merge pull request #234 from adhocteam/cm-370-downloading-files-does-…

b173436

…not-work add region to s3 config

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix migrations/Add URL routing to activity report #234

Fix migrations/Add URL routing to activity report #234

jasalisbury commented Dec 17, 2020 •

edited by rahearn

Loading

rahearn left a comment

rahearn Dec 18, 2020

SarahJaine Dec 18, 2020

rahearn Dec 18, 2020

rahearn Dec 18, 2020

rahearn Dec 18, 2020

rahearn Dec 18, 2020

rahearn commented Dec 18, 2020 •

edited

Loading

rahearn left a comment

Fix migrations/Add URL routing to activity report #234

Fix migrations/Add URL routing to activity report #234

Conversation

jasalisbury commented Dec 17, 2020 • edited by rahearn Loading

Issue 202

Issue 135

Issue 230

rahearn left a comment

Choose a reason for hiding this comment

rahearn Dec 18, 2020

Choose a reason for hiding this comment

SarahJaine Dec 18, 2020

Choose a reason for hiding this comment

rahearn Dec 18, 2020

Choose a reason for hiding this comment

rahearn Dec 18, 2020

Choose a reason for hiding this comment

rahearn Dec 18, 2020

Choose a reason for hiding this comment

rahearn Dec 18, 2020

Choose a reason for hiding this comment

rahearn commented Dec 18, 2020 • edited Loading

rahearn left a comment

Choose a reason for hiding this comment

jasalisbury commented Dec 17, 2020 •

edited by rahearn

Loading

rahearn commented Dec 18, 2020 •

edited

Loading