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

feat(feature-manager): remove webhooks, verified content #2159

Merged
merged 3 commits into from
Jun 21, 2021

Conversation

mantariksh
Copy link
Contributor

@mantariksh mantariksh commented Jun 14, 2021

Removes webhooks and verified content from feature manager.

Note that unlike many of the other modules, the .factory file in the webhooks module was not deleted. This is to make the dependencies between the various webhook modules (service, producer, consumer) clearer by putting all the initialisation in one place.

Part of #1842

Breaking Changes

  • No - this PR is backwards compatible

Tests

  • Webhooks work in development environment

Manual tests

  • Create a storage mode form with verified fields. Submit and check that decrypted responses works, with the "[verified]" prefix for verified fields.
  • Create a SP-authenticated storage mode form. Submit and check that the NRIC can be decrypted.
  • Create a webhook server. Set the server to return a non-200 status the first time, then subsequently return 200. Enable retries and check that the webhook is received and decrypted correctly for both the initial attempt (which returns non-200) and the retry (which returns 200).

Deployment changes

Note that the server can no longer start without the presence of the SIGNING_SECRET_KEY env var. A default has been added here and in the FormSG SDK to make onboarding easier for developers.

@mantariksh
Copy link
Contributor Author

Converting to draft while figuring out a better way to ensure backwards compatibility for dev environment

@mantariksh mantariksh marked this pull request as draft June 15, 2021 01:54
@mantariksh mantariksh force-pushed the remove-fm/webhook-verified-content branch from 10d1a88 to 8bcf2c6 Compare June 18, 2021 10:18
@mantariksh mantariksh marked this pull request as ready for review June 18, 2021 10:21
@mantariksh mantariksh requested review from karrui and tshuli June 21, 2021 02:48
Copy link
Contributor

@karrui karrui left a comment

Choose a reason for hiding this comment

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

lgtm

@mantariksh mantariksh force-pushed the remove-fm/webhook-verified-content branch from 39dfad0 to ffad058 Compare June 21, 2021 03:48
@mantariksh mantariksh merged commit 7c07f2f into develop Jun 21, 2021
@tshuli tshuli mentioned this pull request Jun 22, 2021
@mantariksh mantariksh deleted the remove-fm/webhook-verified-content branch June 23, 2021 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants