fix(alert): merge enable/disable into update across all language SDKs#524
Open
hogan-yuan wants to merge 2 commits into
Open
fix(alert): merge enable/disable into update across all language SDKs#524hogan-yuan wants to merge 2 commits into
hogan-yuan wants to merge 2 commits into
Conversation
hogan-yuan
force-pushed
the
fix/alert-enable-disable-missing-fields-v2
branch
2 times, most recently
from
4d5dc23 to
9939afc
Compare
hogan-yuan
changed the title
Problem: enable() and disable() sent only {id, enabled} to the API,
causing "invalid frequency:0" and "invalid indicator id" errors.
Solution: change the signature to accept an AlertItem (obtained from
list()) so all required fields are available without an extra round-trip.
Changes:
- rust/src/alert/context.rs: enable/disable take &AlertItem; add
internal set_enabled() helper that builds the full request payload
- rust/src/blocking/alert.rs: blocking wrappers accept AlertItem by value
- python/src/alert/: add From<AlertItem> for lb::AlertItem; binding
passes AlertItem to core
- nodejs/src/alert/: add From<AlertItem> for lb::AlertItem; binding
passes AlertItem to core
- java/src/alert_context.rs: add read_alert_item() that reads each
field from the Java object via JNI; no FromJValue needed
- c/src/alert_context/: lb_alert_context_enable/disable accept
*const CAlertItem; add CAlertItem::to_alert_item() conversion
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
hogan-yuan
force-pushed
the
fix/alert-enable-disable-missing-fields-v2
branch
from
9939afc to
2a2b5cb
Compare
- blocking/alert.rs: keep update() replacing enable/disable - python/alert/types.rs: add manual FromPyObject impl for AlertItem (needed since skip_from_py_object prevents auto-derivation) and adopt skip_from_py_object for AlertSymbolGroup/AlertList from main - c/csrc/include/longbridge.h: accept auto-merged version Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
AlertContext::enable()andAlertContext::disable()were two separate methods that:{id, enabled}— missing required fields (indicator_id,frequency,scope,state,value_map), causinginvalid frequency:0/invalid indicator idAPI errorslist()round-trip to look up the missing fields (with pagination risk)Solution
Replace
enableanddisablewith a singleupdate(item, enabled)method:AlertItemfromlist()directly — no extra round-trip, no pagination riskenabled = true→ enable;enabled = false→ disableChanges
rust/src/alert/context.rsenable+disable→update(&AlertItem, bool)rust/src/blocking/alert.rspython/src/alert/context.rsenable+disable→update(AlertItem, bool)nodejs/src/alert/context.rsenable+disable→update(AlertItem, bool)java/src/alert_context.rsalertContextEnable+alertContextDisable→alertContextUpdate(readsenabledfrom JNI)c/src/alert_context/context.rslb_alert_context_enable+lb_alert_context_disable→lb_alert_context_updatec/csrc/include/longbridge.hVerification
All local CI checks pass:
cargo fmt --check✓cargo clippy(all 5 packages) ✓cargo test(99/99) ✓🤖 Generated with Claude Code