1728/notifications

[emilyjablonski]

Pull Request Template

Issue

Addresses #1728

• This change addresses the issue in full
• This change addresses only certain aspects of the issue
• This change is a dependency for another issue
• This change has a dependency from another issue

Description

If a jurisdiction has the notificationsSignUpURL field set, the home page will display a second action block with the sign up link. The migration sets the value for Alameda to be the current sign-up link.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Prototype/POC (not to merge)
• This change is a refactor/address technical debt
• This change requires a documentation update
• This change requires a SQL Script

How Can This Be Tested/Reviewed?

Load up the homepage with the jurisdiction ENV variable set to Alameda and to anything but Alameda to see both states.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have reviewed the changes in a desktop view
• I have reviewed the changes in a mobile view
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules
• I have assigned reviewers
• I have updated the changelog to include a description of my changes

[netlify]

❌ Deploy Preview for dev-storybook-bloom failed.

🔨 Explore the source changes: ef7d046

🔍 Inspect the deploy log: https://app.netlify.com/sites/dev-storybook-bloom/deploys/6142a8affeba8a000852cdb9

[netlify]

✔️ Deploy Preview for clever-edison-cd22c1 ready!

🔨 Explore the source changes: 00c4ee5

🔍 Inspect the deploy log: https://app.netlify.com/sites/clever-edison-cd22c1/deploys/6138da3750eeb600072b844d

😎 Browse the preview: https://deploy-preview-1802--clever-edison-cd22c1.netlify.app

[netlify]

✔️ Deploy Preview for dev-partners-bloom ready!

🔨 Explore the source changes: ef7d046

🔍 Inspect the deploy log: https://app.netlify.com/sites/dev-partners-bloom/deploys/6142a8af637dea0007305046

😎 Browse the preview: https://deploy-preview-1802--dev-partners-bloom.netlify.app

[emilyjablonski]

@seanmalbert Here is what I mentioned in standup today. The two issues I have/see now are (1) Getting a 401 on the jurisdictions service and (2) Feels weird to fetch all the jurisdictions, to find our current one, to find the sign-up URL

[netlify]

✔️ Deploy Preview for dev-bloom ready!

🔨 Explore the source changes: ef7d046

🔍 Inspect the deploy log: https://app.netlify.com/sites/dev-bloom/deploys/6142a8af33a38c000755a07c

😎 Browse the preview: https://deploy-preview-1802--dev-bloom.netlify.app

[seanmalbert]

@seanmalbert Here is what I mentioned in standup today. The two issues I have/see now are (1) Getting a 401 on the jurisdictions service and (2) Feels weird to fetch all the jurisdictions, to find our current one, to find the sign-up URL

1. This shouldn't be the case since anonymous users should be able to read from jurisdictions according to what's in the auth policy in the backend backend/core/src/auth/authz_policy.csv, so there's probably a name mismatch
2. I agree. process.env.jurisdictionName should be available in public, assuming you have JURISDICTION_NAME set in your .env file. I think we can create a new endpoint for getting a jurisdiction by name. See the jurisdiction controller for how get by id is setup as a reference.

[seanmalbert]

@emilyjablonski , I updated jurisdictions to use the optional auth guard so we can now read jurisdictions with an anonymous user. You should now be unblocked on that.

[pbn4]

I would like to keep GET /jurisidctions/:id endpoint because it's a REST API convention and most endpoints follow it. name search you need here could be implemented through list() endpoint. Please add optional query param to `list and do such filtering there. Frontend then should receive and parse a single element array as a response.

[seanmalbert]

@pbn4 ,
It makes sense to keep that endpoint, but I don't like using list for this either, since we're only expecting one to return (name column has been set to unique), so having another route like jurisdictions/byName/:name makes sense to me.

[seanmalbert]

Looks good! Can you update the e2e tests so that the get requests don't have the admin token, since they should be accessible by an anonymous user.

[seanmalbert]

We should be able to get this without the token now. Same with the existing gets above.

[emilyjablonski]

updated 👍