Skip to content

Commit 8e4637a

Browse files
PatelUtkarshPatelUtkarsh
committed
Address review feedback: route abilities through existing classes
Keep abilities thin and delegate to the existing data-flow classes so the admin UI and the Abilities API stay on one code path. - Alerts: add STATUS_ENABLED / STATUS_DISABLED constants and a get_alerts() listing method; consume both from get-alerts ability. - Alert: add delete() method; consume from delete-alert ability. - create-alert ability now delegates the insert + meta save to Alert::save() instead of duplicating wp_insert_post / update_post_meta. - Connectors: add get_all() and get_slugs() helpers; consume from get-connectors and create-exclusion-rule abilities. - Record: add static get_by_id($id, $blog_id) for single-row + meta fetch; consume from get-record ability (multisite scoping kept in the ability layer, where the REST/network-admin distinction belongs). - Settings: add get_setting_value() that transparently reads the network option on network-activated multisite; Abilities loader uses it instead of its own multisite branching.
1 parent 4b2616e commit 8e4637a

12 files changed

Lines changed: 235 additions & 108 deletions

‎abilities/class-ability-create-alert.php‎

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ public function get_input_schema() {
7676
'status' => array(
7777
'type' => 'string',
7878
'description' => 'Initial alert status.',
79-
'enum' => array( 'wp_stream_enabled', 'wp_stream_disabled' ),
80-
'default' => 'wp_stream_enabled',
79+
'enum' => array( Alerts::STATUS_ENABLED, Alerts::STATUS_DISABLED ),
80+
'default' => Alerts::STATUS_ENABLED,
8181
),
8282
),
8383
);
@@ -94,7 +94,7 @@ public function get_output_schema() {
9494
'id' => array( 'type' => 'integer' ),
9595
'status' => array(
9696
'type' => 'string',
97-
'enum' => array( 'wp_stream_enabled', 'wp_stream_disabled' ),
97+
'enum' => array( Alerts::STATUS_ENABLED, Alerts::STATUS_DISABLED ),
9898
),
9999
'title' => array( 'type' => 'string' ),
100100
'alert_type' => array( 'type' => array( 'string', 'null' ) ),
@@ -112,7 +112,7 @@ public function get_output_schema() {
112112
* @param mixed $input Validated input matching get_input_schema(), or null.
113113
*/
114114
public function execute( $input = null ) {
115-
$status = isset( $input['status'] ) ? $input['status'] : 'wp_stream_enabled';
115+
$status = isset( $input['status'] ) ? $input['status'] : Alerts::STATUS_ENABLED;
116116

117117
// Validate alert_type against the registered notifier slugs. The schema
118118
// can't enum these because alert types are extensible via the
@@ -156,33 +156,30 @@ public function execute( $input = null ) {
156156
)
157157
);
158158

159-
// Build an Alert model so we can reuse Stream's title-generation logic
160-
// (otherwise wp_insert_post stores 'Auto Draft' for empty post_title).
161-
$alert_model = new Alert(
159+
// Delegate the actual insert + meta save to Alert::save() so the
160+
// admin UI and the Abilities API stay on a single code path.
161+
$alert = new Alert(
162162
(object) array(
163163
'alert_type' => $input['alert_type'],
164164
'alert_meta' => $alert_meta,
165165
'status' => $status,
166166
),
167167
$this->plugin
168168
);
169-
$post_title = $alert_model->get_title();
170169

171-
$post_id = wp_insert_post(
172-
array(
173-
'post_type' => Alerts::POST_TYPE,
174-
'post_status' => $status,
175-
'post_title' => $post_title,
176-
),
177-
true
178-
);
170+
$post_id = $alert->save();
179171

180172
if ( is_wp_error( $post_id ) ) {
181173
return $post_id;
182174
}
183175

184-
update_post_meta( $post_id, 'alert_type', $input['alert_type'] );
185-
update_post_meta( $post_id, 'alert_meta', $alert_meta );
176+
if ( empty( $post_id ) ) {
177+
return new \WP_Error(
178+
'stream_alert_create_failed',
179+
__( 'Failed to create alert.', 'stream' ),
180+
array( 'status' => 500 )
181+
);
182+
}
186183

187184
return array(
188185
'id' => (int) $post_id,

‎abilities/class-ability-create-exclusion-rule.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,8 @@ public function execute( $input = null ) {
154154

155155
// Validate connector against registered connectors when provided.
156156
if ( '' !== $sanitized['connector'] ) {
157-
$known = isset( $this->plugin->connectors->connectors )
158-
? array_keys( (array) $this->plugin->connectors->connectors )
157+
$known = isset( $this->plugin->connectors )
158+
? $this->plugin->connectors->get_slugs()
159159
: array();
160160
if ( ! empty( $known ) && ! in_array( $sanitized['connector'], $known, true ) ) {
161161
return new \WP_Error(

‎abilities/class-ability-delete-alert.php‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,21 +83,34 @@ public function get_output_schema() {
8383
* @param mixed $input Validated input matching get_input_schema(), or null.
8484
*/
8585
public function execute( $input = null ) {
86-
$id = isset( $input['id'] ) ? (int) $input['id'] : 0;
87-
$post = $id > 0 ? get_post( $id ) : null;
86+
$id = isset( $input['id'] ) ? (int) $input['id'] : 0;
8887

89-
if ( ! $post || Alerts::POST_TYPE !== $post->post_type ) {
88+
// Load via Alerts::get_alert() so we can leverage the existing factory.
89+
// It returns an Alert with ID=null for unknown post IDs, which we treat
90+
// as not-found. The Alert::delete() method then double-checks the post
91+
// type to refuse deleting non-alert posts.
92+
$alert = $id > 0 ? $this->plugin->alerts->get_alert( $id ) : null;
93+
94+
if ( ! $alert || empty( $alert->ID ) ) {
9095
return new \WP_Error(
9196
'stream_alert_not_found',
9297
__( 'Alert not found.', 'stream' ),
9398
array( 'status' => 404 )
9499
);
95100
}
96101

97-
$result = wp_delete_post( $id, true );
102+
$result = $alert->delete();
103+
104+
if ( ! $result ) {
105+
return new \WP_Error(
106+
'stream_alert_not_found',
107+
__( 'Alert not found.', 'stream' ),
108+
array( 'status' => 404 )
109+
);
110+
}
98111

99112
return array(
100-
'deleted' => (bool) $result,
113+
'deleted' => true,
101114
'id' => $id,
102115
);
103116
}

‎abilities/class-ability-get-alerts.php‎

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public function get_output_schema() {
8080
'id' => array( 'type' => 'integer' ),
8181
'status' => array(
8282
'type' => 'string',
83-
'enum' => array( 'wp_stream_enabled', 'wp_stream_disabled' ),
83+
'enum' => array( Alerts::STATUS_ENABLED, Alerts::STATUS_DISABLED ),
8484
),
8585
'title' => array( 'type' => 'string' ),
8686
'alert_type' => array( 'type' => array( 'string', 'null' ) ),
@@ -103,39 +103,32 @@ public function execute( $input = null ) {
103103

104104
switch ( $requested ) {
105105
case 'enabled':
106-
$statuses = array( 'wp_stream_enabled' );
106+
$statuses = array( Alerts::STATUS_ENABLED );
107107
break;
108108
case 'disabled':
109-
$statuses = array( 'wp_stream_disabled' );
109+
$statuses = array( Alerts::STATUS_DISABLED );
110110
break;
111111
default:
112-
$statuses = array( 'wp_stream_enabled', 'wp_stream_disabled' );
112+
$statuses = array( Alerts::STATUS_ENABLED, Alerts::STATUS_DISABLED );
113113
}
114114

115-
$posts = get_posts(
116-
array(
117-
'post_type' => Alerts::POST_TYPE,
118-
'post_status' => $statuses,
119-
'posts_per_page' => -1, // phpcs:ignore WordPress.WP.PostsPerPage.posts_per_page_posts_per_page
120-
)
121-
);
115+
$alerts = $this->plugin->alerts->get_alerts( $statuses );
122116

123117
$out = array();
124-
foreach ( $posts as $post ) {
125-
// get_post_meta() returns '' (string) when the key is missing; an
126-
// empty PHP array() also JSON-encodes as a list ([]), which violates
127-
// the declared object output schema. Normalize both cases to a real
128-
// object so wp_json_encode() emits {} when there is no meta.
129-
$alert_meta = get_post_meta( $post->ID, 'alert_meta', true );
130-
if ( ! is_array( $alert_meta ) || array() === $alert_meta ) {
131-
$alert_meta = new \stdClass();
132-
}
118+
foreach ( $alerts as $alert ) {
119+
// Alert::$alert_meta defaults to array(); an empty PHP array() also
120+
// JSON-encodes as a list ([]), which violates the declared object
121+
// output schema. Normalize empty/non-array values to a real object
122+
// so wp_json_encode() emits {} when there is no meta.
123+
$alert_meta = is_array( $alert->alert_meta ) && ! empty( $alert->alert_meta )
124+
? $alert->alert_meta
125+
: new \stdClass();
133126

134127
$out[] = array(
135-
'id' => (int) $post->ID,
136-
'status' => (string) $post->post_status,
137-
'title' => (string) $post->post_title,
138-
'alert_type' => get_post_meta( $post->ID, 'alert_type', true ),
128+
'id' => (int) $alert->ID,
129+
'status' => (string) $alert->status,
130+
'title' => (string) get_the_title( $alert->ID ),
131+
'alert_type' => $alert->alert_type,
139132
'alert_meta' => $alert_meta,
140133
);
141134
}

‎abilities/class-ability-get-connectors.php‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -97,18 +97,10 @@ public function get_output_schema() {
9797
public function execute( $input = null ) {
9898
unset( $input );
9999

100-
$out = array();
101-
$connectors = isset( $this->plugin->connectors->connectors ) ? (array) $this->plugin->connectors->connectors : array();
102-
103-
foreach ( $connectors as $slug => $connector ) {
104-
$out[] = array(
105-
'slug' => (string) $slug,
106-
'label' => method_exists( $connector, 'get_label' ) ? (string) $connector->get_label() : (string) $slug,
107-
'contexts' => method_exists( $connector, 'get_context_labels' ) ? (array) $connector->get_context_labels() : array(),
108-
'actions' => method_exists( $connector, 'get_action_labels' ) ? (array) $connector->get_action_labels() : array(),
109-
);
100+
if ( ! isset( $this->plugin->connectors ) ) {
101+
return array();
110102
}
111103

112-
return $out;
104+
return $this->plugin->connectors->get_all();
113105
}
114106
}

‎abilities/class-ability-get-record.php‎

Lines changed: 11 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -100,33 +100,20 @@ public function get_output_schema() {
100100
* @param mixed $input Validated input matching get_input_schema(), or null.
101101
*/
102102
public function execute( $input = null ) {
103-
global $wpdb;
104-
105103
$id = isset( $input['id'] ) ? (int) $input['id'] : 0;
106104

107-
// Stream's Query class doesn't expose a single-ID filter (record__in
108-
// is broken for one-element arrays due to array_shift() in
109-
// Query::query()), so query the table directly. On any multisite
110-
// install, scope reads to the current blog unless the request is
111-
// running inside Network Admin — this mirrors Network::network_query_args()
112-
// (default blog_id is get_current_blog_id() outside network admin) and
113-
// applies the same protection in REST contexts, where is_network_admin()
114-
// is always false. Without this guard, a user with view_stream on one
115-
// site of a network-activated install could fetch records from other
116-
// sites by guessing IDs.
117-
$where = '';
118-
$prepared = array( $id );
119-
if ( is_multisite() && ! is_network_admin() ) {
120-
$where = ' AND blog_id = %d';
121-
$prepared[] = get_current_blog_id();
122-
}
105+
// On any multisite install, scope reads to the current blog unless the
106+
// request is running inside Network Admin — this mirrors
107+
// Network::network_query_args() (default blog_id is get_current_blog_id()
108+
// outside network admin) and applies the same protection in REST
109+
// contexts, where is_network_admin() is always false. Without this
110+
// guard, a user with view_stream on one site of a network-activated
111+
// install could fetch records from other sites by guessing IDs.
112+
$blog_id = ( is_multisite() && ! is_network_admin() )
113+
? get_current_blog_id()
114+
: null;
123115

124-
// $wpdb->stream and {$where} are constructed from string literals in this
125-
// method (no user input), and $prepared holds only an int id and (on
126-
// multisite) the integer blog id from get_current_blog_id().
127-
$sql = "SELECT * FROM {$wpdb->stream} WHERE ID = %d{$where}";
128-
// phpcs:ignore WordPress.DB.DirectDatabaseQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.PreparedSQL.NotPrepared
129-
$row = $wpdb->get_row( $wpdb->prepare( $sql, $prepared ), ARRAY_A );
116+
$row = Record::get_by_id( $id, $blog_id );
130117

131118
if ( empty( $row ) ) {
132119
return new \WP_Error(
@@ -136,8 +123,6 @@ public function execute( $input = null ) {
136123
);
137124
}
138125

139-
$row['meta'] = (array) get_metadata( 'record', $row['ID'] );
140-
141126
return $row;
142127
}
143128
}

‎classes/class-abilities.php‎

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -167,34 +167,19 @@ public function is_available() {
167167
/**
168168
* Whether the integration is enabled in Stream settings.
169169
*
170-
* Reads the network-level option directly when Stream is network-activated,
171-
* because Settings::get_options() only loads from get_site_option() inside
172-
* is_network_admin() screens. In REST and frontend contexts on a
173-
* network-activated install, $plugin->settings->options reflects the
174-
* (typically empty) per-site option, which would silently keep the
175-
* Abilities API disabled regardless of the network admin's setting.
170+
* Delegates to Settings::get_setting_value() so the multisite/network
171+
* fallback logic lives next to the settings storage.
176172
*
177173
* @return bool
178174
*/
179175
public function is_enabled() {
180-
$key = 'advanced_' . self::SETTING_NAME;
181-
182-
if (
183-
is_multisite()
184-
&& isset( $this->plugin->settings )
185-
&& $this->plugin->is_network_activated()
186-
) {
187-
$options = (array) get_site_option(
188-
$this->plugin->settings->network_options_key,
189-
array()
190-
);
191-
} else {
192-
$options = isset( $this->plugin->settings )
193-
? (array) $this->plugin->settings->options
194-
: array();
176+
if ( ! isset( $this->plugin->settings ) ) {
177+
return false;
195178
}
196179

197-
return ! empty( $options[ $key ] );
180+
return ! empty(
181+
$this->plugin->settings->get_setting_value( 'advanced_' . self::SETTING_NAME )
182+
);
198183
}
199184

200185
/**

‎classes/class-alert.php‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,24 @@ public function save() {
125125
return $post_id;
126126
}
127127

128+
/**
129+
* Permanently delete the underlying alert post.
130+
*
131+
* @return bool True if the post was deleted, false otherwise.
132+
*/
133+
public function delete() {
134+
if ( empty( $this->ID ) ) {
135+
return false;
136+
}
137+
138+
$post = get_post( $this->ID );
139+
if ( ! ( $post instanceof \WP_Post ) || Alerts::POST_TYPE !== $post->post_type ) {
140+
return false;
141+
}
142+
143+
return (bool) wp_delete_post( $this->ID, true );
144+
}
145+
128146
/**
129147
* Process settings form data
130148
*

‎classes/class-alerts.php‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,16 @@ class Alerts {
1919
*/
2020
const POST_TYPE = 'wp_stream_alerts';
2121

22+
/**
23+
* Enabled alert post status slug.
24+
*/
25+
const STATUS_ENABLED = 'wp_stream_enabled';
26+
27+
/**
28+
* Disabled alert post status slug.
29+
*/
30+
const STATUS_DISABLED = 'wp_stream_disabled';
31+
2232
/**
2333
* Triggered Alerts meta key for Records
2434
*/
@@ -427,6 +437,35 @@ public function get_alert( $post_id = '' ) {
427437
return new Alert( $obj, $this->plugin );
428438
}
429439

440+
/**
441+
* Return a list of alerts, optionally filtered by post status.
442+
*
443+
* @param array $statuses Optional list of alert post statuses to include.
444+
* Defaults to enabled + disabled.
445+
*
446+
* @return Alert[]
447+
*/
448+
public function get_alerts( array $statuses = array() ) {
449+
if ( empty( $statuses ) ) {
450+
$statuses = array( self::STATUS_ENABLED, self::STATUS_DISABLED );
451+
}
452+
453+
$posts = get_posts(
454+
array(
455+
'post_type' => self::POST_TYPE,
456+
'post_status' => $statuses,
457+
'posts_per_page' => -1, // phpcs:ignore WordPress.WP.PostsPerPage.posts_per_page_posts_per_page
458+
)
459+
);
460+
461+
$alerts = array();
462+
foreach ( $posts as $post ) {
463+
$alerts[] = $this->get_alert( $post->ID );
464+
}
465+
466+
return $alerts;
467+
}
468+
430469
/**
431470
* Add custom post type to menu
432471
*

0 commit comments

Comments
 (0)