#2014 - Add callback url to GET and POST requests #2029
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kalbfled
requested changes
Oct 8, 2024
There was a problem hiding this comment.
@k-macmillan Yikes! I missed this during #1436. Tests should be using the sample_notification fixture and not using create_notification. I'm not asking Mack to change that for this ticket.
| 'callback_url', | ||
| [ | ||
| ('invalid-url', 'is not a valid URI.'), | ||
| ('htp://wrongformat.com', 'does not match ^https.*'), |
There was a problem hiding this comment.
I put htp but http makes more sense.
Co-authored-by: David Kalbfleisch <[email protected]>
Co-authored-by: David Kalbfleisch <[email protected]>
Co-authored-by: David Kalbfleisch <[email protected]>
Co-authored-by: David Kalbfleisch <[email protected]>
Co-authored-by: David Kalbfleisch <[email protected]>
kalbfled
previously approved these changes
Oct 8, 2024
cris-oddball
requested changes
Oct 8, 2024
|
|
||
| assert resp_json['id'] == str(notification.id) | ||
| assert resp_json['content']['body'] == template.content.replace('(( Name))', 'Jo') | ||
| assert 'callback_url' not in resp_json or resp_json['callback_url'] is None |
There was a problem hiding this comment.
callback_url should always be in the response?
shouldn't we just check for resp_json['callback_url'] is None ?
There was a problem hiding this comment.
Ops - Yes that should be just resp_json['callback_url'] is None
Updated.
kalbfled
approved these changes
Oct 8, 2024
cris-oddball
approved these changes
Oct 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
callback_urlvalue to response in related GET and POST notifications request.callback_urlin body of POST request, update value in DB.htttps_urlschema because of impact on POST to ServiceCallbackDescription
issue #2014
How Has This Been Tested?
Tested in Postman and confirmed that
callback_urlcan be added to the Notification ObjectPOST Notification SMS
GET Notification
POST Notification Email
POST Notification Validation Errors
Checklist