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

GAP-2487: Replace contentful webhooks with synchronous api call #251

Merged
merged 4 commits into from
Mar 14, 2024

Conversation

dominicwest
Copy link
Contributor

@dominicwest dominicwest commented Mar 14, 2024

Description

Ticket GAP-2487

Summary of the changes and the related issue. List any dependencies that are required for this change:

  • Replacing contentfuls webhooks that update elastic search with manual api calls from our backend
  • Prevents race conditions in the webhooks
  • Prevents async nature of webhooks sometimes taking 15 mins to trigger

TODO:

  • Refactor cypress to seed elastic when seeding contentful
  • Infra PR to add secrets to admin backend task def

Type of change

Please check the relevant options.

  • 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).
  • This change requires a documentation update.

How Has This Been Tested?

Please describe the tests that you ran to verify your changes:

  • Unit Test

  • Integration Test (if applicable)

  • End-to-End Test (if applicable)

Screenshots (if appropriate):

Please attach screenshots of the change if it is a UI change:

Screen.Recording.2024-03-14.at.13.17.40.mov

Checklist:

  • If I have listed dependencies above, I have ensured that they are present in the target branch.
  • I have performed a self-review of my code.
  • I have commented my code in hard-to-understand areas.
  • I have made corresponding changes to the documentation where applicable.
  • I have run cypress tests, and they all pass locally.

@dominicwest dominicwest marked this pull request as ready for review March 14, 2024 13:20
@@ -22,6 +23,7 @@ public ObjectMapper getObjectMapper() {
objectMapper.disable(FAIL_ON_UNKNOWN_PROPERTIES);
objectMapper.registerModule(new JavaTimeModule());
objectMapper.registerModule(new ParameterNamesModule(JsonCreator.Mode.PROPERTIES));
objectMapper.setSerializationInclusion(JsonInclude.Include.NON_NULL);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has potential to affect other places which scares me a bit

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah depends on wether we check if a field exists by name rather than value

@dominicwest dominicwest merged commit 87ef8ca into develop Mar 14, 2024
1 check passed
@arul-fourseals arul-fourseals deleted the GAP-2487-replace-contentful-webhooks branch August 27, 2024 16:48
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