Fix: add missing deeplink recording and device actions - #1645
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| #[test] | ||
| fn parses_pause_recording_legacy_json_deeplink() { | ||
| let url = Url::parse("cap-desktop://action?value=%22pause_recording%22").unwrap(); | ||
| let action = DeepLinkAction::try_from(&url).unwrap(); | ||
|
|
||
| assert!(matches!(action, DeepLinkAction::PauseRecording)); | ||
| } |
There was a problem hiding this comment.
Missing tests for ResumeRecording and TogglePauseRecording
The PR description states that parsing tests are added "to prevent regressions," but only 3 of the 5 new actions have tests. ResumeRecording and TogglePauseRecording follow the same unit-variant URL pattern as PauseRecording (i.e., %22resume_recording%22 / %22toggle_pause_recording%22) and should be trivial to cover. Leaving them untested means a future serde rename or typo in those variant names would go undetected.
Suggested additions:
#[test]
fn parses_resume_recording_legacy_json_deeplink() {
let url = Url::parse("cap-desktop://action?value=%22resume_recording%22").unwrap();
let action = DeepLinkAction::try_from(&url).unwrap();
assert!(matches!(action, DeepLinkAction::ResumeRecording));
}
#[test]
fn parses_toggle_pause_recording_legacy_json_deeplink() {
let url = Url::parse("cap-desktop://action?value=%22toggle_pause_recording%22").unwrap();
let action = DeepLinkAction::try_from(&url).unwrap();
assert!(matches!(action, DeepLinkAction::TogglePauseRecording));
}Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src-tauri/src/deeplink_actions.rs
Line: 187-193
Comment:
**Missing tests for `ResumeRecording` and `TogglePauseRecording`**
The PR description states that parsing tests are added "to prevent regressions," but only 3 of the 5 new actions have tests. `ResumeRecording` and `TogglePauseRecording` follow the same unit-variant URL pattern as `PauseRecording` (i.e., `%22resume_recording%22` / `%22toggle_pause_recording%22`) and should be trivial to cover. Leaving them untested means a future serde rename or typo in those variant names would go undetected.
Suggested additions:
```rust
#[test]
fn parses_resume_recording_legacy_json_deeplink() {
let url = Url::parse("cap-desktop://action?value=%22resume_recording%22").unwrap();
let action = DeepLinkAction::try_from(&url).unwrap();
assert!(matches!(action, DeepLinkAction::ResumeRecording));
}
#[test]
fn parses_toggle_pause_recording_legacy_json_deeplink() {
let url = Url::parse("cap-desktop://action?value=%22toggle_pause_recording%22").unwrap();
let action = DeepLinkAction::try_from(&url).unwrap();
assert!(matches!(action, DeepLinkAction::TogglePauseRecording));
}
```
How can I resolve this? If you propose a fix, please make it concise.| DeepLinkAction::SetCamera { camera } => { | ||
| crate::set_camera_input(app.clone(), app.state(), camera, Some(true)).await | ||
| } |
There was a problem hiding this comment.
Camera preview suppressed silently for deeplink camera switches
Some(true) is passed for skip_camera_window, which means the camera preview popup is unconditionally skipped when switching cameras via deeplink. Compare with StartRecording, which passes None (causing the preview window to be shown when a camera is selected). Depending on intent, a user who triggers set_camera via a deeplink would have no visible feedback that the camera actually switched. If this is intentional (e.g., to keep the action "headless"), a brief comment here would prevent future maintainers from wondering why Some(true) and not None or Some(false) is used.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src-tauri/src/deeplink_actions.rs
Line: 170-172
Comment:
**Camera preview suppressed silently for deeplink camera switches**
`Some(true)` is passed for `skip_camera_window`, which means the camera preview popup is unconditionally skipped when switching cameras via deeplink. Compare with `StartRecording`, which passes `None` (causing the preview window to be shown when a camera is selected). Depending on intent, a user who triggers `set_camera` via a deeplink would have no visible feedback that the camera actually switched. If this is intentional (e.g., to keep the action "headless"), a brief comment here would prevent future maintainers from wondering why `Some(true)` and not `None` or `Some(false)` is used.
How can I resolve this? If you propose a fix, please make it concise.
|
We are closing the deeplinks and Raycast bounty (#1540) without awarding it, and closing the pull requests opened against it. The full reasoning is on that issue: the remaining actions it asked for are deliberately limited to debug builds because deeplinks are a remote control surface any web page can invoke, and widening that in production is a security decision we do not want to take through a bounty. This is not a reflection on your work. Leaving it unreviewed for this long is our failure, and we are sorry for the wasted effort. If you want to contribute again, the open bug reports are the most useful place and we will review promptly. |
2 participants
Summary
pause_recording,resume_recording,toggle_pause_recordingset_microphoneandset_camerarecording.rsandlib.rsTesting
cargo test -p cap-desktop deeplink_actions -- --nocapturein this environment becausecargois not installed in PATH./claim #1540
Greptile Summary
This PR extends
deeplink_actions.rswith five new deeplink actions (PauseRecording,ResumeRecording,TogglePauseRecording,SetMicrophone, andSetCamera) and wires them to the existing Tauri command handlers inrecording.rsandlib.rs. The changes are additive and follow established patterns already used byStartRecordingandStopRecording.Key changes:
pause_recording,resume_recording,toggle_pause_recording) delegate to the correspondingcrate::recording::*functions.SetMicrophone,SetCamera) delegate to the private-but-descendant-accessiblecrate::set_mic_inputandcrate::set_camera_inputTauri commands.ResumeRecordingandTogglePauseRecordingdo not have equivalent tests yet.SetCameraunconditionally passesskip_camera_window: Some(true), suppressing the camera preview popup on camera switch — behaviour that differs fromStartRecordingand is currently undocumented.Confidence Score: 4/5
ResumeRecordingandTogglePauseRecording, and the silently-suppressed camera preview window forSetCameradeeplinks that could confuse future maintainers.skip_camera_window: Some(true)decision.Important Files Changed
PauseRecording,ResumeRecording,TogglePauseRecording,SetMicrophone, andSetCameradeeplink actions and wires them to existing handlers; parsing tests cover only 3 of 5 new actions (missingResumeRecordingandTogglePauseRecording), and the decision to suppress the camera preview window (skip_camera_window: Some(true)) for theSetCameradeeplink is undocumented.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD URL["cap-desktop://action?value=..."] --> Parse["DeepLinkAction::try_from(url)"] Parse --> Deserialise["serde_json::from_str(value)"] Deserialise -->|pause_recording| PR["PauseRecording"] Deserialise -->|resume_recording| RR["ResumeRecording"] Deserialise -->|toggle_pause_recording| TPR["TogglePauseRecording"] Deserialise -->|set_microphone| SM["SetMicrophone { mic_label }"] Deserialise -->|set_camera| SC["SetCamera { camera }"] PR --> pause["crate::recording::pause_recording()"] RR --> resume["crate::recording::resume_recording()"] TPR --> toggle["crate::recording::toggle_pause_recording()"] SM --> setMic["crate::set_mic_input(state, mic_label)"] SC --> setCam["crate::set_camera_input(app, state, camera, Some(true))"] setCam -->|skip_camera_window=true| noPop["Camera preview window NOT shown"] setCam -->|camera=None| disableCam["Camera input removed"]Last reviewed commit: bf6f9aa
(2/5) Greptile learns from your feedback when you react with thumbs up/down!