-
Notifications
You must be signed in to change notification settings - Fork 191
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
Feature: Notification Handler endpoints #7356
base: develop
Are you sure you want to change the base?
Conversation
give.php
Outdated
@@ -58,6 +58,7 @@ | |||
use Give\Form\Templates; | |||
use Give\FormBuilder\ServiceProvider as FormBuilderServiceProvider; | |||
use Give\FormMigration\ServiceProvider as FormMigrationServiceProvider; | |||
use Give\NotificationsHandler\ServiceProvider as NotificationHandlerServiceProvider; |
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.
Looks like we are giving NotificationsHandler
it's own top level domain in the src folder. I'm wondering if this should actually be in the FormBuilder
domain or if we do want to make this more general like Notifications
to leave room for other notification type apis.
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.
Good point. For me, this is not entirely the Form Builder thing, but I guess it would make sense to move this to the FB domain.
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.
On another thought, I think having Notifications TLD might be a better option.
* | ||
* @unreleased | ||
*/ | ||
class DismissNotification implements RestRoute |
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.
Let's write a test for this 😄
@pauloiankoski made a RestApiTestCase
for rest api testing specifically - but you can also do a unit test for handleRequest
and mock the response like i've done here
This PR is stale because it has been open 45 days with no activity. Stale PRs will NOT be automatically closed. |
Description
This PR adds endpoints for handling notifications for the new
NotificationPlaceholder
componentimpress-org/form-builder-library#8
Pre-review Checklist