From 03df2d75ac9eb073f00065a03c271507899010ae Mon Sep 17 00:00:00 2001 From: Dino Constantinou Date: Wed, 7 Oct 2026 08:19:49 +0100 Subject: [PATCH 1/3] Add Expense Attachments Adds attachment to the expense create and update payloads, with a new ExpenseAttachmentUpdatePayload carrying _destroy, and an expense attachment add/remove command group. Both commands read the expense first: add refuses one that already has an attachment, and remove refuses one without. File reading moves to a shared AttachmentFile. Co-Authored-By: Claude Opus 5.5 --- .../ExpenseAttachmentAddCommand.swift | 84 ++++++++++++++++++ .../ExpenseAttachmentRemoveCommand.swift | 69 +++++++++++++++ .../Expense/ExpenseAttachmentCommand.swift | 12 +++ .../Expense/ExpenseUpdateCommand.swift | 5 +- .../Commands/ExpenseCommand.swift | 1 + .../ExplanationAttachmentAddCommand.swift | 56 ++---------- .../FreeAgentCLI/Models/AttachmentFile.swift | 64 ++++++++++++++ .../ExpenseIntegrationTests.swift | 50 +++++++++++ .../ExpenseAttachmentAddCommandTests.swift | 86 +++++++++++++++++++ .../ExpenseAttachmentRemoveCommandTests.swift | 65 ++++++++++++++ ...ExplanationAttachmentAddCommandTests.swift | 15 ---- .../Models/AttachmentFileTests.swift | 52 +++++++++++ .../ExpenseAttachmentUpdatePayload.yaml | 20 +++++ .../schemas/ExpenseCreatePayload.yaml | 3 + .../schemas/ExpenseUpdatePayload.yaml | 3 + openapi/openapi.yaml | 2 + 16 files changed, 520 insertions(+), 67 deletions(-) create mode 100644 Sources/FreeAgentCLI/Commands/Expense/Attachment/ExpenseAttachmentAddCommand.swift create mode 100644 Sources/FreeAgentCLI/Commands/Expense/Attachment/ExpenseAttachmentRemoveCommand.swift create mode 100644 Sources/FreeAgentCLI/Commands/Expense/ExpenseAttachmentCommand.swift create mode 100644 Sources/FreeAgentCLI/Models/AttachmentFile.swift create mode 100644 Tests/FreeAgentCLITests/Commands/Expense/Attachment/ExpenseAttachmentAddCommandTests.swift create mode 100644 Tests/FreeAgentCLITests/Commands/Expense/Attachment/ExpenseAttachmentRemoveCommandTests.swift delete mode 100644 Tests/FreeAgentCLITests/Commands/Explanation/Attachment/ExplanationAttachmentAddCommandTests.swift create mode 100644 Tests/FreeAgentCLITests/Models/AttachmentFileTests.swift create mode 100644 openapi/components/schemas/ExpenseAttachmentUpdatePayload.yaml diff --git a/Sources/FreeAgentCLI/Commands/Expense/Attachment/ExpenseAttachmentAddCommand.swift b/Sources/FreeAgentCLI/Commands/Expense/Attachment/ExpenseAttachmentAddCommand.swift new file mode 100644 index 0000000..ef91fd4 --- /dev/null +++ b/Sources/FreeAgentCLI/Commands/Expense/Attachment/ExpenseAttachmentAddCommand.swift @@ -0,0 +1,84 @@ +import ArgumentParser +import Foundation +import FreeAgentAPI +import Noora + +// MARK: - ExpenseAttachmentAddCommand + +struct ExpenseAttachmentAddCommand: MutatingCommand { + static let configuration = CommandConfiguration( + commandName: "add", + abstract: "Add an attachment to an expense", + discussion: "An expense holds one attachment of up to 5MB. Remove the one it has before adding another." + ) + + @Argument(help: "Expense ID or URL") + var id: ResourceID + + @Option(name: .long, help: "Path to the file to attach (pdf, png, jpg, jpeg or gif)") + var file: String + + @Option(name: .long, help: "Description of the attached file") + var description: String? + + @Flag(help: "Print the request instead of sending it") + var dryRun = false + + @Flag(name: .long, help: "Output JSON") + var json = false + + func perform(client: Client) async throws -> Components.Schemas.ExpenseResponse { + let attachment = try AttachmentFile(path: file) + + let expense = try await client.getASingleExpense(.init(path: .init(id: id.value))).ok.body.json.expense + + if let existing = expense.attachment { + throw ExpenseAttachmentAddCommandError.alreadyAttached(id: id, fileName: existing.fileName) + } + + let input = Operations.UpdateExpense.Input( + path: .init(id: id.value), + body: .json(.init(expense: .init(attachment: .init( + data: attachment.data, + fileName: attachment.fileName, + contentType: attachment.contentType, + description: description + )))) + ) + + return try await client.updateExpense(input) + .ok.body.json + } + + func success(for _: Components.Schemas.ExpenseResponse) -> String { + "Added the attachment to expense \(id)" + } +} + +// MARK: - ExpenseAttachmentAddCommandError + +enum ExpenseAttachmentAddCommandError: CommandError { + case alreadyAttached(id: ResourceID, fileName: String?) + + // MARK: Internal + + var errorDescription: String? { + switch self { + case .alreadyAttached(let id, let fileName?): + "Expense \(id) already has an attachment, \(fileName)" + case .alreadyAttached(let id, nil): + "Expense \(id) already has an attachment" + } + } + + var exitCode: ExitCode { + APIError.Kind.rejected.exitCode + } + + var takeaways: [TerminalText] { + switch self { + case .alreadyAttached(let id, _): + ["Run \(.command("freeagent expense attachment remove \(id)")) first to replace it"] + } + } +} diff --git a/Sources/FreeAgentCLI/Commands/Expense/Attachment/ExpenseAttachmentRemoveCommand.swift b/Sources/FreeAgentCLI/Commands/Expense/Attachment/ExpenseAttachmentRemoveCommand.swift new file mode 100644 index 0000000..1a20d72 --- /dev/null +++ b/Sources/FreeAgentCLI/Commands/Expense/Attachment/ExpenseAttachmentRemoveCommand.swift @@ -0,0 +1,69 @@ +import ArgumentParser +import Foundation +import FreeAgentAPI +import Noora + +// MARK: - ExpenseAttachmentRemoveCommand + +struct ExpenseAttachmentRemoveCommand: DestructiveCommand { + static let configuration = CommandConfiguration( + commandName: "remove", + abstract: "Remove the attachment from an expense", + discussion: "FreeAgent deletes the attached file." + ) + + @Argument(help: "Expense ID or URL") + var id: ResourceID + + @Flag(help: "Print the request instead of sending it") + var dryRun = false + + @Flag(help: "Skip the confirmation prompt") + var yes = false + + @Flag(name: .long, help: "Output JSON") + var json = false + + var confirmation: String { + "Remove the attachment from expense \(id)?" + } + + func perform(client: Client) async throws -> Components.Schemas.ExpenseResponse { + let expense = try await client.getASingleExpense(.init(path: .init(id: id.value))).ok.body.json.expense + + guard expense.attachment != nil else { + throw ExpenseAttachmentRemoveCommandError.noAttachment(id: id) + } + + let input = Operations.UpdateExpense.Input( + path: .init(id: id.value), + body: .json(.init(expense: .init(attachment: .init(_destroy: 1)))) + ) + + return try await client.updateExpense(input) + .ok.body.json + } + + func success(for _: Components.Schemas.ExpenseResponse) -> String { + "Removed the attachment from expense \(id)" + } +} + +// MARK: - ExpenseAttachmentRemoveCommandError + +enum ExpenseAttachmentRemoveCommandError: CommandError { + case noAttachment(id: ResourceID) + + // MARK: Internal + + var errorDescription: String? { + switch self { + case .noAttachment(let id): + "Expense \(id) has no attachment" + } + } + + var exitCode: ExitCode { + APIError.Kind.notFound.exitCode + } +} diff --git a/Sources/FreeAgentCLI/Commands/Expense/ExpenseAttachmentCommand.swift b/Sources/FreeAgentCLI/Commands/Expense/ExpenseAttachmentCommand.swift new file mode 100644 index 0000000..ee05ad4 --- /dev/null +++ b/Sources/FreeAgentCLI/Commands/Expense/ExpenseAttachmentCommand.swift @@ -0,0 +1,12 @@ +import ArgumentParser + +struct ExpenseAttachmentCommand: AsyncParsableCommand { + static let configuration = CommandConfiguration( + commandName: "attachment", + abstract: "Manage the attachment on an expense", + subcommands: [ + ExpenseAttachmentAddCommand.self, + ExpenseAttachmentRemoveCommand.self, + ] + ) +} diff --git a/Sources/FreeAgentCLI/Commands/Expense/ExpenseUpdateCommand.swift b/Sources/FreeAgentCLI/Commands/Expense/ExpenseUpdateCommand.swift index efcb05f..a0853b7 100644 --- a/Sources/FreeAgentCLI/Commands/Expense/ExpenseUpdateCommand.swift +++ b/Sources/FreeAgentCLI/Commands/Expense/ExpenseUpdateCommand.swift @@ -6,7 +6,10 @@ struct ExpenseUpdateCommand: MutatingCommand { static let configuration = CommandConfiguration( commandName: "update", abstract: "Update an expense", - discussion: "Fields left out are kept, and an empty --receipt-reference removes it. A mileage claim's engine can't be changed." + discussion: """ + Fields left out are kept, and an empty --receipt-reference removes it. A mileage claim's engine can't be \ + changed. The attachment is managed with 'freeagent expense attachment'. + """ ) @Argument(help: "Expense ID or URL") diff --git a/Sources/FreeAgentCLI/Commands/ExpenseCommand.swift b/Sources/FreeAgentCLI/Commands/ExpenseCommand.swift index 2a8ea02..9ff716b 100644 --- a/Sources/FreeAgentCLI/Commands/ExpenseCommand.swift +++ b/Sources/FreeAgentCLI/Commands/ExpenseCommand.swift @@ -10,6 +10,7 @@ struct ExpenseCommand: AsyncParsableCommand { ExpenseShowCommand.self, ExpenseUpdateCommand.self, ExpenseDeleteCommand.self, + ExpenseAttachmentCommand.self, ExpenseMileageSettingsCommand.self, ] ) diff --git a/Sources/FreeAgentCLI/Commands/Explanation/Attachment/ExplanationAttachmentAddCommand.swift b/Sources/FreeAgentCLI/Commands/Explanation/Attachment/ExplanationAttachmentAddCommand.swift index 48b70c0..609f068 100644 --- a/Sources/FreeAgentCLI/Commands/Explanation/Attachment/ExplanationAttachmentAddCommand.swift +++ b/Sources/FreeAgentCLI/Commands/Explanation/Attachment/ExplanationAttachmentAddCommand.swift @@ -1,14 +1,8 @@ import ArgumentParser import Foundation import FreeAgentAPI -import Noora - -// MARK: - ExplanationAttachmentAddCommand struct ExplanationAttachmentAddCommand: MutatingCommand { - - // MARK: Internal - static let configuration = CommandConfiguration( commandName: "add", abstract: "Add attachments to a bank transaction explanation", @@ -32,17 +26,12 @@ struct ExplanationAttachmentAddCommand: MutatingCommand { func perform(client: Client) async throws -> Components.Schemas.AttachmentListResponse { let attachments = try file.map { path in - let url = URL(fileURLWithPath: path) - let ext = url.pathExtension.lowercased() + let attachment = try AttachmentFile(path: path) - guard let contentType = Self.contentTypes[ext] else { - throw ExplanationAttachmentAddCommandError.unsupportedFileType(ext) - } - - return try Components.Schemas.AttachmentCreatePayload( - data: Data(contentsOf: url).base64EncodedString(), - fileName: url.lastPathComponent, - contentType: contentType, + return Components.Schemas.AttachmentCreatePayload( + data: attachment.data, + fileName: attachment.fileName, + contentType: attachment.contentType, description: description ) } @@ -61,39 +50,4 @@ struct ExplanationAttachmentAddCommand: MutatingCommand { ? "Added the attachment to explanation \(id)" : "Added \(file.count) attachments to explanation \(id)" } - - // MARK: Private - - private static let contentTypes = [ - "pdf": "application/pdf", - "png": "image/png", - "jpg": "image/jpeg", - "jpeg": "image/jpeg", - "gif": "image/gif", - ] - -} - -// MARK: - ExplanationAttachmentAddCommandError - -enum ExplanationAttachmentAddCommandError: CommandError { - case unsupportedFileType(String) - - var errorDescription: String? { - switch self { - case .unsupportedFileType(let ext): - "Unsupported attachment type '\(ext)'" - } - } - - var exitCode: ExitCode { - .validationFailure - } - - var takeaways: [TerminalText] { - switch self { - case .unsupportedFileType: - ["Attach a pdf, png, jpg, jpeg or gif file"] - } - } } diff --git a/Sources/FreeAgentCLI/Models/AttachmentFile.swift b/Sources/FreeAgentCLI/Models/AttachmentFile.swift new file mode 100644 index 0000000..c5393bd --- /dev/null +++ b/Sources/FreeAgentCLI/Models/AttachmentFile.swift @@ -0,0 +1,64 @@ +import ArgumentParser +import Foundation +import Noora + +// MARK: - AttachmentFile + +struct AttachmentFile { + + // MARK: Lifecycle + + init(path: String) throws { + let url = URL(fileURLWithPath: path) + let ext = url.pathExtension.lowercased() + + guard let contentType = Self.contentTypes[ext] else { + throw AttachmentFileError.unsupportedFileType(ext) + } + + data = try Data(contentsOf: url).base64EncodedString() + fileName = url.lastPathComponent + self.contentType = contentType + } + + // MARK: Internal + + let data: String + let fileName: String + let contentType: String + + // MARK: Private + + private static let contentTypes = [ + "pdf": "application/pdf", + "png": "image/png", + "jpg": "image/jpeg", + "jpeg": "image/jpeg", + "gif": "image/gif", + ] + +} + +// MARK: - AttachmentFileError + +enum AttachmentFileError: CommandError { + case unsupportedFileType(String) + + var errorDescription: String? { + switch self { + case .unsupportedFileType(let ext): + "Unsupported attachment type '\(ext)'" + } + } + + var exitCode: ExitCode { + .validationFailure + } + + var takeaways: [TerminalText] { + switch self { + case .unsupportedFileType: + ["Attach a pdf, png, jpg, jpeg or gif file"] + } + } +} diff --git a/Tests/FreeAgentAPIIntegrationTests/ExpenseIntegrationTests.swift b/Tests/FreeAgentAPIIntegrationTests/ExpenseIntegrationTests.swift index 596dfb0..d6573be 100644 --- a/Tests/FreeAgentAPIIntegrationTests/ExpenseIntegrationTests.swift +++ b/Tests/FreeAgentAPIIntegrationTests/ExpenseIntegrationTests.swift @@ -136,6 +136,53 @@ struct ExpenseIntegrationTests { _ = try await client.deleteExpense(.init(path: .init(id: id))).ok } + @Test("An expense attachment is sent on create, removed with _destroy and added on update") + func attachment() async throws { + let user = try await client.showCurrentUser(.init()).ok.body.json.user + + let created = try await client.createExpense( + .init(body: .json(.init(expense: .init( + user: Self.id(of: user.url), + category: "285", + datedOn: Self.today, + description: "ZZ Integration Test Attachment", + grossValue: "-12.0", + attachment: .init( + data: Self.onePixelPNG, + fileName: "receipt.png", + contentType: .imagePng, + description: "Integration test receipt" + ) + )))) + ).created.body.json.expense + + let attached = try #require(created.attachment) + #expect(attached.url.contains("/v2/attachments/")) + #expect(attached.fileName == "receipt.png") + #expect(attached.contentType == "image/png") + #expect(attached.description == "Integration test receipt") + + let id = Self.id(of: created.url) + + let removed = try await client.updateExpense( + .init(path: .init(id: id), body: .json(.init(expense: .init(attachment: .init(_destroy: 1))))) + ).ok.body.json.expense + + #expect(removed.attachment == nil) + + let added = try await client.updateExpense( + .init(path: .init(id: id), body: .json(.init(expense: .init(attachment: .init( + data: Self.onePixelPNG, + fileName: "replacement.png", + contentType: "image/png" + ))))) + ).ok.body.json.expense + + #expect(added.attachment?.fileName == "replacement.png") + + _ = try await client.deleteExpense(.init(path: .init(id: id))).ok + } + @Test("GET /v2/expenses/mileage_settings returns typed engine options and rates") func mileageSettings() async throws { let settings = try await client.showMileageSettings(.init()).ok.body.json.mileageSettings @@ -200,6 +247,9 @@ struct ExpenseIntegrationTests { // MARK: Private + private static let onePixelPNG = + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==" + private static var today: String { date(monthsFromNow: 0) } diff --git a/Tests/FreeAgentCLITests/Commands/Expense/Attachment/ExpenseAttachmentAddCommandTests.swift b/Tests/FreeAgentCLITests/Commands/Expense/Attachment/ExpenseAttachmentAddCommandTests.swift new file mode 100644 index 0000000..bce50b1 --- /dev/null +++ b/Tests/FreeAgentCLITests/Commands/Expense/Attachment/ExpenseAttachmentAddCommandTests.swift @@ -0,0 +1,86 @@ +import ArgumentParser +import Foundation +import FreeAgentAPI +import HTTPTypes +import Mockable +import OpenAPIRuntime +import Testing + +@testable import FreeAgentCLI + +struct ExpenseAttachmentAddCommandTests { + + // MARK: Internal + + @Test("adds the attachment to an expense without one") + func adds() async throws { + let transport = transport(showing: #"{"expense":{"url":"https://api.freeagent.com/v2/expenses/577140"}}"#) + let command = try ExpenseAttachmentAddCommand.parse(["577140", "--file", Self.receipt()]) + + let response = try await command.perform(client: client(transport)) + + #expect(command.success(for: response) == "Added the attachment to expense 577140") + verify(transport).send(.any, body: .any, baseURL: .any, operationID: .value("updateExpense")).called(1) + } + + @Test("refuses an expense that already has an attachment, naming its file") + func alreadyAttached() async throws { + let transport = transport( + showing: #"{"expense":{"url":"https://api.freeagent.com/v2/expenses/577140","attachment":{"url":"https://api.freeagent.com/v2/attachments/183146","file_name":"receipt.png"}}}"# + ) + let command = try ExpenseAttachmentAddCommand.parse(["577140", "--file", Self.receipt()]) + + let error = await #expect(throws: ExpenseAttachmentAddCommandError.self) { + try await command.perform(client: client(transport)) + } + + #expect(error?.errorDescription == "Expense 577140 already has an attachment, receipt.png") + #expect(error?.exitCode == ExitCode(4)) + #expect(error?.takeaways.map { $0.plain() } == [ + "Run 'freeagent expense attachment remove 577140' first to replace it" + ]) + verify(transport).send(.any, body: .any, baseURL: .any, operationID: .value("updateExpense")).called(0) + } + + @Test("rejects an unsupported file before reading the expense") + func unsupportedFile() async throws { + let transport = transport(showing: #"{"expense":{"url":"https://api.freeagent.com/v2/expenses/577140"}}"#) + let command = try ExpenseAttachmentAddCommand.parse(["577140", "--file", "notes.txt"]) + + await #expect(throws: AttachmentFileError.self) { + try await command.perform(client: client(transport)) + } + + verify(transport).send(.any, body: .any, baseURL: .any, operationID: .value("getASingleExpense")).called(0) + } + + // MARK: Private + + private static func receipt() throws -> String { + let path = FileManager.default.temporaryDirectory.appendingPathComponent("\(UUID().uuidString).pdf").path + try Data("%PDF".utf8).write(to: URL(fileURLWithPath: path)) + return path + } + + private func transport(showing body: String) -> MockClientTransportInterface { + let transport = MockClientTransportInterface() + given(transport) + .send(.any, body: .any, baseURL: .any, operationID: .value("getASingleExpense")) + .willReturn(( + HTTPResponse(status: .ok, headerFields: [.contentType: "application/json"]), + HTTPBody(Data(body.utf8)) + )) + given(transport) + .send(.any, body: .any, baseURL: .any, operationID: .value("updateExpense")) + .willReturn(( + HTTPResponse(status: .ok, headerFields: [.contentType: "application/json"]), + HTTPBody(Data(#"{"expense":{"url":"https://api.freeagent.com/v2/expenses/577140"}}"#.utf8)) + )) + + return transport + } + + private func client(_ transport: MockClientTransportInterface) -> Client { + Client(serverURL: Environment.sandbox.baseURL, transport: transport, middlewares: [.apiError()]) + } +} diff --git a/Tests/FreeAgentCLITests/Commands/Expense/Attachment/ExpenseAttachmentRemoveCommandTests.swift b/Tests/FreeAgentCLITests/Commands/Expense/Attachment/ExpenseAttachmentRemoveCommandTests.swift new file mode 100644 index 0000000..dca299a --- /dev/null +++ b/Tests/FreeAgentCLITests/Commands/Expense/Attachment/ExpenseAttachmentRemoveCommandTests.swift @@ -0,0 +1,65 @@ +import ArgumentParser +import Foundation +import FreeAgentAPI +import HTTPTypes +import Mockable +import OpenAPIRuntime +import Testing + +@testable import FreeAgentCLI + +struct ExpenseAttachmentRemoveCommandTests { + + // MARK: Internal + + @Test("removes the attachment from an expense with one") + func removes() async throws { + let transport = transport( + showing: #"{"expense":{"url":"https://api.freeagent.com/v2/expenses/577140","attachment":{"url":"https://api.freeagent.com/v2/attachments/183146"}}}"# + ) + let command = try ExpenseAttachmentRemoveCommand.parse(["577140", "--yes"]) + + let response = try await command.perform(client: client(transport)) + + #expect(command.success(for: response) == "Removed the attachment from expense 577140") + verify(transport).send(.any, body: .any, baseURL: .any, operationID: .value("updateExpense")).called(1) + } + + @Test("refuses an expense with no attachment as not found") + func noAttachment() async throws { + let transport = transport(showing: #"{"expense":{"url":"https://api.freeagent.com/v2/expenses/577140"}}"#) + let command = try ExpenseAttachmentRemoveCommand.parse(["577140", "--yes"]) + + let error = await #expect(throws: ExpenseAttachmentRemoveCommandError.self) { + try await command.perform(client: client(transport)) + } + + #expect(error?.errorDescription == "Expense 577140 has no attachment") + #expect(error?.exitCode == ExitCode(3)) + verify(transport).send(.any, body: .any, baseURL: .any, operationID: .value("updateExpense")).called(0) + } + + // MARK: Private + + private func transport(showing body: String) -> MockClientTransportInterface { + let transport = MockClientTransportInterface() + given(transport) + .send(.any, body: .any, baseURL: .any, operationID: .value("getASingleExpense")) + .willReturn(( + HTTPResponse(status: .ok, headerFields: [.contentType: "application/json"]), + HTTPBody(Data(body.utf8)) + )) + given(transport) + .send(.any, body: .any, baseURL: .any, operationID: .value("updateExpense")) + .willReturn(( + HTTPResponse(status: .ok, headerFields: [.contentType: "application/json"]), + HTTPBody(Data(#"{"expense":{"url":"https://api.freeagent.com/v2/expenses/577140"}}"#.utf8)) + )) + + return transport + } + + private func client(_ transport: MockClientTransportInterface) -> Client { + Client(serverURL: Environment.sandbox.baseURL, transport: transport, middlewares: [.apiError()]) + } +} diff --git a/Tests/FreeAgentCLITests/Commands/Explanation/Attachment/ExplanationAttachmentAddCommandTests.swift b/Tests/FreeAgentCLITests/Commands/Explanation/Attachment/ExplanationAttachmentAddCommandTests.swift deleted file mode 100644 index 491f64b..0000000 --- a/Tests/FreeAgentCLITests/Commands/Explanation/Attachment/ExplanationAttachmentAddCommandTests.swift +++ /dev/null @@ -1,15 +0,0 @@ -import ArgumentParser -import Testing - -@testable import FreeAgentCLI - -struct ExplanationAttachmentAddCommandTests { - @Test("rejects an unsupported attachment type as a usage error") - func rejectsUnsupportedType() { - let error = ExplanationAttachmentAddCommandError.unsupportedFileType("txt") - - #expect(error.exitCode == .validationFailure) - #expect(error.errorDescription == "Unsupported attachment type 'txt'") - #expect(error.takeaways.map { $0.plain() } == ["Attach a pdf, png, jpg, jpeg or gif file"]) - } -} diff --git a/Tests/FreeAgentCLITests/Models/AttachmentFileTests.swift b/Tests/FreeAgentCLITests/Models/AttachmentFileTests.swift new file mode 100644 index 0000000..76ac021 --- /dev/null +++ b/Tests/FreeAgentCLITests/Models/AttachmentFileTests.swift @@ -0,0 +1,52 @@ +import ArgumentParser +import Foundation +import Testing + +@testable import FreeAgentCLI + +struct AttachmentFileTests { + + // MARK: Internal + + @Test( + "takes the content type from the extension, ignoring case", + arguments: [ + ("pdf", "application/pdf"), + ("png", "image/png"), + ("jpg", "image/jpeg"), + ("JPEG", "image/jpeg"), + ("gif", "image/gif"), + ] + ) + func contentType(ext: String, contentType: String) throws { + let path = try Self.file(ext: ext, contents: "receipt") + + let attachment = try AttachmentFile(path: path) + + #expect(attachment.contentType == contentType) + #expect(attachment.fileName == URL(fileURLWithPath: path).lastPathComponent) + #expect(attachment.data == Data("receipt".utf8).base64EncodedString()) + } + + @Test("rejects an unsupported attachment type as a usage error") + func rejectsUnsupportedType() throws { + let path = try Self.file(ext: "txt", contents: "receipt") + + let error = #expect(throws: AttachmentFileError.self) { + try AttachmentFile(path: path) + } + + #expect(error?.exitCode == .validationFailure) + #expect(error?.errorDescription == "Unsupported attachment type 'txt'") + #expect(error?.takeaways.map { $0.plain() } == ["Attach a pdf, png, jpg, jpeg or gif file"]) + } + + // MARK: Private + + private static func file(ext: String, contents: String) throws -> String { + let path = FileManager.default.temporaryDirectory.appendingPathComponent("\(UUID().uuidString).\(ext)").path + try Data(contents.utf8).write(to: URL(fileURLWithPath: path)) + return path + } + +} diff --git a/openapi/components/schemas/ExpenseAttachmentUpdatePayload.yaml b/openapi/components/schemas/ExpenseAttachmentUpdatePayload.yaml new file mode 100644 index 0000000..21a32ca --- /dev/null +++ b/openapi/components/schemas/ExpenseAttachmentUpdatePayload.yaml @@ -0,0 +1,20 @@ +type: object +properties: + data: + type: string + description: Binary data of the file encoded as base64 + example: iVBORw0KGgo... + file_name: + type: string + example: receipt.pdf + content_type: + type: string + description: Ignored - FreeAgent detects the type from the data + example: application/pdf + description: + type: string + example: Invoice receipt + _destroy: + type: integer + description: Set to 1 to remove the attachment - any value removes it, 0 included + example: 1 diff --git a/openapi/components/schemas/ExpenseCreatePayload.yaml b/openapi/components/schemas/ExpenseCreatePayload.yaml index d900c69..1a4149e 100644 --- a/openapi/components/schemas/ExpenseCreatePayload.yaml +++ b/openapi/components/schemas/ExpenseCreatePayload.yaml @@ -64,3 +64,6 @@ properties: have_vat_receipt: type: boolean example: true + attachment: + $ref: '#/components/schemas/AttachmentPayload' + description: A receipt of up to 5MB diff --git a/openapi/components/schemas/ExpenseUpdatePayload.yaml b/openapi/components/schemas/ExpenseUpdatePayload.yaml index 625d8b2..4d69e62 100644 --- a/openapi/components/schemas/ExpenseUpdatePayload.yaml +++ b/openapi/components/schemas/ExpenseUpdatePayload.yaml @@ -51,3 +51,6 @@ properties: have_vat_receipt: type: boolean example: true + attachment: + $ref: '#/components/schemas/ExpenseAttachmentUpdatePayload' + description: A receipt of up to 5MB, replacing any the expense has and deleting it, or _destroy to remove it diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml index 3e90d9b..271ffab 100644 --- a/openapi/openapi.yaml +++ b/openapi/openapi.yaml @@ -183,6 +183,8 @@ components: $ref: 'components/schemas/ExpenseCreatePayload.yaml' ExpenseUpdatePayload: $ref: 'components/schemas/ExpenseUpdatePayload.yaml' + ExpenseAttachmentUpdatePayload: + $ref: 'components/schemas/ExpenseAttachmentUpdatePayload.yaml' ExpenseRecurringPeriod: $ref: 'components/schemas/ExpenseRecurringPeriod.yaml' ExpenseRebillType: From 8f6afb3a12e1424e6bbaa81a82bc85edfe218628 Mon Sep 17 00:00:00 2001 From: Dino Constantinou Date: Wed, 7 Oct 2026 08:24:42 +0100 Subject: [PATCH 2/3] Move Expense Attachments Onto Create And Update The attachment is a field on the expense, with no endpoint of its own, so it belongs on the existing commands rather than in a command group. expense create and update take --attachment and --attachment-description, and update takes --remove-attachment, which sends _destroy. Create gets its own ExpenseAttachmentPayload, since FreeAgent ignores content_type there too. Co-Authored-By: Claude Opus 5.5 --- .../ExpenseAttachmentAddCommand.swift | 84 ------------------ .../ExpenseAttachmentRemoveCommand.swift | 69 --------------- .../Expense/ExpenseAttachmentCommand.swift | 12 --- .../Expense/ExpenseUpdateCommand.swift | 16 +++- .../Commands/ExpenseCommand.swift | 1 - .../Options/ExpenseAttachmentOptions.swift | 38 ++++++++ .../FreeAgentCLI/Options/ExpenseOptions.swift | 19 ++-- .../ExpenseIntegrationTests.swift | 2 +- .../ExpenseAttachmentAddCommandTests.swift | 86 ------------------- .../ExpenseAttachmentRemoveCommandTests.swift | 65 -------------- .../ExpenseAttachmentOptionsTests.swift | 40 +++++++++ .../schemas/ExpenseAttachmentPayload.yaml | 19 ++++ .../schemas/ExpenseCreatePayload.yaml | 2 +- openapi/openapi.yaml | 2 + 14 files changed, 126 insertions(+), 329 deletions(-) delete mode 100644 Sources/FreeAgentCLI/Commands/Expense/Attachment/ExpenseAttachmentAddCommand.swift delete mode 100644 Sources/FreeAgentCLI/Commands/Expense/Attachment/ExpenseAttachmentRemoveCommand.swift delete mode 100644 Sources/FreeAgentCLI/Commands/Expense/ExpenseAttachmentCommand.swift create mode 100644 Sources/FreeAgentCLI/Options/ExpenseAttachmentOptions.swift delete mode 100644 Tests/FreeAgentCLITests/Commands/Expense/Attachment/ExpenseAttachmentAddCommandTests.swift delete mode 100644 Tests/FreeAgentCLITests/Commands/Expense/Attachment/ExpenseAttachmentRemoveCommandTests.swift create mode 100644 Tests/FreeAgentCLITests/Options/ExpenseAttachmentOptionsTests.swift create mode 100644 openapi/components/schemas/ExpenseAttachmentPayload.yaml diff --git a/Sources/FreeAgentCLI/Commands/Expense/Attachment/ExpenseAttachmentAddCommand.swift b/Sources/FreeAgentCLI/Commands/Expense/Attachment/ExpenseAttachmentAddCommand.swift deleted file mode 100644 index ef91fd4..0000000 --- a/Sources/FreeAgentCLI/Commands/Expense/Attachment/ExpenseAttachmentAddCommand.swift +++ /dev/null @@ -1,84 +0,0 @@ -import ArgumentParser -import Foundation -import FreeAgentAPI -import Noora - -// MARK: - ExpenseAttachmentAddCommand - -struct ExpenseAttachmentAddCommand: MutatingCommand { - static let configuration = CommandConfiguration( - commandName: "add", - abstract: "Add an attachment to an expense", - discussion: "An expense holds one attachment of up to 5MB. Remove the one it has before adding another." - ) - - @Argument(help: "Expense ID or URL") - var id: ResourceID - - @Option(name: .long, help: "Path to the file to attach (pdf, png, jpg, jpeg or gif)") - var file: String - - @Option(name: .long, help: "Description of the attached file") - var description: String? - - @Flag(help: "Print the request instead of sending it") - var dryRun = false - - @Flag(name: .long, help: "Output JSON") - var json = false - - func perform(client: Client) async throws -> Components.Schemas.ExpenseResponse { - let attachment = try AttachmentFile(path: file) - - let expense = try await client.getASingleExpense(.init(path: .init(id: id.value))).ok.body.json.expense - - if let existing = expense.attachment { - throw ExpenseAttachmentAddCommandError.alreadyAttached(id: id, fileName: existing.fileName) - } - - let input = Operations.UpdateExpense.Input( - path: .init(id: id.value), - body: .json(.init(expense: .init(attachment: .init( - data: attachment.data, - fileName: attachment.fileName, - contentType: attachment.contentType, - description: description - )))) - ) - - return try await client.updateExpense(input) - .ok.body.json - } - - func success(for _: Components.Schemas.ExpenseResponse) -> String { - "Added the attachment to expense \(id)" - } -} - -// MARK: - ExpenseAttachmentAddCommandError - -enum ExpenseAttachmentAddCommandError: CommandError { - case alreadyAttached(id: ResourceID, fileName: String?) - - // MARK: Internal - - var errorDescription: String? { - switch self { - case .alreadyAttached(let id, let fileName?): - "Expense \(id) already has an attachment, \(fileName)" - case .alreadyAttached(let id, nil): - "Expense \(id) already has an attachment" - } - } - - var exitCode: ExitCode { - APIError.Kind.rejected.exitCode - } - - var takeaways: [TerminalText] { - switch self { - case .alreadyAttached(let id, _): - ["Run \(.command("freeagent expense attachment remove \(id)")) first to replace it"] - } - } -} diff --git a/Sources/FreeAgentCLI/Commands/Expense/Attachment/ExpenseAttachmentRemoveCommand.swift b/Sources/FreeAgentCLI/Commands/Expense/Attachment/ExpenseAttachmentRemoveCommand.swift deleted file mode 100644 index 1a20d72..0000000 --- a/Sources/FreeAgentCLI/Commands/Expense/Attachment/ExpenseAttachmentRemoveCommand.swift +++ /dev/null @@ -1,69 +0,0 @@ -import ArgumentParser -import Foundation -import FreeAgentAPI -import Noora - -// MARK: - ExpenseAttachmentRemoveCommand - -struct ExpenseAttachmentRemoveCommand: DestructiveCommand { - static let configuration = CommandConfiguration( - commandName: "remove", - abstract: "Remove the attachment from an expense", - discussion: "FreeAgent deletes the attached file." - ) - - @Argument(help: "Expense ID or URL") - var id: ResourceID - - @Flag(help: "Print the request instead of sending it") - var dryRun = false - - @Flag(help: "Skip the confirmation prompt") - var yes = false - - @Flag(name: .long, help: "Output JSON") - var json = false - - var confirmation: String { - "Remove the attachment from expense \(id)?" - } - - func perform(client: Client) async throws -> Components.Schemas.ExpenseResponse { - let expense = try await client.getASingleExpense(.init(path: .init(id: id.value))).ok.body.json.expense - - guard expense.attachment != nil else { - throw ExpenseAttachmentRemoveCommandError.noAttachment(id: id) - } - - let input = Operations.UpdateExpense.Input( - path: .init(id: id.value), - body: .json(.init(expense: .init(attachment: .init(_destroy: 1)))) - ) - - return try await client.updateExpense(input) - .ok.body.json - } - - func success(for _: Components.Schemas.ExpenseResponse) -> String { - "Removed the attachment from expense \(id)" - } -} - -// MARK: - ExpenseAttachmentRemoveCommandError - -enum ExpenseAttachmentRemoveCommandError: CommandError { - case noAttachment(id: ResourceID) - - // MARK: Internal - - var errorDescription: String? { - switch self { - case .noAttachment(let id): - "Expense \(id) has no attachment" - } - } - - var exitCode: ExitCode { - APIError.Kind.notFound.exitCode - } -} diff --git a/Sources/FreeAgentCLI/Commands/Expense/ExpenseAttachmentCommand.swift b/Sources/FreeAgentCLI/Commands/Expense/ExpenseAttachmentCommand.swift deleted file mode 100644 index ee05ad4..0000000 --- a/Sources/FreeAgentCLI/Commands/Expense/ExpenseAttachmentCommand.swift +++ /dev/null @@ -1,12 +0,0 @@ -import ArgumentParser - -struct ExpenseAttachmentCommand: AsyncParsableCommand { - static let configuration = CommandConfiguration( - commandName: "attachment", - abstract: "Manage the attachment on an expense", - subcommands: [ - ExpenseAttachmentAddCommand.self, - ExpenseAttachmentRemoveCommand.self, - ] - ) -} diff --git a/Sources/FreeAgentCLI/Commands/Expense/ExpenseUpdateCommand.swift b/Sources/FreeAgentCLI/Commands/Expense/ExpenseUpdateCommand.swift index a0853b7..811f32e 100644 --- a/Sources/FreeAgentCLI/Commands/Expense/ExpenseUpdateCommand.swift +++ b/Sources/FreeAgentCLI/Commands/Expense/ExpenseUpdateCommand.swift @@ -8,7 +8,7 @@ struct ExpenseUpdateCommand: MutatingCommand { abstract: "Update an expense", discussion: """ Fields left out are kept, and an empty --receipt-reference removes it. A mileage claim's engine can't be \ - changed. The attachment is managed with 'freeagent expense attachment'. + changed. --attachment replaces any attachment the expense has, and FreeAgent deletes the old file. """ ) @@ -29,6 +29,9 @@ struct ExpenseUpdateCommand: MutatingCommand { @OptionGroup var expense: ExpenseOptions + @Flag(name: .long, help: "Remove the expense's attachment, which FreeAgent deletes") + var removeAttachment = false + @Flag(help: "Print the request instead of sending it") var dryRun = false @@ -36,11 +39,12 @@ struct ExpenseUpdateCommand: MutatingCommand { var json = false func perform(client: Client) async throws -> Components.Schemas.ExpenseResponse { - let expensePayload = expense.updatePayload( + let expensePayload = try expense.updatePayload( user: user?.value, category: category?.value, datedOn: datedOn, - description: description + description: description, + removeAttachment: removeAttachment ) let input = Operations.UpdateExpense.Input( @@ -52,6 +56,12 @@ struct ExpenseUpdateCommand: MutatingCommand { .ok.body.json } + func validate() throws { + if removeAttachment, expense.attachment.path != nil { + throw ValidationError("Pass --attachment or --remove-attachment, not both") + } + } + func success(for _: Components.Schemas.ExpenseResponse) -> String { "Updated expense \(id)" } diff --git a/Sources/FreeAgentCLI/Commands/ExpenseCommand.swift b/Sources/FreeAgentCLI/Commands/ExpenseCommand.swift index 9ff716b..2a8ea02 100644 --- a/Sources/FreeAgentCLI/Commands/ExpenseCommand.swift +++ b/Sources/FreeAgentCLI/Commands/ExpenseCommand.swift @@ -10,7 +10,6 @@ struct ExpenseCommand: AsyncParsableCommand { ExpenseShowCommand.self, ExpenseUpdateCommand.self, ExpenseDeleteCommand.self, - ExpenseAttachmentCommand.self, ExpenseMileageSettingsCommand.self, ] ) diff --git a/Sources/FreeAgentCLI/Options/ExpenseAttachmentOptions.swift b/Sources/FreeAgentCLI/Options/ExpenseAttachmentOptions.swift new file mode 100644 index 0000000..f19da7f --- /dev/null +++ b/Sources/FreeAgentCLI/Options/ExpenseAttachmentOptions.swift @@ -0,0 +1,38 @@ +import ArgumentParser +import FreeAgentAPI + +struct ExpenseAttachmentOptions: ParsableArguments { + @Option(name: .customLong("attachment"), help: "Path to a receipt to attach (pdf, png, jpg, jpeg or gif), up to 5MB") + var path: String? + + @Option(name: .customLong("attachment-description"), help: "Description of the attached receipt - needs --attachment") + var description: String? + + var createPayload: Components.Schemas.ExpenseAttachmentPayload? { + get throws { + guard let path else { + return nil + } + + let file = try AttachmentFile(path: path) + return .init(data: file.data, fileName: file.fileName, contentType: file.contentType, description: description) + } + } + + var updatePayload: Components.Schemas.ExpenseAttachmentUpdatePayload? { + get throws { + guard let path else { + return nil + } + + let file = try AttachmentFile(path: path) + return .init(data: file.data, fileName: file.fileName, contentType: file.contentType, description: description) + } + } + + func validate() throws { + if description != nil, path == nil { + throw ValidationError("--attachment-description needs --attachment") + } + } +} diff --git a/Sources/FreeAgentCLI/Options/ExpenseOptions.swift b/Sources/FreeAgentCLI/Options/ExpenseOptions.swift index a8723f4..b26abf0 100644 --- a/Sources/FreeAgentCLI/Options/ExpenseOptions.swift +++ b/Sources/FreeAgentCLI/Options/ExpenseOptions.swift @@ -46,14 +46,16 @@ struct ExpenseOptions: ParsableArguments { @Option(name: .long, help: "Whether a mileage claim has a VAT receipt for its fuel") var haveVatReceipt: Bool? + @OptionGroup var attachment: ExpenseAttachmentOptions + func createPayload( user: String, category: String, datedOn: String, description: String, engine: ExpenseEngineOptions - ) -> Components.Schemas.ExpenseCreatePayload { - .init( + ) throws -> Components.Schemas.ExpenseCreatePayload { + try .init( user: user, category: category, datedOn: datedOn, @@ -73,7 +75,8 @@ struct ExpenseOptions: ParsableArguments { vehicleType: vehicleType, engineType: engine.type, engineSize: engine.size, - haveVatReceipt: haveVatReceipt + haveVatReceipt: haveVatReceipt, + attachment: attachment.createPayload ) } @@ -81,9 +84,10 @@ struct ExpenseOptions: ParsableArguments { user: String?, category: String?, datedOn: String?, - description: String? - ) -> Components.Schemas.ExpenseUpdatePayload { - .init( + description: String?, + removeAttachment: Bool + ) throws -> Components.Schemas.ExpenseUpdatePayload { + try .init( user: user, category: category, datedOn: datedOn, @@ -101,7 +105,8 @@ struct ExpenseOptions: ParsableArguments { recurringEndDate: recurring.endDate, mileage: mileage, vehicleType: vehicleType, - haveVatReceipt: haveVatReceipt + haveVatReceipt: haveVatReceipt, + attachment: removeAttachment ? .init(_destroy: 1) : attachment.updatePayload ) } } diff --git a/Tests/FreeAgentAPIIntegrationTests/ExpenseIntegrationTests.swift b/Tests/FreeAgentAPIIntegrationTests/ExpenseIntegrationTests.swift index d6573be..edfe805 100644 --- a/Tests/FreeAgentAPIIntegrationTests/ExpenseIntegrationTests.swift +++ b/Tests/FreeAgentAPIIntegrationTests/ExpenseIntegrationTests.swift @@ -150,7 +150,7 @@ struct ExpenseIntegrationTests { attachment: .init( data: Self.onePixelPNG, fileName: "receipt.png", - contentType: .imagePng, + contentType: "image/png", description: "Integration test receipt" ) )))) diff --git a/Tests/FreeAgentCLITests/Commands/Expense/Attachment/ExpenseAttachmentAddCommandTests.swift b/Tests/FreeAgentCLITests/Commands/Expense/Attachment/ExpenseAttachmentAddCommandTests.swift deleted file mode 100644 index bce50b1..0000000 --- a/Tests/FreeAgentCLITests/Commands/Expense/Attachment/ExpenseAttachmentAddCommandTests.swift +++ /dev/null @@ -1,86 +0,0 @@ -import ArgumentParser -import Foundation -import FreeAgentAPI -import HTTPTypes -import Mockable -import OpenAPIRuntime -import Testing - -@testable import FreeAgentCLI - -struct ExpenseAttachmentAddCommandTests { - - // MARK: Internal - - @Test("adds the attachment to an expense without one") - func adds() async throws { - let transport = transport(showing: #"{"expense":{"url":"https://api.freeagent.com/v2/expenses/577140"}}"#) - let command = try ExpenseAttachmentAddCommand.parse(["577140", "--file", Self.receipt()]) - - let response = try await command.perform(client: client(transport)) - - #expect(command.success(for: response) == "Added the attachment to expense 577140") - verify(transport).send(.any, body: .any, baseURL: .any, operationID: .value("updateExpense")).called(1) - } - - @Test("refuses an expense that already has an attachment, naming its file") - func alreadyAttached() async throws { - let transport = transport( - showing: #"{"expense":{"url":"https://api.freeagent.com/v2/expenses/577140","attachment":{"url":"https://api.freeagent.com/v2/attachments/183146","file_name":"receipt.png"}}}"# - ) - let command = try ExpenseAttachmentAddCommand.parse(["577140", "--file", Self.receipt()]) - - let error = await #expect(throws: ExpenseAttachmentAddCommandError.self) { - try await command.perform(client: client(transport)) - } - - #expect(error?.errorDescription == "Expense 577140 already has an attachment, receipt.png") - #expect(error?.exitCode == ExitCode(4)) - #expect(error?.takeaways.map { $0.plain() } == [ - "Run 'freeagent expense attachment remove 577140' first to replace it" - ]) - verify(transport).send(.any, body: .any, baseURL: .any, operationID: .value("updateExpense")).called(0) - } - - @Test("rejects an unsupported file before reading the expense") - func unsupportedFile() async throws { - let transport = transport(showing: #"{"expense":{"url":"https://api.freeagent.com/v2/expenses/577140"}}"#) - let command = try ExpenseAttachmentAddCommand.parse(["577140", "--file", "notes.txt"]) - - await #expect(throws: AttachmentFileError.self) { - try await command.perform(client: client(transport)) - } - - verify(transport).send(.any, body: .any, baseURL: .any, operationID: .value("getASingleExpense")).called(0) - } - - // MARK: Private - - private static func receipt() throws -> String { - let path = FileManager.default.temporaryDirectory.appendingPathComponent("\(UUID().uuidString).pdf").path - try Data("%PDF".utf8).write(to: URL(fileURLWithPath: path)) - return path - } - - private func transport(showing body: String) -> MockClientTransportInterface { - let transport = MockClientTransportInterface() - given(transport) - .send(.any, body: .any, baseURL: .any, operationID: .value("getASingleExpense")) - .willReturn(( - HTTPResponse(status: .ok, headerFields: [.contentType: "application/json"]), - HTTPBody(Data(body.utf8)) - )) - given(transport) - .send(.any, body: .any, baseURL: .any, operationID: .value("updateExpense")) - .willReturn(( - HTTPResponse(status: .ok, headerFields: [.contentType: "application/json"]), - HTTPBody(Data(#"{"expense":{"url":"https://api.freeagent.com/v2/expenses/577140"}}"#.utf8)) - )) - - return transport - } - - private func client(_ transport: MockClientTransportInterface) -> Client { - Client(serverURL: Environment.sandbox.baseURL, transport: transport, middlewares: [.apiError()]) - } -} diff --git a/Tests/FreeAgentCLITests/Commands/Expense/Attachment/ExpenseAttachmentRemoveCommandTests.swift b/Tests/FreeAgentCLITests/Commands/Expense/Attachment/ExpenseAttachmentRemoveCommandTests.swift deleted file mode 100644 index dca299a..0000000 --- a/Tests/FreeAgentCLITests/Commands/Expense/Attachment/ExpenseAttachmentRemoveCommandTests.swift +++ /dev/null @@ -1,65 +0,0 @@ -import ArgumentParser -import Foundation -import FreeAgentAPI -import HTTPTypes -import Mockable -import OpenAPIRuntime -import Testing - -@testable import FreeAgentCLI - -struct ExpenseAttachmentRemoveCommandTests { - - // MARK: Internal - - @Test("removes the attachment from an expense with one") - func removes() async throws { - let transport = transport( - showing: #"{"expense":{"url":"https://api.freeagent.com/v2/expenses/577140","attachment":{"url":"https://api.freeagent.com/v2/attachments/183146"}}}"# - ) - let command = try ExpenseAttachmentRemoveCommand.parse(["577140", "--yes"]) - - let response = try await command.perform(client: client(transport)) - - #expect(command.success(for: response) == "Removed the attachment from expense 577140") - verify(transport).send(.any, body: .any, baseURL: .any, operationID: .value("updateExpense")).called(1) - } - - @Test("refuses an expense with no attachment as not found") - func noAttachment() async throws { - let transport = transport(showing: #"{"expense":{"url":"https://api.freeagent.com/v2/expenses/577140"}}"#) - let command = try ExpenseAttachmentRemoveCommand.parse(["577140", "--yes"]) - - let error = await #expect(throws: ExpenseAttachmentRemoveCommandError.self) { - try await command.perform(client: client(transport)) - } - - #expect(error?.errorDescription == "Expense 577140 has no attachment") - #expect(error?.exitCode == ExitCode(3)) - verify(transport).send(.any, body: .any, baseURL: .any, operationID: .value("updateExpense")).called(0) - } - - // MARK: Private - - private func transport(showing body: String) -> MockClientTransportInterface { - let transport = MockClientTransportInterface() - given(transport) - .send(.any, body: .any, baseURL: .any, operationID: .value("getASingleExpense")) - .willReturn(( - HTTPResponse(status: .ok, headerFields: [.contentType: "application/json"]), - HTTPBody(Data(body.utf8)) - )) - given(transport) - .send(.any, body: .any, baseURL: .any, operationID: .value("updateExpense")) - .willReturn(( - HTTPResponse(status: .ok, headerFields: [.contentType: "application/json"]), - HTTPBody(Data(#"{"expense":{"url":"https://api.freeagent.com/v2/expenses/577140"}}"#.utf8)) - )) - - return transport - } - - private func client(_ transport: MockClientTransportInterface) -> Client { - Client(serverURL: Environment.sandbox.baseURL, transport: transport, middlewares: [.apiError()]) - } -} diff --git a/Tests/FreeAgentCLITests/Options/ExpenseAttachmentOptionsTests.swift b/Tests/FreeAgentCLITests/Options/ExpenseAttachmentOptionsTests.swift new file mode 100644 index 0000000..b04ca7f --- /dev/null +++ b/Tests/FreeAgentCLITests/Options/ExpenseAttachmentOptionsTests.swift @@ -0,0 +1,40 @@ +import ArgumentParser +import Testing + +@testable import FreeAgentCLI + +struct ExpenseAttachmentOptionsTests { + @Test("refuses --attachment-description without --attachment") + func descriptionNeedsAttachment() { + #expect(throws: (any Error).self) { + try ExpenseAttachmentOptions.parse(["--attachment-description", "Hotel receipt"]) + } + } + + @Test("accepts --attachment-description with --attachment") + func descriptionWithAttachment() throws { + let options = try ExpenseAttachmentOptions.parse([ + "--attachment", + "receipt.pdf", + "--attachment-description", + "Hotel receipt", + ]) + + #expect(options.path == "receipt.pdf") + #expect(options.description == "Hotel receipt") + } + + @Test("refuses --attachment with --remove-attachment on expense update") + func updateAttachmentAndRemove() { + #expect(throws: (any Error).self) { + try ExpenseUpdateCommand.parse(["577139", "--attachment", "receipt.pdf", "--remove-attachment"]) + } + } + + @Test("accepts --remove-attachment alone on expense update") + func updateRemove() throws { + let command = try ExpenseUpdateCommand.parse(["577139", "--remove-attachment"]) + + #expect(command.removeAttachment) + } +} diff --git a/openapi/components/schemas/ExpenseAttachmentPayload.yaml b/openapi/components/schemas/ExpenseAttachmentPayload.yaml new file mode 100644 index 0000000..47b19df --- /dev/null +++ b/openapi/components/schemas/ExpenseAttachmentPayload.yaml @@ -0,0 +1,19 @@ +type: object +required: +- data +- file_name +properties: + data: + type: string + description: Binary data of the file encoded as base64 + example: iVBORw0KGgo... + file_name: + type: string + example: receipt.pdf + content_type: + type: string + description: Ignored - FreeAgent detects the type from the data + example: application/pdf + description: + type: string + example: Invoice receipt diff --git a/openapi/components/schemas/ExpenseCreatePayload.yaml b/openapi/components/schemas/ExpenseCreatePayload.yaml index 1a4149e..d087333 100644 --- a/openapi/components/schemas/ExpenseCreatePayload.yaml +++ b/openapi/components/schemas/ExpenseCreatePayload.yaml @@ -65,5 +65,5 @@ properties: type: boolean example: true attachment: - $ref: '#/components/schemas/AttachmentPayload' + $ref: '#/components/schemas/ExpenseAttachmentPayload' description: A receipt of up to 5MB diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml index 271ffab..6043c54 100644 --- a/openapi/openapi.yaml +++ b/openapi/openapi.yaml @@ -183,6 +183,8 @@ components: $ref: 'components/schemas/ExpenseCreatePayload.yaml' ExpenseUpdatePayload: $ref: 'components/schemas/ExpenseUpdatePayload.yaml' + ExpenseAttachmentPayload: + $ref: 'components/schemas/ExpenseAttachmentPayload.yaml' ExpenseAttachmentUpdatePayload: $ref: 'components/schemas/ExpenseAttachmentUpdatePayload.yaml' ExpenseRecurringPeriod: From 3e98a0e42e7e73e4ae821382ce9c58042638ec0a Mon Sep 17 00:00:00 2001 From: Dino Constantinou Date: Wed, 7 Oct 2026 08:32:03 +0100 Subject: [PATCH 3/3] Report An Unreadable Attachment File A missing, unreadable or directory path passed to --attachment or --file surfaced as a raw Foundation error with exit 1. AttachmentFile now reports it as a usage error naming the path. Co-Authored-By: Claude Opus 5.5 --- Sources/FreeAgentCLI/Models/AttachmentFile.swift | 14 +++++++++++++- .../Models/AttachmentFileTests.swift | 13 +++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/Sources/FreeAgentCLI/Models/AttachmentFile.swift b/Sources/FreeAgentCLI/Models/AttachmentFile.swift index c5393bd..5bf8ad0 100644 --- a/Sources/FreeAgentCLI/Models/AttachmentFile.swift +++ b/Sources/FreeAgentCLI/Models/AttachmentFile.swift @@ -16,7 +16,12 @@ struct AttachmentFile { throw AttachmentFileError.unsupportedFileType(ext) } - data = try Data(contentsOf: url).base64EncodedString() + do { + data = try Data(contentsOf: url).base64EncodedString() + } catch { + throw AttachmentFileError.unreadable(path: path) + } + fileName = url.lastPathComponent self.contentType = contentType } @@ -43,11 +48,16 @@ struct AttachmentFile { enum AttachmentFileError: CommandError { case unsupportedFileType(String) + case unreadable(path: String) + + // MARK: Internal var errorDescription: String? { switch self { case .unsupportedFileType(let ext): "Unsupported attachment type '\(ext)'" + case .unreadable(let path): + "Couldn't read \(path)" } } @@ -59,6 +69,8 @@ enum AttachmentFileError: CommandError { switch self { case .unsupportedFileType: ["Attach a pdf, png, jpg, jpeg or gif file"] + case .unreadable: + ["Check the file exists and is readable"] } } } diff --git a/Tests/FreeAgentCLITests/Models/AttachmentFileTests.swift b/Tests/FreeAgentCLITests/Models/AttachmentFileTests.swift index 76ac021..616a7b4 100644 --- a/Tests/FreeAgentCLITests/Models/AttachmentFileTests.swift +++ b/Tests/FreeAgentCLITests/Models/AttachmentFileTests.swift @@ -41,6 +41,19 @@ struct AttachmentFileTests { #expect(error?.takeaways.map { $0.plain() } == ["Attach a pdf, png, jpg, jpeg or gif file"]) } + @Test("reports a file it can't read as a usage error, naming the path") + func rejectsUnreadableFile() { + let path = FileManager.default.temporaryDirectory.appendingPathComponent("\(UUID().uuidString).pdf").path + + let error = #expect(throws: AttachmentFileError.self) { + try AttachmentFile(path: path) + } + + #expect(error?.exitCode == .validationFailure) + #expect(error?.errorDescription == "Couldn't read \(path)") + #expect(error?.takeaways.map { $0.plain() } == ["Check the file exists and is readable"]) + } + // MARK: Private private static func file(ext: String, contents: String) throws -> String {