feat: add hub bookmarks to nodes #56

chickenn00dle · 2024-02-15T23:33:18Z

Closes 1206274567818686-as-1206445911335988

This PR adds the bookmarks dropdown that was previously only available on the hub to all nodes in the network.

To do this, we add a new network event which is triggered whenever a node is saved or updated, which propagates some basic node data to all nodes in the network. The nodes store this data as an option and use it to render this dropdown.

Testing Instructions

Have a network with at least two nodes setup (in addition to the hub)
With these changes checked out, go to wp-admin in the hub site, and select a node to edit via Newspack Network > Nodes > YOUR NODE
Click update (no need to make any changes)
Go to the event log in the hub via Newspack Network > Event Log
Confirm a nodes synced event appears:
Wait for the event to propagate to the nodes, then visit the wp-admin of a node site.
Confirm the dropdown menu is available and contains an entry for the hub as well as the other node in the network, but not for itself.
Do the same for the other node site.
Go back to the hub site and trash one of the nodes.
Confirm another nodes synced event in the event logs
Wait for the event to propagate
Go to wp-admin for the node that has not been removed from the network
Confirm the removed node is not present in the dropdown menu

Note: The removed node still has the dropdown and still thinks its part of the network. I think we should address cleaning up not only this dropdown, but all network settings for a removed node in a follow-up PR.

chickenn00dle · 2024-02-20T22:10:31Z

includes/class-data-listeners.php

+	 * @return array
+	 */
+	public static function nodes_synced( $nodes_data ) {
+		return [ 'nodes_data' => $nodes_data ];


I ran into some issues treating $data as an array in the incoming events class so had to massage this here to work with the nodes data array.

leogermani

Beautiful! Working great.

Left a couple of minor comments but it's great to see more people creating events :)

leogermani · 2024-02-21T12:52:42Z

includes/hub/admin/class-nodes-list.php

@@ -54,7 +54,7 @@ public static function posts_columns_values( $column, $post_id ) {
 				function ( $bookmark ) {
 					return sprintf( '<a href="%s" target="_blank">%s</a>', esc_url( $bookmark['url'] ), esc_html( $bookmark['label'] ) );
 				},
-				$node->get_bookmarks()
+				Node::get_bookmarks( $node->get_url() )


See my other comment, I think we could keep this method as it is

leogermani · 2024-02-21T12:54:30Z

includes/hub/class-node.php

 	 * @return array
 	 */
-	public function get_bookmarks() {
+	public static function get_bookmarks( $url ) {


This is kind of a nit, but can I ask that instead of making this method static, and then having to change its usage, can we create another static method that could be used in the new places

Then this method could be a shortcode for the other static method. It would just call it passing the url.

The new static method could live in this class, or maybe it makes sense for it to live in the Nodes (plural) class

Sure thing! Done in 9ac9775

leogermani · 2024-02-21T12:56:26Z

includes/hub/class-nodes.php

@@ -28,6 +27,7 @@ class Nodes {
 	public static function init() {
 		add_action( 'init', [ __CLASS__, 'register_post_type' ] );
 		add_action( 'save_post', [ __CLASS__, 'save_post' ] );
+		add_action( 'save_post_' . self::POST_TYPE_SLUG, [ __CLASS__, 'sync_nodes' ] );


Pleas use the newspack_network_node_saved action. We have had race condition issues around this #57

unrelated to this PR, but maybe as a safeguard we should make the callback that saves the nodes run on an higher priority (less then 10) here:

newspack-network/includes/hub/class-nodes.php

Line 30 in a9230d9

add_action( 'save_post', [ __CLASS__, 'save_post' ] );

Sure! One issue with using this action though is when a node is trashed from the edit posts page vs. the edit single post page. In this case the nonce is not present and so the sync event won't trigger.

Is it possible to inject the nonce field to the edit posts page? If not, I guess we can either replace the trash and restore URLs to include our nonces, or simply remove the option to trash from the edit posts page altogether.

I'd love your thoughts on this before I make any changes here.

I think you could listen to an additional hook to trigger this then

for example you listen to newspack_network_node_saved and also to trashed_post, calling the same callback.

for example you listen to newspack_network_node_saved and also to trashed_post, calling the same callback.

This works too! I wasn't sure if we wanted to avoid using the standard wordpress post hooks to avoid running into similar issues in the future. In any case I made the change in 1e590d1.

I also added some np network specific hooks for trashing and untrashing nodes in case we need to do more than just sync nodes in the future. This might be overkill though, so if you'd like I can remove these and just hook directly on the un/trashed_post hooks.

# [1.3.0-alpha.1](v1.2.0...v1.3.0-alpha.1) (2024-02-23) ### Bug Fixes * memberships sync ([#63](#63)) ([0a54f1d](0a54f1d)) ### Features * add hub bookmarks to nodes ([#56](#56)) ([54700a0](54700a0)) * add option to manually sync users ([#53](#53)) ([3ec1b19](3ec1b19)) * display membership plans from the network ([#59](#59)) ([6fa01fd](6fa01fd)) * enhance network subscriptions and orders search ([#55](#55)) ([bcb0615](bcb0615)) * Node connection using a link ([#58](#58)) ([721f8b2](721f8b2)) * **wp-cli:** add command to backfill events ([#51](#51)) ([13d2498](13d2498)) * **wp-cli:** add process pending webhook requests command ([#67](#67)) ([7dbd8dc](7dbd8dc)) * **wp-cli:** sync all events from the Hub ([#65](#65)) ([dc595ca](dc595ca))

matticbot · 2024-02-23T15:15:13Z

🎉 This PR is included in version 1.3.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

# [1.3.0](v1.2.0...v1.3.0) (2024-02-28) ### Bug Fixes * backfill duplicate handling; woo links in wp-admin-bar ([#71](#71), [#72](#72)) ([bbce13b](bbce13b)) * cli commands ([#73](#73)) ([ff563ac](ff563ac)) * memberships sync ([#63](#63)) ([0a54f1d](0a54f1d)) * missing woocommerce-memberships handling ([76dbdf7](76dbdf7)) ### Features * add hub bookmarks to nodes ([#56](#56)) ([54700a0](54700a0)) * add option to manually sync users ([#53](#53)) ([3ec1b19](3ec1b19)) * display membership plans from the network ([#59](#59)) ([6fa01fd](6fa01fd)) * enhance network subscriptions and orders search ([#55](#55)) ([bcb0615](bcb0615)) * Node connection using a link ([#58](#58)) ([721f8b2](721f8b2)) * **wp-cli:** add command to backfill events ([#51](#51)) ([13d2498](13d2498)) * **wp-cli:** add process pending webhook requests command ([#67](#67)) ([7dbd8dc](7dbd8dc)) * **wp-cli:** sync all events from the Hub ([#65](#65)) ([dc595ca](dc595ca))

matticbot · 2024-02-28T13:09:55Z

🎉 This PR is included in version 1.3.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

chickenn00dle force-pushed the feat/add-network-sites-dropdown-to-nodes branch 2 times, most recently from 586a727 to d88c134 Compare February 20, 2024 22:09

chickenn00dle commented Feb 20, 2024

View reviewed changes

chickenn00dle force-pushed the feat/add-network-sites-dropdown-to-nodes branch from 60c07dd to c0b423f Compare February 20, 2024 22:50

chickenn00dle marked this pull request as ready for review February 20, 2024 22:52

chickenn00dle requested a review from a team as a code owner February 20, 2024 22:52

chickenn00dle added the [Status] Needs Review label Feb 20, 2024

leogermani requested changes Feb 21, 2024

View reviewed changes

chickenn00dle added 5 commits February 22, 2024 11:26

feat: add hub bookmarks to nodes

ff0e0fb

feat: store node data as option

af6a429

feat: correctly store nodes data as array

1c3f3d5

chore: reorganize some code

7d8e3ae

chore: refactor get_bookmarks

9ac9775

chickenn00dle force-pushed the feat/add-network-sites-dropdown-to-nodes branch from c0b423f to 9ac9775 Compare February 22, 2024 18:27

chore: use network node hooks

1e590d1

chickenn00dle requested a review from leogermani February 22, 2024 19:38

leogermani approved these changes Feb 22, 2024

View reviewed changes

leogermani added [Status] Approved and removed [Status] Needs Review labels Feb 22, 2024

chickenn00dle merged commit 54700a0 into trunk Feb 22, 2024
3 checks passed

adekbadek deleted the feat/add-network-sites-dropdown-to-nodes branch February 23, 2024 09:15

matticbot added the released on @alpha label Feb 23, 2024

matticbot added the released label Feb 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add hub bookmarks to nodes #56

feat: add hub bookmarks to nodes #56

chickenn00dle commented Feb 15, 2024 •

edited

Loading

chickenn00dle Feb 20, 2024

leogermani left a comment

leogermani Feb 21, 2024

leogermani Feb 21, 2024

chickenn00dle Feb 22, 2024

leogermani Feb 21, 2024

chickenn00dle Feb 22, 2024 •

edited

Loading

leogermani Feb 22, 2024

chickenn00dle Feb 22, 2024

matticbot commented Feb 23, 2024

matticbot commented Feb 28, 2024

feat: add hub bookmarks to nodes #56

feat: add hub bookmarks to nodes #56

Conversation

chickenn00dle commented Feb 15, 2024 • edited Loading

Testing Instructions

chickenn00dle Feb 20, 2024

Choose a reason for hiding this comment

leogermani left a comment

Choose a reason for hiding this comment

leogermani Feb 21, 2024

Choose a reason for hiding this comment

leogermani Feb 21, 2024

Choose a reason for hiding this comment

chickenn00dle Feb 22, 2024

Choose a reason for hiding this comment

leogermani Feb 21, 2024

Choose a reason for hiding this comment

chickenn00dle Feb 22, 2024 • edited Loading

Choose a reason for hiding this comment

leogermani Feb 22, 2024

Choose a reason for hiding this comment

chickenn00dle Feb 22, 2024

Choose a reason for hiding this comment

matticbot commented Feb 23, 2024

matticbot commented Feb 28, 2024

chickenn00dle commented Feb 15, 2024 •

edited

Loading

chickenn00dle Feb 22, 2024 •

edited

Loading