TSD Push Notification plugin #165

hesyifei · 2019-03-13T05:18:13Z

This plugin was moved to a separate repo TheStanfordDaily/tsd-push-notification.

For:

Data Structure

CREATE TABLE wp_tsd_pn (
        id bigint(20) NOT NULL AUTO_INCREMENT,
        receiver_id bigint(20) unsigned NOT NULL,
        item_type varchar(20) NOT NULL,
        item_id bigint(20) unsigned NOT NULL,
        PRIMARY KEY (id),
        UNIQUE (receiver_id, item_type, item_id),
        INDEX (receiver_id, item_type),
        INDEX (item_type, item_id)
);

Suppose:

For subscriber 1144875:

  "subscribing": {
    "list": ["daily", "breaking", "weekly"],
    "category_ids": [2429],
    "author_ids": [239, 1002026],
    "location_ids": [9, 10, 11]
  }

For subscriber 1144876:

  "subscribing": {
    "list": ["breaking"],
    "category_ids": [2429, 4408],
    "author_ids": [239],
    "location_ids": [3, 9]
  }

Then,

mysql> SELECT * FROM wp_tsd_pn;
+-----+-------------+--------------+---------+
| id  | receiver_id | item_type    | item_id |
+-----+-------------+--------------+---------+
| 111 |     1144875 | author_ids   |     239 |
| 112 |     1144875 | author_ids   | 1002026 |
| 110 |     1144875 | category_ids |    2429 |
| 103 |     1144875 | list         |   51671 |
| 101 |     1144875 | list         |   51672 |
| 102 |     1144875 | list         |   51673 |
|  88 |     1144875 | location_ids |       9 |
|  89 |     1144875 | location_ids |      10 |
|  90 |     1144875 | location_ids |      11 |
| 115 |     1144876 | author_ids   |     239 |
| 113 |     1144876 | category_ids |    2429 |
| 114 |     1144876 | category_ids |    4408 |
| 104 |     1144876 | list         |   51673 |
| 116 |     1144876 | location_ids |       3 |
|  98 |     1144876 | location_ids |       9 |
+-----+-------------+--------------+---------+

`item_id`

item_id for:

item_type = list
Custom taxonomy tsd_push_msg_receiver_group’s term_id.
Note that the strings used in subscribing (such as "breaking") are the custom taxonomy's slug.
item_type = category_ids
Category’s term_id.
item_type = author_ids
Author’s ID.
item_type = location_ids
Location’s id defined by tsd_locations_plugin_get_locations().

Procedure of Sending Notification to Subscribers

When the “Publish” button of a normal post is clicked:

Find all authors / (if possible) co-authors of the post
- Find the subscriber for each author
- Add these subscriber ids to $notification_receiver_ids
Find all categories of the post
- Find the subscriber for each category
- Add these subscriber ids to $notification_receiver_ids
Find all tags of the post
- Find all locations related to the tags
- Find the subscriber for each location
- Add these subscriber ids to $notification_receiver_ids
Send the notification
- Title: Post title
- Body: Plain text of post content before  tag
- Data: [ "post_id" => $post_id ]
- To: array_unique( $notification_receiver_ids );

When the “Publish” button of a tsd_push_msg post is clicked:

Find all tsd_push_msg_receiver_groups of the post
- Add these subscriber ids to $notification_receiver_ids
Send the notification
- Title: Post title
- Body: Post excerpt
- Data: Custom fields which have keys starting with tsd_pn_.
  For example, having a custom field called tsd_pn_post_id will add post_id and its value to the data.
- To: array_unique( $notification_receiver_ids );

…/edit/publish push notifications

… in tsd_pn_receiver

…_push_msg`

…shing push notification

…ush-notification/users/{token}` Ref: https://restfulapi.net/rest-put-vs-post/

…h `tsd_pn_`)

…wp_tsd_pn

…ding custom post type is being deleted

epicfaace

Hey @hesyifei excellent work! I've added a few comments.

Few long-term things to keep in mind:

Your SQL is great 👌, but I think it might be better to use an ORM such as Propel2 or wp-eloquent. This will help for future maintainability.
Excellent function decomposition; I would really think we should work on adding unit tests using phpunit. This will also help for future maintainability; otherwise once we're gone, people may not touch this code and it may become legacy code then 😉

That being said, it is the end of the quarter and we have to be realistic, so I'm good with merging this and deploying this since it works, without necessarily doing these large rewrites. We will definitely need to work on unit tests though at least; I will work on getting at least a continuous integration pipeline running for these tests on this repo.

I think a solution going forward would be to spin this off into a separate plugin. It doesn't look like there are many Wordpress PN plugins out there, so spinning it off would be a great open source initiative that the Daily Tech Team could do. That way, we can get more contributors on the project, and who knows, maybe even the Expo team could pick it up. We could also assemble a team next quarter that would be in change of this!

Again, great work, and what I'm saying is mostly motivated by the fact that I think this code is well-written and could be spun off into a separate plugin for maintainability and helping others as well. Let me know what you think!

epicfaace · 2019-03-14T01:27:03Z

wp-content/plugins/tsd-push-notification/inc/cpt-push-msg.php

+		'not_found'             => 'No Push Notifications found.',
+		'not_found_in_trash'    => 'No Push Notifications found in Trash.',
+		'featured_image'        => 'Push Notification Feature Image', // Overrides the “Featured Image” phrase for this post type. Added in 4.3
+		'set_featured_image'    => 'Set feature image', // Overrides the “Set featured image” phrase for this post type. Added in 4.3


@hesyifei Can you remove these comments?

epicfaace · 2019-03-14T01:28:20Z

wp-content/plugins/tsd-push-notification/inc/rest-api-endpoints.php

+		return [];
+	}
+
+	// TODO: Find better way to store category_ids, author_ids, location_ids.


You can remove this todo comment

epicfaace · 2019-03-14T01:28:29Z

wp-content/plugins/tsd-push-notification/inc/rest-api-endpoints.php

+			'post_type' => 'tsd_pn_receiver',
+			'post_title' => $token,
+			'post_status' => "publish",
+			/*'meta_input' => [


You can remove this commented-out block of code

epicfaace · 2019-03-14T01:29:32Z

wp-content/plugins/tsd-push-notification/inc/rest-api-endpoints.php

+			tsd_create_update_pn_receiver( $post_id, $user_subscribing[ "category_ids" ], $user_subscribing[ "author_ids" ], $user_subscribing[ "location_ids" ] );
+		} else {
+			echo "WARNING: SHOULD NOT HAVE MORE THAN 1 POST WITH SAME NAME";
+			return [];  // TODO: return error;


I believe for returning errors, you can just use the WP_Error object: https://wordpress.stackexchange.com/questions/253445/wp-rest-api-how-to-change-http-response-status-code

epicfaace · 2019-03-14T01:29:55Z

wp-content/plugins/tsd-push-notification/inc/rest-api-endpoints.php

+			$post_id = tsd_create_new_pn_receiver( $user_token, $user_subscribing[ "category_ids" ], $user_subscribing[ "author_ids" ], $user_subscribing[ "location_ids" ] );
+		} else if ( count( $this_user ) == 1 ) {
+			$post_id = $this_user[0]->ID;
+			tsd_create_update_pn_receiver( $post_id, $user_subscribing[ "category_ids" ], $user_subscribing[ "author_ids" ], $user_subscribing[ "location_ids" ] );


You can remove this line of code

epicfaace · 2019-03-14T01:31:34Z

wp-content/plugins/tsd-push-notification/inc/send-notification.php

+
+	// Ref: https://docs.expo.io/versions/latest/guides/push-notifications/#http2-api
+	// TODO: "an array of up to 100 messages" - need divide 100
+	$response = wp_remote_post( "https://exp.host/--/api/v2/push/send", [


Suggested change

$response = wp_remote_post( "https://exp.host/--/api/v2/push/send", [

$response = wp_safe_remote_post( "https://exp.host/--/api/v2/push/send", [

epicfaace · 2019-03-14T01:33:19Z

wp-content/plugins/tsd-push-notification/inc/send-notification.php

+	}
+
+	tsd_pn_set_admin_notice( 'success', "Response: \n" . json_encode( $decoded_body ) . "\nYour message: \n" . json_encode( $message_body ) );
+	//wp_die( "Notification sent!<br />".$log_content, "Notification sent!", [ "response" => 200, "back_link" => true ] );


Remove this

epicfaace · 2019-03-14T01:34:38Z

wp-content/plugins/tsd-push-notification/tsd-push-notification.php

+
+	$charset_collate = $wpdb->get_charset_collate();
+
+	$sql = "CREATE TABLE $tsd_pn_db_table_name (


Suggested change

$sql = "CREATE TABLE $tsd_pn_db_table_name (

$sql = "CREATE TABLE IF NOT EXISTS $tsd_pn_db_table_name (

epicfaace · 2019-03-14T01:35:19Z

wp-content/plugins/tsd-push-notification/tsd-push-notification.php

+	$charset_collate = $wpdb->get_charset_collate();
+
+	$sql = "CREATE TABLE $tsd_pn_db_table_name (
+		id bigint(20) NOT NULL AUTO_INCREMENT,


I believe you can just use ints instead of bigints for these columns

epicfaace · 2019-03-14T01:40:37Z

wp-content/plugins/tsd-push-notification/inc/admin-menu.php

+	);
+
+	// DEBUG
+	//update_option( "tsd_pn_debug_info", [] );


Remove this

wp-content/plugins/tsd-push-notification/inc/auto-send-subscription.php

epicfaace · 2019-03-14T01:54:41Z

wp-content/plugins/tsd-push-notification/inc/rest-api-endpoints.php

+			return [];  // TODO: return error;
+		}
+
+		if ( $post_id != -1 ) {


Suggested change

if ( $post_id != -1 ) {

if ( $post_id !== -1 ) {

wp-content/plugins/tsd-push-notification/inc/auto-send-subscription.php

epicfaace · 2019-03-14T01:58:38Z

wp-content/plugins/tsd-push-notification/inc/auto-send-subscription.php

+			$notification_data
+		);
+
+		// DEBUG


Remove these two lines

hesyifei · 2019-03-22T19:59:59Z

Thanks for all the comments! 👍🏻

Since we moved this plugin to a separate repo TheStanfordDaily/tsd-push-notification, we will add TSD Push Notification Plugin as a submodule. See #167.

The comments were all addressed in the new repo. See also TheStanfordDaily/tsd-push-notification#1, TheStanfordDaily/tsd-push-notification#2.

Documentation was moved to https://github.com/TheStanfordDaily/tsd-push-notification/wiki.

Same as commit 675e91b. Forgot to add it when #165 was closed and #167 was merged.

hesyifei added 30 commits February 18, 2019 21:34

add basic custom post structure

9c64e25

[Push Notifications Plugin] set permission so that only admin can see…

29d0a06

…/edit/publish push notifications

[PN Plugin] add hook when publishing new push notification

81418b7

[PN Plugin] add custom taxonomy to store receiver group

3585549

[PN Plugin] use excerpt instead of editor for post

f6fe227

[PN Plugin] add tsd_pn_receiver custom post type to store app user info

cfa4538

[PN Plugin] add todo

5407293

Merge branch 'custom-theme' into push-notification-plugin

084e279

[PN Plugin] add support for sending notification to all users (posts)…

fc95485

… in tsd_pn_receiver

[PN Plugin] use publish_tsd_push_msg hook instead of `save_post_tsd…

f9af870

…_push_msg`

[PN Plugin] show success/error message using admin_notices when publi…

06e2884

…shing push notification

[PN Plugin] use PUT instead of POST and change endpoint format to `/p…

41fff56

…ush-notification/users/{token}` Ref: https://restfulapi.net/rest-put-vs-post/

Merge branch 'master' into push-notification-plugin

e3b6687

Merge branch 'master' into push-notification-plugin

18d8145

[PN Plugin] add support for custom data fields (they should begin wit…

d05b5c5

…h `tsd_pn_`)

[PN Plugin] minor change

309fd2b

[PN Plugin] minor change

1ae8f5b

Merge branch 'master' into push-notification-plugin

70f1516

Merge branch 'master' into push-notification-plugin

4e4a175

[PN Plugin] minor change

597bcc2

[PN Plugin] add basic support for storing subscription data in table …

9a7c800

…wp_tsd_pn

[PN Plugin] do not store subscription data as post meta

77e7840

[PN Plugin] add support for sending to specific group

e521874

[PN Plugin] minor change

2466044

[PN Plugin] remove related rows from wp_tsd_pn too when its correspon…

92892e1

…ding custom post type is being deleted

[PN Plugin] minor change

4e9cf48

[PN Plugin] reorganize code

38c5273

[PN Plugin] seperate send expo push notification function

6358e47

[PN Plugin] handle empty receiver or empty title case

c3c6ae2

[PN Plugin] minor change

07260db

hesyifei added 15 commits March 10, 2019 21:35

[PN Plugin] minor change

09aeb3a

[PN Plugin] use tsd_push_msg_receiver_group ID to store sub list

0b74475

[PN Plugin] support basic sending notification for post on publish

7085f7f

Merge branch 'master' into push-notification-plugin

51ec9a2

[PN Plugin] only send notification when publishing post post type

5f4126e

[PN Plugin] add basic support for folloing authors

4ef9ba3

[PN Plugin] add support for sending notification to author's followers

b317e82

[PN Plugin] minor change

c11caf7

[PN Plugin] add support for following categories

279047e

[PN Plugin] add note

8de0622

[PN Plugin] minor change in debug

8637e42

[Locations Plugin] add support for getting location ids from tags slubs

675e91b

[PN Plugin] add support for subscribing locations

b31d0b7

[PN Plugin] use option instead of transient to store debug

cc0dbb0

[PN Plugin] minor change

36e8da8

hesyifei requested a review from epicfaace March 13, 2019 05:18

hesyifei added the mobile label Mar 13, 2019

[PN Plugin] minor change

a4d78ba

epicfaace reviewed Mar 14, 2019

View reviewed changes

wp-content/plugins/tsd-push-notification/inc/auto-send-subscription.php Show resolved Hide resolved

epicfaace reviewed Mar 14, 2019

View reviewed changes

wp-content/plugins/tsd-push-notification/inc/auto-send-subscription.php Show resolved Hide resolved

epicfaace reviewed Mar 14, 2019

View reviewed changes

wp-content/plugins/tsd-push-notification/inc/auto-send-subscription.php

$notification_data

);

// DEBUG

Copy link

Member

epicfaace Mar 14, 2019

Choose a reason for hiding this comment

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

Remove these two lines

This was referenced Mar 22, 2019

Add unit tests TheStanfordDaily/tsd-push-notification#1

Open

Use an ORM TheStanfordDaily/tsd-push-notification#2

Open

Add TSD Push Notification Plugin as a submodule #167

Merged

hesyifei closed this Mar 22, 2019

hesyifei deleted the push-notification-plugin branch March 22, 2019 20:00

hesyifei added a commit that referenced this pull request Jun 13, 2019

[Locations Plugin] Add support for getting location ids from tags slubs

04cec55

Same as commit 675e91b. Forgot to add it when #165 was closed and #167 was merged.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TSD Push Notification plugin #165

TSD Push Notification plugin #165

hesyifei commented Mar 13, 2019 •

edited

Loading

epicfaace left a comment

epicfaace Mar 14, 2019

epicfaace Mar 14, 2019

epicfaace Mar 14, 2019

epicfaace Mar 14, 2019

epicfaace Mar 14, 2019

epicfaace Mar 14, 2019

epicfaace Mar 14, 2019

epicfaace Mar 14, 2019

epicfaace Mar 14, 2019

epicfaace Mar 14, 2019

epicfaace Mar 14, 2019

epicfaace Mar 14, 2019

hesyifei commented Mar 22, 2019

	$response = wp_remote_post( "https://exp.host/--/api/v2/push/send", [
	$response = wp_safe_remote_post( "https://exp.host/--/api/v2/push/send", [


		$charset_collate = $wpdb->get_charset_collate();

		$sql = "CREATE TABLE $tsd_pn_db_table_name (

	$sql = "CREATE TABLE $tsd_pn_db_table_name (
	$sql = "CREATE TABLE IF NOT EXISTS $tsd_pn_db_table_name (

TSD Push Notification plugin #165

TSD Push Notification plugin #165

Conversation

hesyifei commented Mar 13, 2019 • edited Loading

Data Structure

item_id

Procedure of Sending Notification to Subscribers

epicfaace left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hesyifei commented Mar 22, 2019

hesyifei commented Mar 13, 2019 •

edited

Loading

`item_id`