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

MCR-4893: Adjust Apollo handler to use feature flag #3099

Closed

Conversation

JasonLin0991
Copy link
Contributor

@JasonLin0991 JasonLin0991 commented Jan 22, 2025

Summary

MCR-4893

AC:

  • Pull out code in apollo_gql.ts that reads and configures email configuration data into a function and returns the configuration for the Apollo server.
  • The new function should gather email configuration based on feature flag state.
    • With "Remove Parameter Store" feature flag ON, the source of the emails are queried from the database.
    • With "Remove Parameter Store" feature flag OFF the source of the emails are queried from parameter store.
  • Unit tests for flag on and flag off
    • Use the new function for gathering email configuration in constructTestPostgresServer to configure unit tests.
    • For fetchMcReviewSettings resolver the configuration emails are from the correct source
    • There are several other resolvers that use the emailer dependency for CMS emails. Unit test some of these resolvers so that the CMS recipients are from the correct source.
      • NOTE: Upon working on these tests I figured it was pretty straight forward so I only tested 1 automated email since they all work the same with the dependency injection.I don't mind adding more if anyone disagrees.

Additional work:

  • I had to refactor ldService so that we can use it in before initialization and getting the user data. Before we passed our Context into ldService methods and it would extract out the user's email for LaunchDarkly targeting. Now we have to explicitly give a key to the method, which would be the email or any identifiable string.
    • Now in the apollo hanlder, we can use the ldService methods by giving it a default key email-configuration. This flag is temporary so I think this should be ok. If we had some kind of permanent flag that is read from the handler, we should either figure out a way to get the user email or we define a new kind in LaunchDarkly to label it correctly. Something like handler so that we know flag was read by the handler.
  • Updated the default email settings values in Prisma schema to not have a cms.hhs.gov email and only use our truss email.
    • The migration will also update the existing settings.
  • There is a drift in the types EmailSettings (values from the database) and EmailConfiguration (values for the emailer).
    • 3 fields cmsReviewHelpEmailAddress, cmsRateHelpEmailAddress, and helpDeskEmail differ in types (string and string[]). If we make the types the same we will have to update all the emailer functions, tests, mocks, and helpers. This seemed way out of scope for this ticket so I opted to just have the configureEmailerFromDatabase function to handle the type difference for now and will spin up a new ticket for that work.
    • Add ticket to fix type drift between EmailSettings (values from the database) and EmailConfiguration (values for the emailer)
      • MCR-5018: Clean up type drift between EmailSettingsType and EmailConfiguration type

Related issues

Screenshots

Test cases covered

submitContract.test.ts

  • 'uses email settings from database with remove-parameter-store flag on'

fetchMcReviewSettings.test.ts

  • 'uses email settings from database with remove-parameter-store flag on'

QA guidance

  • Since the defaults in Prisma are the same as dev we wont see any changes on the settings page. Just test that emails still send.

@JasonLin0991 JasonLin0991 changed the title MCR-4893: Adjust Apollo gql hanlder to use feature flag [DRAFT] MCR-4893: Adjust Apollo gql hanlder to use feature flag Jan 22, 2025
@JasonLin0991 JasonLin0991 changed the title [DRAFT] MCR-4893: Adjust Apollo gql hanlder to use feature flag MCR-4893: Adjust Apollo handler to use feature flag Jan 23, 2025
@JasonLin0991 JasonLin0991 marked this pull request as ready for review January 23, 2025 23:27
Copy link
Contributor

@pearl-truss pearl-truss left a comment

Choose a reason for hiding this comment

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

I think there's something going on. As a state user I was able to submit but didn't see any emails. One weird thing was going through the state submission form the Supporting Docs page was present but in LD the 'hide-supporting-docs' flag is turned on for dev

Also, as a CMS user I wasn't able to add a contract or rate question. I got the attached error regarding unable to send emails
Screenshot 2025-01-24 at 3 47 19 PM

@JasonLin0991
Copy link
Contributor Author

I think there's something going on. As a state user I was able to submit but didn't see any emails. One weird thing was going through the state submission form the Supporting Docs page was present but in LD the 'hide-supporting-docs' flag is turned on for dev

Also, as a CMS user I wasn't able to add a contract or rate question. I got the attached error regarding unable to send emails

Yea, looks like the way had the default emails in the Prisma schema caused email issues. The display name of the email is formatted incorrectly for AWS SES. We can use escape characters to make the default emails display name work, but it might cause issues with the escape characters. Instead I removed the email display name.

Copy link
Contributor

@haworku haworku left a comment

Choose a reason for hiding this comment

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

I would like to see some changes to the LDservice - I think rather than making up our own pattern we could use the anonymous user functionality of LD. I'm going to look into briefly see if its quick fix

@haworku
Copy link
Contributor

haworku commented Jan 29, 2025

FYI @pearl-truss the reason you saw that page is we have targetting on for aang user in LD. I did look into that

@haworku
Copy link
Contributor

haworku commented Jan 29, 2025

closing in favor of the new #3116

@haworku haworku closed this Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AI Assist Github Copilot test for DSG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants