-
Notifications
You must be signed in to change notification settings - Fork 278
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
Issue / 8735 Update PAX termsAndConditions.notify
Response
#8736
Issue / 8735 Update PAX termsAndConditions.notify
Response
#8736
Conversation
…otify callback returns empty object.
Build files for e7376e4 have been deleted. |
Size Change: -1 B (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @10upsimon – this LGTM there are just a few minor corrections to make in the test
describe( 'termsAndConditionsService', () => { | ||
it( 'notify callback should return an empty object', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hierarchy has gotten off here, although it works. Let's correct it for consistency.
It should have
- describe service
- - describe method
- - - it ...
termsAndConditionsService
is missing the inner describe fornotify
getSupportedConversionTrackingTypes
is out of place (not nested under its service)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in latest commit thanks @aaemnnosttv
Co-authored-by: Evan Mattson <emattson@google.com>
…b.com:google/site-kit-wp into issue/8735-update-pax-t-and-c-notify-response.
Summary
Addresses issue:
termsAndConditionsService.notify
Function Body Being Empty #8735Relevant technical choices
assets/js/modules/ads/pax/services.js
termsAndConditions.notify
function body to return empty object,return {};
assets/js/modules/ads/pax/services.test.js
termsAndConditionsService
to assert thattermsAndConditions.notify
returns an empty objectPR Author Checklist
Do not alter or remove anything below. The following sections will be managed by moderators only.
Code Reviewer Checklist
Merge Reviewer Checklist