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

GH-188 events_mgmt adding support for Adobe Runtime Actions as webhooks #187

Merged
merged 8 commits into from
Oct 13, 2023

Conversation

abhupadh
Copy link
Collaborator

@abhupadh abhupadh commented Oct 10, 2023

Description

Adding support for managing runtime webhook registrations

Related Issue

Motivation and Context

To enable integration testing for runtime webhook registrations.

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • 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 change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

@abhupadh abhupadh temporarily deployed to aio_stage October 11, 2023 00:50 — with GitHub Actions Inactive
@abhupadh abhupadh temporarily deployed to aio_prod October 11, 2023 00:50 — with GitHub Actions Inactive
Copy link
Collaborator

@francoisledroff francoisledroff left a comment

Choose a reason for hiding this comment

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

a few first comments

@francoisledroff francoisledroff changed the title feat: adding support for runtime webhook registrations GH-128 adding support for runtime webhook registrations Oct 12, 2023
@francoisledroff francoisledroff changed the title GH-128 adding support for runtime webhook registrations GH-128 adding support for runtime webhook registrations Oct 12, 2023
@francoisledroff francoisledroff changed the title GH-128 adding support for runtime webhook registrations GH-188 events_mgmt adding support for Adobe Runtime Actions as webhooks Oct 12, 2023
@abhupadh abhupadh temporarily deployed to aio_stage October 12, 2023 19:33 — with GitHub Actions Inactive
@abhupadh abhupadh temporarily deployed to aio_prod October 12, 2023 19:33 — with GitHub Actions Inactive
@abhupadh abhupadh temporarily deployed to aio_stage October 12, 2023 19:43 — with GitHub Actions Inactive
@abhupadh abhupadh temporarily deployed to aio_prod October 12, 2023 19:43 — with GitHub Actions Inactive
Copy link
Collaborator

@francoisledroff francoisledroff left a comment

Choose a reason for hiding this comment

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

a few requests:

@abhupadh abhupadh temporarily deployed to aio_stage October 12, 2023 20:41 — with GitHub Actions Inactive
@abhupadh abhupadh temporarily deployed to aio_prod October 12, 2023 20:41 — with GitHub Actions Inactive
Copy link
Collaborator

@francoisledroff francoisledroff left a comment

Choose a reason for hiding this comment

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

I'll propose some cleanup in my own PR

@@ -58,11 +61,16 @@ public class RegistrationUpdateModel {
this.webhookUrl = webhookUrl;
this.eventsOfInterestInputModels = eventsOfInterestInputModels;
if (DeliveryType.fromFriendlyName(deliveryType).isWebhookDelivery() && StringUtils.isEmpty(webhookUrl)) {
throw new IllegalArgumentException(
"Registration is a webhook registration, but missing a webhook url");
if (StringUtils.isNotEmpty(runtimeAction)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we move this condition inside the enclosing if by adding && StringUtils.isEmpty(runtimeAction)?

@francoisledroff
Copy link
Collaborator

I pushed some cleanup against your branch see #189

…elease in this open-source sdk (#189)

GH-188 code cleanup 
* removing the complexity/code we don't want to release in this open-source sdk
* plus adding a bit of doc on the new OAuth support
@francoisledroff francoisledroff temporarily deployed to aio_stage October 13, 2023 14:00 — with GitHub Actions Inactive
@francoisledroff francoisledroff temporarily deployed to aio_prod October 13, 2023 14:00 — with GitHub Actions Inactive
nicdard
nicdard previously approved these changes Oct 13, 2023
Copy link
Collaborator

@nicdard nicdard left a comment

Choose a reason for hiding this comment

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

LGTM

@francoisledroff francoisledroff temporarily deployed to aio_prod October 13, 2023 14:14 — with GitHub Actions Inactive
@francoisledroff francoisledroff temporarily deployed to aio_stage October 13, 2023 14:14 — with GitHub Actions Inactive
@francoisledroff francoisledroff merged commit e6a3358 into main Oct 13, 2023
@nicdard nicdard deleted the abhupadh-runtime-webhook-test branch October 13, 2023 18:11
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.

3 participants