Feat/add run automation once on finish - #1410
Conversation
Signed-off-by: Lucian Maly <lucian@redhat.com>
Signed-off-by: Lucian Maly <lucian@redhat.com>
…nish" UI Signed-off-by: Lucian Maly <lucian@redhat.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
WalkthroughThis pull request adds a "run once on finish" feature to automations, allowing rules to execute immediately when torrents reach 100% completion. IntervalSeconds value of 0 now indicates this behavior across the API, backend services, and frontend UI, with corresponding validation and processing logic updates. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as WorkflowDialog
participant API as API Handler
participant Service as Automation Service
participant Processor as Rule Processor
participant DB as Torrent Data
User->>UI: Check "Run once on finish"
UI->>UI: Set intervalSeconds = 0
UI->>API: POST automation config
API->>API: Validate intervalSeconds (0 allowed)
API->>Service: Store automation rule
Service->>Service: Mark as run-once (interval=0)
Note over Processor: On torrent progress update
DB->>Processor: Check matching rules
Processor->>Processor: Filter by intervalSeconds
alt IntervalSeconds == 0
Processor->>DB: Check torrent progress
alt progress >= 100%
Processor->>Processor: Match rule
Processor->>Service: Execute action once
Service->>Service: Do NOT update cadence
else progress < 100%
Processor->>Processor: Skip rule
end
else IntervalSeconds > 0
Processor->>Service: Check interval throttling
Processor->>Processor: Match if eligible
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/services/automations/processor.go (1)
94-114:⚠️ Potential issue | 🟡 MinorPreallocate
matchingto satisfy lint and reduce allocations.
Line 98 currently triggers theprealloclint failure; preallocating fixes the pipeline warning.💡 Suggested fix
- var matching []*models.Automation + matching := make([]*models.Automation, 0, len(rules))
🤖 Fix all issues with AI agents
In `@web/src/lib/workflow-utils.ts`:
- Around line 177-179: The code currently copies obj.intervalSeconds into
data.intervalSeconds only for valid values and silently ignores invalid numbers
(e.g., 1–59); update the validation so that when obj.intervalSeconds is present
but not (=== 0 || >= 60) you reject it (throw or return a validation error)
instead of skipping it. Locate the block that reads obj.intervalSeconds and
change it to: if typeof obj.intervalSeconds === "number" and
(obj.intervalSeconds === 0 || obj.intervalSeconds >= 60) set
data.intervalSeconds = obj.intervalSeconds; else if obj.intervalSeconds is
defined but invalid, raise a clear validation error referencing
obj.intervalSeconds so callers know the value is out of allowed range.
| // Optional intervalSeconds: 0 = run once on finish, otherwise must be >= 60 | ||
| if (typeof obj.intervalSeconds === "number" && (obj.intervalSeconds === 0 || obj.intervalSeconds >= 60)) { | ||
| data.intervalSeconds = obj.intervalSeconds |
There was a problem hiding this comment.
Reject invalid intervalSeconds instead of silently ignoring.
Line 177 only includes valid values but doesn’t error on invalid ones (e.g., 1–59). Since the comment says “must be >= 60,” returning an error keeps import validation consistent with backend rules.
💡 Suggested fix
- if (typeof obj.intervalSeconds === "number" && (obj.intervalSeconds === 0 || obj.intervalSeconds >= 60)) {
- data.intervalSeconds = obj.intervalSeconds
- }
+ if (obj.intervalSeconds !== undefined) {
+ if (typeof obj.intervalSeconds !== "number" || Number.isNaN(obj.intervalSeconds) || (obj.intervalSeconds !== 0 && obj.intervalSeconds < 60)) {
+ return { data: null, error: "intervalSeconds must be 0 (run once on finish) or at least 60" }
+ }
+ data.intervalSeconds = obj.intervalSeconds
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Optional intervalSeconds: 0 = run once on finish, otherwise must be >= 60 | |
| if (typeof obj.intervalSeconds === "number" && (obj.intervalSeconds === 0 || obj.intervalSeconds >= 60)) { | |
| data.intervalSeconds = obj.intervalSeconds | |
| // Optional intervalSeconds: 0 = run once on finish, otherwise must be >= 60 | |
| if (obj.intervalSeconds !== undefined) { | |
| if (typeof obj.intervalSeconds !== "number" || Number.isNaN(obj.intervalSeconds) || (obj.intervalSeconds !== 0 && obj.intervalSeconds < 60)) { | |
| return { data: null, error: "intervalSeconds must be 0 (run once on finish) or at least 60" } | |
| } | |
| data.intervalSeconds = obj.intervalSeconds | |
| } |
🤖 Prompt for AI Agents
In `@web/src/lib/workflow-utils.ts` around lines 177 - 179, The code currently
copies obj.intervalSeconds into data.intervalSeconds only for valid values and
silently ignores invalid numbers (e.g., 1–59); update the validation so that
when obj.intervalSeconds is present but not (=== 0 || >= 60) you reject it
(throw or return a validation error) instead of skipping it. Locate the block
that reads obj.intervalSeconds and change it to: if typeof obj.intervalSeconds
=== "number" and (obj.intervalSeconds === 0 || obj.intervalSeconds >= 60) set
data.intervalSeconds = obj.intervalSeconds; else if obj.intervalSeconds is
defined but invalid, raise a clear validation error referencing
obj.intervalSeconds so callers know the value is out of allowed range.
|
@luckylittle Is the end goal of this running the workflow on the torrent that triggered it or does the workflow running on all torrents when any torrent finishes fit your usecase? Edit: |
|
@jabloink
The goal is identical to what qBt does - run a script for each torrent that completed, but as part of the workflow so you can trigger different script for different conditions: 
[screenshot from qBt] I think this will require checking how qBittorrent reports completion and whether a completion callback exists and implement that in https://github.com/autobrr/go-qbittorrent ?
Moved to draft for now. |
|
Related: autobrr/go-qbittorrent#104 |
|
Polling for this is not really ideal. A better solution to this would be make the qbit instances set a script in on complete that sends a webhook to Qui when a torrent is completed. There was another PR about this or maybe it was a discussion on Discord. I'll try to find it. Edit: found it: This PR: #338 |
4 participants
What is the relevant ticket?
What’s this PR do?
Added support for “run once on finish” rules using
intervalSeconds = 0.Validation now allows
0(run once when the torrent finishes) or>= 60; anything in between is rejected.Rules with
intervalSeconds = 0:lastRuleRun.UI update:
—.Import/export fixes:
intervalSeconds = 0is now correctly exported and imported.0as a valid value.Add
Where should the reviewer start?
internal/api/handlers/automations.gointernal/models/automation.gointernal/services/automations/processor.gointernal/services/automations/service.goweb/src/components/instances/preferences/WorkflowDialog.tsxweb/src/lib/workflow-utils.tsHow should this be manually tested?
./qui serveRisk involved?
Screenshots (if appropriate):
Does the documentation or dependencies need an update?
Summary by CodeRabbit
New Features
Bug Fixes