From 2608257db93e811f14e2e7e1cab4747bea0548db Mon Sep 17 00:00:00 2001 From: Lionel Guichard Date: Wed, 1 Jul 2026 11:20:23 +0200 Subject: [PATCH] Rule editor now validates conditions and actions before saving --- CHANGELOG.md | 3 + README.md | 1 + Sources/ForelApp/Views/RuleEditorView.swift | 57 ++++++++-- Sources/ForelCore/Models/RuleValidator.swift | 57 ++++++++++ Tests/ForelCoreTests/RuleValidatorTests.swift | 105 ++++++++++++++++++ 5 files changed, 214 insertions(+), 9 deletions(-) create mode 100644 Sources/ForelCore/Models/RuleValidator.swift create mode 100644 Tests/ForelCoreTests/RuleValidatorTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b45540..5a65ae5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ All notable changes to Forel are documented here. Format loosely follows ## [Unreleased] +### Added +- Rule editor now validates conditions and actions before saving — empty condition values, invalid regex, empty destination or rename pattern are reported on save instead of silently causing wrong behaviour. + ### Changed - Removed the Light/Dark theme override from Settings; Forel now always follows the system appearance. - When no rules exist, the action bar is hidden and a centered "New Rule" button is shown in the empty state. diff --git a/README.md b/README.md index 98af43e..65d72f9 100644 --- a/README.md +++ b/README.md @@ -191,6 +191,7 @@ History / Undo (SQLite) - [x] Automatic cleaning database - [x] Uncompress actions - [x] Shortcuts actions +- [ ] Validate actions / conditions before save - [ ] Export / Import rules - [ ] Toggle extension hidden / visible - [ ] Compress actions diff --git a/Sources/ForelApp/Views/RuleEditorView.swift b/Sources/ForelApp/Views/RuleEditorView.swift index 0b0ccca..c79cb65 100644 --- a/Sources/ForelApp/Views/RuleEditorView.swift +++ b/Sources/ForelApp/Views/RuleEditorView.swift @@ -23,6 +23,9 @@ import Photos struct RuleEditorView: View { @State private var rule: Rule + @State private var showConditionError = false + @State private var showActionError = false + @State private var errorDismissTask: Task? @EnvironmentObject private var model: AppModel let onSave: (Rule) -> Void let onCancel: () -> Void @@ -79,6 +82,11 @@ struct RuleEditorView: View { rule.conditions.removeAll { $0.id == condition.id } } } + if showConditionError, let issue = conditionIssues.first { + Text(issue.message) + .font(.system(size: 11)) + .foregroundStyle(.red) + } } .padding(18) } @@ -103,6 +111,11 @@ struct RuleEditorView: View { rule.actions.removeAll { $0.id == action.id } } } + if showActionError, let issue = actionIssues.first { + Text(issue.message) + .font(.system(size: 11)) + .foregroundStyle(.red) + } } .padding(18) } @@ -120,10 +133,27 @@ struct RuleEditorView: View { .foregroundStyle(ForelTheme.primaryText) Spacer() Button("Cancel", action: onCancel).buttonStyle(SecondaryButtonStyle()) - Button("Save") { onSave(rule) } + Button("Save") { + if hasInvalidCondition || hasInvalidAction { + showConditionError = hasInvalidCondition + showActionError = hasInvalidAction + errorDismissTask?.cancel() + errorDismissTask = Task { + try? await Task.sleep(for: .seconds(5)) + guard !Task.isCancelled else { return } + showConditionError = false + showActionError = false + } + } else { + showConditionError = false + showActionError = false + errorDismissTask?.cancel() + onSave(rule) + } + } .buttonStyle(PrimaryButtonStyle()) .keyboardShortcut(.defaultAction) - .disabled(rule.name.trimmingCharacters(in: .whitespaces).isEmpty || hasInvalidRegexCondition) + .disabled(rule.name.trimmingCharacters(in: .whitespaces).isEmpty) } } .padding(22) @@ -132,13 +162,20 @@ struct RuleEditorView: View { .background(WindowActivationBridge(showsDockIcon: model.showDockIcon)) } - /// A rule with an unparsable regex would just silently never match at - /// run time; block saving it instead of letting that ship invisibly. - private var hasInvalidRegexCondition: Bool { - rule.conditions.contains { condition in - guard condition.operator == .matchesRegex, !condition.value.isEmpty else { return false } - return (try? NSRegularExpression(pattern: condition.value)) == nil - } + private var conditionIssues: [RuleValidator.Issue] { + RuleValidator.validate(rule.conditions) + } + + private var hasInvalidCondition: Bool { + !conditionIssues.isEmpty + } + + private var actionIssues: [RuleValidator.Issue] { + RuleValidator.validate(rule.actions) + } + + private var hasInvalidAction: Bool { + !actionIssues.isEmpty } private func placeholder(_ text: String) -> some View { @@ -1287,3 +1324,5 @@ private enum DateValueFormatter { formatter.string(from: date) } } + + diff --git a/Sources/ForelCore/Models/RuleValidator.swift b/Sources/ForelCore/Models/RuleValidator.swift new file mode 100644 index 0000000..52f94dc --- /dev/null +++ b/Sources/ForelCore/Models/RuleValidator.swift @@ -0,0 +1,57 @@ +// Forel - A native macOS file-automation app +// Copyright (C) 2026 Lab421 +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +import Foundation + +/// Validates rule-level constraints so callers can surface issues before +/// persisting a rule that would silently produce wrong results at run time. +public enum RuleValidator { + public struct Issue: Equatable { + public let message: String + public init(message: String) { self.message = message } + } + + public static func validate(_ conditions: [Condition]) -> [Issue] { + conditions.compactMap { condition in + if condition.value.trimmingCharacters(in: .whitespaces).isEmpty { + return Issue(message: "Condition value cannot be empty") + } + if condition.operator == .matchesRegex, + (try? NSRegularExpression(pattern: condition.value)) == nil { + return Issue(message: "Regex pattern is invalid") + } + return nil + } + } + + public static func validate(_ actions: [Action]) -> [Issue] { + actions.compactMap { action in + switch action.kind { + case .moveToFolder, .copyToFolder: + if action.params[ActionParam.destination]?.stringValue?.trimmingCharacters(in: .whitespaces).isEmpty != false { + return Issue(message: "Destination path cannot be empty") + } + case .rename: + if action.params[ActionParam.pattern]?.stringValue?.trimmingCharacters(in: .whitespaces).isEmpty != false { + return Issue(message: "Rename pattern cannot be empty") + } + default: + break + } + return nil + } + } +} diff --git a/Tests/ForelCoreTests/RuleValidatorTests.swift b/Tests/ForelCoreTests/RuleValidatorTests.swift new file mode 100644 index 0000000..89d7359 --- /dev/null +++ b/Tests/ForelCoreTests/RuleValidatorTests.swift @@ -0,0 +1,105 @@ +import Testing +import Foundation +@testable import ForelCore + +@Suite struct RuleValidatorTests { + // MARK: - Conditions + + @Test func validConditionProducesNoIssues() { + let conditions = [makeCondition(.extension_, .is, "pdf")] + #expect(RuleValidator.validate(conditions).isEmpty) + } + + @Test func conditionWithEmptyValueReportsIssue() { + let conditions = [makeCondition(.extension_, .is, "")] + #expect(RuleValidator.validate(conditions) == [.init(message: "Condition value cannot be empty")]) + } + + @Test func conditionWithWhitespaceOnlyValueReportsIssue() { + let conditions = [makeCondition(.name, .contains, " ")] + #expect(RuleValidator.validate(conditions) == [.init(message: "Condition value cannot be empty")]) + } + + @Test func conditionWithInvalidRegexReportsIssue() { + let conditions = [makeCondition(.name, .matchesRegex, "[invalid")] + #expect(RuleValidator.validate(conditions) == [.init(message: "Regex pattern is invalid")]) + } + + @Test func conditionWithValidRegexProducesNoIssues() { + let conditions = [makeCondition(.name, .matchesRegex, "^file\\d+\\.txt$")] + #expect(RuleValidator.validate(conditions).isEmpty) + } + + @Test func multipleInvalidConditionsReportAllIssues() { + let conditions = [ + makeCondition(.extension_, .is, ""), + makeCondition(.name, .matchesRegex, "[invalid"), + ] + let issues = RuleValidator.validate(conditions) + #expect(issues.count == 2) + } + + // MARK: - Actions + + @Test func moveToFolderWithValidDestinationProducesNoIssues() { + let actions = [makeAction(.moveToFolder, .object(["destination": .string("/some/path")]))] + #expect(RuleValidator.validate(actions).isEmpty) + } + + @Test func moveToFolderWithEmptyDestinationReportsIssue() { + let actions = [makeAction(.moveToFolder, .object(["destination": .string("")]))] + #expect(RuleValidator.validate(actions) == [.init(message: "Destination path cannot be empty")]) + } + + @Test func moveToFolderWithMissingDestinationReportsIssue() { + let actions = [makeAction(.moveToFolder, .object([:]))] + #expect(RuleValidator.validate(actions) == [.init(message: "Destination path cannot be empty")]) + } + + @Test func copyToFolderWithValidDestinationProducesNoIssues() { + let actions = [makeAction(.copyToFolder, .object(["destination": .string("/some/path")]))] + #expect(RuleValidator.validate(actions).isEmpty) + } + + @Test func copyToFolderWithEmptyDestinationReportsIssue() { + let actions = [makeAction(.copyToFolder, .object(["destination": .string("")]))] + #expect(RuleValidator.validate(actions) == [.init(message: "Destination path cannot be empty")]) + } + + @Test func renameWithValidPatternProducesNoIssues() { + let actions = [makeAction(.rename, .object(["pattern": .string("{name}")]))] + #expect(RuleValidator.validate(actions).isEmpty) + } + + @Test func renameWithEmptyPatternReportsIssue() { + let actions = [makeAction(.rename, .object(["pattern": .string("")]))] + #expect(RuleValidator.validate(actions) == [.init(message: "Rename pattern cannot be empty")]) + } + + @Test func renameWithMissingPatternReportsIssue() { + let actions = [makeAction(.rename, .object([:]))] + #expect(RuleValidator.validate(actions) == [.init(message: "Rename pattern cannot be empty")]) + } + + @Test func unrelatedActionProducesNoIssues() { + let actions = [makeAction(.moveToTrash, .object([:]))] + #expect(RuleValidator.validate(actions).isEmpty) + } + + @Test func multipleInvalidActionsReportAllIssues() { + let actions = [ + makeAction(.moveToFolder, .object(["destination": .string("")])), + makeAction(.rename, .object(["pattern": .string("")])), + ] + let issues = RuleValidator.validate(actions) + #expect(issues.count == 2) + } + + @Test func mixedValidAndInvalidActionReportsOnlyInvalid() { + let actions = [ + makeAction(.moveToFolder, .object(["destination": .string("/valid")])), + makeAction(.rename, .object(["pattern": .string("")])), + ] + #expect(RuleValidator.validate(actions) == [.init(message: "Rename pattern cannot be empty")]) + } +}