Skip to content

Commit e1b6a37

Browse files
hogan-yuanclaude
hogan-yuan
and
claude
committed
fix(alert): pass AlertItem instead of alert_id to enable/disable
The previous fix used list() internally to find the alert, which fails if the list is paginated or the alert is not returned. Instead, change the enable/disable signatures to accept an AlertItem (obtained by the caller from list()), eliminating the implicit extra round-trip and the "not found" risk. Changes: - rust/src/alert/context.rs: enable/disable take &AlertItem - rust/src/blocking/alert.rs: blocking wrappers take AlertItem by value - python/src/alert/: add From<AlertItem> for lb::AlertItem; binding uses AlertItem - nodejs/src/alert/: add From<AlertItem> for lb::AlertItem; binding uses AlertItem - java/src/alert_context.rs: JNI deserialises AlertItem object instead of String Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent ed91edc commit e1b6a37

7 files changed

Lines changed: 78 additions & 55 deletions

File tree

‎java/src/alert_context.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,14 +84,14 @@ pub unsafe extern "system" fn Java_com_longbridge_SdkNative_alertContextEnable(
8484
mut env: JNIEnv,
8585
_class: JClass,
8686
context: i64,
87-
alert_id: JObject,
87+
item: JObject,
8888
callback: JObject,
8989
) {
9090
jni_result(&mut env, (), |env| {
9191
let context = &*(context as *const ContextObj);
92-
let id: String = FromJValue::from_jvalue(env, alert_id.into())?;
92+
let alert_item: longbridge::alert::AlertItem = FromJValue::from_jvalue(env, item.into())?;
9393
async_util::execute(env, callback, async move {
94-
context.ctx.enable(id).await?;
94+
context.ctx.enable(&alert_item).await?;
9595
Ok(())
9696
})?;
9797
Ok(())
@@ -103,14 +103,14 @@ pub unsafe extern "system" fn Java_com_longbridge_SdkNative_alertContextDisable(
103103
mut env: JNIEnv,
104104
_class: JClass,
105105
context: i64,
106-
alert_id: JObject,
106+
item: JObject,
107107
callback: JObject,
108108
) {
109109
jni_result(&mut env, (), |env| {
110110
let context = &*(context as *const ContextObj);
111-
let id: String = FromJValue::from_jvalue(env, alert_id.into())?;
111+
let alert_item: longbridge::alert::AlertItem = FromJValue::from_jvalue(env, item.into())?;
112112
async_util::execute(env, callback, async move {
113-
context.ctx.disable(id).await?;
113+
context.ctx.disable(&alert_item).await?;
114114
Ok(())
115115
})?;
116116
Ok(())

‎nodejs/src/alert/context.rs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,16 +46,20 @@ impl AlertContext {
4646
}
4747

4848
/// Enable a previously disabled price alert.
49+
///
50+
/// Pass the [`AlertItem`] obtained from [`list`](Self::list).
4951
#[napi]
50-
pub async fn enable(&self, alert_id: String) -> Result<()> {
51-
self.ctx.enable(alert_id).await.map_err(ErrorNewType)?;
52+
pub async fn enable(&self, item: AlertItem) -> Result<()> {
53+
self.ctx.enable(&item.into()).await.map_err(ErrorNewType)?;
5254
Ok(())
5355
}
5456

5557
/// Disable a price alert without deleting it.
58+
///
59+
/// Pass the [`AlertItem`] obtained from [`list`](Self::list).
5660
#[napi]
57-
pub async fn disable(&self, alert_id: String) -> Result<()> {
58-
self.ctx.disable(alert_id).await.map_err(ErrorNewType)?;
61+
pub async fn disable(&self, item: AlertItem) -> Result<()> {
62+
self.ctx.disable(&item.into()).await.map_err(ErrorNewType)?;
5963
Ok(())
6064
}
6165

‎nodejs/src/alert/types.rs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,21 @@ impl From<lb::AlertItem> for AlertItem {
3636
}
3737
}
3838

39+
impl From<AlertItem> for lb::AlertItem {
40+
fn from(v: AlertItem) -> Self {
41+
Self {
42+
id: v.id,
43+
indicator_id: v.indicator_id,
44+
enabled: v.enabled,
45+
frequency: v.frequency,
46+
scope: v.scope,
47+
text: v.text,
48+
state: v.state,
49+
value_map: v.value_map,
50+
}
51+
}
52+
}
53+
3954
/// Alert items for one security
4055
#[napi_derive::napi(object)]
4156
#[derive(Debug, Clone)]

‎python/src/alert/context.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,12 @@ impl AlertContext {
3333
.map_err(ErrorNewType)?;
3434
Ok(())
3535
}
36-
fn enable(&self, alert_id: String) -> PyResult<()> {
37-
self.ctx.enable(alert_id).map_err(ErrorNewType)?;
36+
fn enable(&self, item: AlertItem) -> PyResult<()> {
37+
self.ctx.enable(item.into()).map_err(ErrorNewType)?;
3838
Ok(())
3939
}
40-
fn disable(&self, alert_id: String) -> PyResult<()> {
41-
self.ctx.disable(alert_id).map_err(ErrorNewType)?;
40+
fn disable(&self, item: AlertItem) -> PyResult<()> {
41+
self.ctx.disable(item.into()).map_err(ErrorNewType)?;
4242
Ok(())
4343
}
4444
fn delete(&self, alert_ids: Vec<String>) -> PyResult<()> {

‎python/src/alert/types.rs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,21 @@ impl From<lb::AlertItem> for AlertItem {
4949
}
5050
}
5151

52+
impl From<AlertItem> for lb::AlertItem {
53+
fn from(v: AlertItem) -> Self {
54+
Self {
55+
id: v.id,
56+
indicator_id: v.indicator_id,
57+
enabled: v.enabled,
58+
frequency: v.frequency,
59+
scope: v.scope,
60+
text: v.text,
61+
state: v.state,
62+
value_map: v.value_map.0,
63+
}
64+
}
65+
}
66+
5267
#[pyclass(get_all)]
5368
#[derive(Debug, Clone)]
5469
pub(crate) struct AlertSymbolGroup {

‎rust/src/alert/context.rs‎

Lines changed: 26 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -144,45 +144,40 @@ impl AlertContext {
144144

145145
/// Enable a price alert.
146146
///
147+
/// Requires the [`AlertItem`] from [`list`](Self::list) so that all required
148+
/// fields (`indicator_id`, `frequency`, `scope`, `state`, `value_map`) are
149+
/// available without an extra round-trip.
150+
///
147151
/// Path: `POST /v1/notify/reminders`
148-
pub async fn enable(&self, alert_id: impl Into<String>) -> Result<serde_json::Value> {
149-
self.set_enabled(alert_id.into(), true).await
152+
pub async fn enable(&self, item: &AlertItem) -> Result<serde_json::Value> {
153+
self.set_enabled(item, true).await
150154
}
151155

152156
/// Disable a price alert.
153157
///
158+
/// Requires the [`AlertItem`] from [`list`](Self::list) so that all required
159+
/// fields (`indicator_id`, `frequency`, `scope`, `state`, `value_map`) are
160+
/// available without an extra round-trip.
161+
///
154162
/// Path: `POST /v1/notify/reminders`
155-
pub async fn disable(&self, alert_id: impl Into<String>) -> Result<serde_json::Value> {
156-
self.set_enabled(alert_id.into(), false).await
163+
pub async fn disable(&self, item: &AlertItem) -> Result<serde_json::Value> {
164+
self.set_enabled(item, false).await
157165
}
158166

159-
/// Internal: fetch alert by id and re-POST with updated enabled flag.
160-
async fn set_enabled(&self, alert_id: String, enabled: bool) -> Result<serde_json::Value> {
161-
let list = self.list().await?;
162-
for group in &list.lists {
163-
for item in &group.indicators {
164-
if item.id == alert_id {
165-
return self
166-
.post(
167-
"/v1/notify/reminders",
168-
serde_json::json!({
169-
"id": item.id,
170-
"indicator_id": item.indicator_id,
171-
"frequency": item.frequency,
172-
"scope": item.scope,
173-
"state": item.state,
174-
"value_map": item.value_map,
175-
"enabled": enabled,
176-
}),
177-
)
178-
.await;
179-
}
180-
}
181-
}
182-
Err(crate::Error::ParseField {
183-
name: "alert_id",
184-
error: format!("alert {} not found", alert_id),
185-
})
167+
async fn set_enabled(&self, item: &AlertItem, enabled: bool) -> Result<serde_json::Value> {
168+
self.post(
169+
"/v1/notify/reminders",
170+
serde_json::json!({
171+
"id": item.id,
172+
"indicator_id": item.indicator_id,
173+
"frequency": item.frequency,
174+
"scope": item.scope,
175+
"state": item.state,
176+
"value_map": item.value_map,
177+
"enabled": enabled,
178+
}),
179+
)
180+
.await
186181
}
187182

188183
/// Delete price alerts.

‎rust/src/blocking/alert.rs‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -40,19 +40,13 @@ impl AlertContextSync {
4040
ctx.add(symbol, condition, trigger_value, frequency).await
4141
})
4242
}
43-
pub fn enable(
44-
&self,
45-
alert_id: impl Into<String> + Send + 'static,
46-
) -> Result<serde_json::Value> {
43+
pub fn enable(&self, item: AlertItem) -> Result<serde_json::Value> {
4744
self.rt
48-
.call(move |ctx| async move { ctx.enable(alert_id).await })
45+
.call(move |ctx| async move { ctx.enable(&item).await })
4946
}
50-
pub fn disable(
51-
&self,
52-
alert_id: impl Into<String> + Send + 'static,
53-
) -> Result<serde_json::Value> {
47+
pub fn disable(&self, item: AlertItem) -> Result<serde_json::Value> {
5448
self.rt
55-
.call(move |ctx| async move { ctx.disable(alert_id).await })
49+
.call(move |ctx| async move { ctx.disable(&item).await })
5650
}
5751
pub fn delete(&self, alert_ids: Vec<String>) -> Result<serde_json::Value> {
5852
self.rt

0 commit comments

Comments
 (0)