Skip to content

Commit a7eb8ab

Browse files
diegotldiegotl
committed
Fix critical bugs from code review
- Fix rate limit handling by checking 429 before isSuccess - Fix InputBlock dispatch_action coding key (was dispatch_action_config) - Fix InputBlock element decoding using superDecoder - Fix DatePickerElement initialDate to String (YYYY-MM-DD format) - Fix TextContextElement to support configurable type field - Remove unused decoder from URLSessionNetworkClient - Add formatDate helper to DatePickerElement for proper date formatting - Add markdown static helper to TextContextElement
1 parent f743ee2 commit a7eb8ab

8 files changed

Lines changed: 82 additions & 30 deletions

File tree

‎Sources/SlackKit/Client/SlackWebhookClient.swift‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -86,19 +86,18 @@ public final actor SlackWebhookClient {
8686
// Send the request
8787
let response = try await networkClient.post(url: webhookURL, body: body)
8888

89+
// Check for rate limiting (must check before isSuccess)
90+
if response.statusCode == 429 {
91+
let retryAfter = extractRetryAfter(from: response) ?? 60
92+
throw SlackError.rateLimitExceeded(retryAfter: retryAfter)
93+
}
94+
8995
// Check for HTTP errors
9096
guard response.isSuccess else {
9197
let bodyString = String(data: response.data, encoding: .utf8)
9298
throw SlackError.invalidResponse(statusCode: response.statusCode, body: bodyString)
9399
}
94100

95-
// Check for rate limiting
96-
if response.statusCode == 429 {
97-
if let retryAfter = extractRetryAfter(from: response) {
98-
throw SlackError.rateLimitExceeded(retryAfter: retryAfter)
99-
}
100-
}
101-
102101
// Decode the response
103102
// Slack webhooks return "ok" as plain text on success
104103
if let bodyString = String(data: response.data, encoding: .utf8),

‎Sources/SlackKit/Client/URLSessionNetworkClient.swift‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,11 @@ import Foundation
88
/// A URLSession-based implementation of NetworkClient
99
public actor URLSessionNetworkClient: NetworkClient {
1010
private let session: URLSession
11-
private let decoder: JSONDecoder
1211

1312
/// Initializes a new URLSession network client
14-
/// - Parameters:
15-
/// - session: The URLSession to use for requests (defaults to shared)
16-
/// - decoder: The JSONDecoder to use for decoding responses (defaults to standard)
17-
public init(
18-
session: URLSession = .shared,
19-
decoder: JSONDecoder = JSONDecoder()
20-
) {
13+
/// - Parameter session: The URLSession to use for requests (defaults to shared)
14+
public init(session: URLSession = .shared) {
2115
self.session = session
22-
self.decoder = decoder
2316
}
2417

2518
public func post(url: URL, body: Data) async throws -> HTTPResponse {

‎Sources/SlackKit/Models/Blocks/ContextBlock.swift‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,19 @@ public protocol ContextElement: BlockElement {}
7171

7272
/// A text element for context blocks
7373
public struct TextContextElement: ContextElement {
74-
public let type: String = "plain_text"
74+
public let type: String // Can be "plain_text" or "mrkdwn"
7575
public var text: String
7676

77-
public init(text: String) {
77+
public init(text: String, type: String = "plain_text") {
7878
self.text = text
79+
self.type = type
80+
}
81+
82+
/// Creates a markdown text context element
83+
/// - Parameter markdown: The markdown text
84+
/// - Returns: A TextContextElement with markdown type
85+
public static func markdown(_ markdown: String) -> TextContextElement {
86+
TextContextElement(text: markdown, type: "mrkdwn")
7987
}
8088

8189
enum CodingKeys: String, CodingKey {

‎Sources/SlackKit/Models/Blocks/InputBlock.swift‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public struct InputBlock: Block {
5454
case blockID = "block_id"
5555
case label
5656
case element
57-
case dispatchAction = "dispatch_action_config"
57+
case dispatchAction = "dispatch_action"
5858
case hint
5959
case optional
6060
}
@@ -73,21 +73,23 @@ public struct InputBlock: Block {
7373
// Decode element polymorphically
7474
if let elementContainer = try? container.nestedContainer(keyedBy: CodingKeys.self, forKey: .element) {
7575
let elementType = try elementContainer.decode(String.self, forKey: .type)
76+
let elementDecoder = try container.superDecoder(forKey: .element)
77+
7678
switch elementType {
7779
case "plain_text_input":
78-
element = try PlainTextInputElement(from: decoder)
80+
element = try PlainTextInputElement(from: elementDecoder)
7981
case "static_select":
80-
element = try StaticSelectElement(from: decoder)
82+
element = try StaticSelectElement(from: elementDecoder)
8183
case "datepicker":
82-
element = try DatePickerElement(from: decoder)
84+
element = try DatePickerElement(from: elementDecoder)
8385
case "multi_static_select":
84-
element = try MultiStaticSelectElement(from: decoder)
86+
element = try MultiStaticSelectElement(from: elementDecoder)
8587
case "multi_users_select":
86-
element = try MultiUsersSelectElement(from: decoder)
88+
element = try MultiUsersSelectElement(from: elementDecoder)
8789
case "multi_conversations_select":
88-
element = try MultiConversationsSelectElement(from: decoder)
90+
element = try MultiConversationsSelectElement(from: elementDecoder)
8991
case "multi_channels_select":
90-
element = try MultiChannelsSelectElement(from: decoder)
92+
element = try MultiChannelsSelectElement(from: elementDecoder)
9193
default:
9294
throw DecodingError.dataCorruptedError(
9395
forKey: .element,

‎Sources/SlackKit/Models/Elements/DatePickerElement.swift‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,19 @@ public struct DatePickerElement: BlockElement {
77
public let type: String = "datepicker"
88
public var actionID: String?
99
public var placeholder: TextObject
10-
public var initialDate: Int?
10+
public var initialDate: String?
1111
public var confirm: ConfirmationDialog?
1212

1313
/// Initializes a new date picker element
1414
/// - Parameters:
1515
/// - actionID: An identifier for the action
1616
/// - placeholder: The placeholder text
17-
/// - initialDate: The initial date as a Unix timestamp
17+
/// - initialDate: The initial date in YYYY-MM-DD format
1818
/// - confirm: An optional confirmation dialog
1919
public init(
2020
actionID: String? = nil,
2121
placeholder: TextObject,
22-
initialDate: Int? = nil,
22+
initialDate: String? = nil,
2323
confirm: ConfirmationDialog? = nil
2424
) {
2525
self.actionID = actionID
@@ -28,6 +28,18 @@ public struct DatePickerElement: BlockElement {
2828
self.confirm = confirm
2929
}
3030

31+
/// Creates a date string in YYYY-MM-DD format from a Date
32+
/// - Parameter date: The date to format
33+
/// - Returns: A string in YYYY-MM-DD format
34+
public static func formatDate(_ date: Date) -> String {
35+
let formatter = DateFormatter()
36+
formatter.dateFormat = "yyyy-MM-dd"
37+
formatter.locale = Locale(identifier: "en_US_POSIX")
38+
formatter.calendar = Calendar(identifier: .iso8601)
39+
formatter.timeZone = TimeZone(secondsFromGMT: 0)
40+
return formatter.string(from: date)
41+
}
42+
3143
enum CodingKeys: String, CodingKey {
3244
case type
3345
case actionID = "action_id"

‎Tests/SlackKitTests/ClientTests/SlackWebhookClientTests.swift‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,32 @@ struct SlackWebhookClientTests {
115115
}
116116
}
117117

118+
@Test("Handle rate limit response")
119+
func handleRateLimitResponse() async throws {
120+
// Arrange
121+
let webhookURL = URL(string: "https://hooks.slack.com/services/T/B/C")!
122+
let mockClient = MockNetworkClient()
123+
let slackClient = SlackWebhookClient(webhookURL: webhookURL, networkClient: mockClient)
124+
125+
// Mock a 429 response without body hints
126+
let responseData = #"{"error": "rate_limited"}"#.data(using: .utf8)!
127+
await mockClient.addResponse(statusCode: 429, data: responseData)
128+
129+
// Act & Assert
130+
let message = Message(text: "Test")
131+
do {
132+
_ = try await slackClient.send(message)
133+
#expect(false, "Expected rate limit error")
134+
} catch let error as SlackError {
135+
switch error {
136+
case .rateLimitExceeded(let retryAfter):
137+
#expect(retryAfter == 60)
138+
default:
139+
#expect(false, "Unexpected error: \(error)")
140+
}
141+
}
142+
}
143+
118144
@Test("Send message with attachments")
119145
func sendMessageWithAttachments() async throws {
120146
// Arrange

‎Tests/SlackKitTests/IntegrationTests/SlackKitIntegrationTests.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,7 @@ struct SlackKitIntegrationTests {
546546
text: .plainText("Select a date:"),
547547
accessory: DatePickerElement(
548548
placeholder: .plainText("Pick a date"),
549-
initialDate: Int(Date().timeIntervalSince1970)
549+
initialDate: DatePickerElement.formatDate(Date())
550550
)
551551
)
552552
]

‎Tests/SlackKitTests/ModelTests/BlockTests.swift‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,18 @@ struct BlockTests {
200200
#expect(json.contains("\"value\":\"opt1\""))
201201
}
202202

203+
@Test("Format DatePicker initial date")
204+
func formatDatePickerInitialDate() throws {
205+
// Arrange
206+
let epoch = Date(timeIntervalSince1970: 0)
207+
208+
// Act
209+
let formatted = DatePickerElement.formatDate(epoch)
210+
211+
// Assert
212+
#expect(formatted == "1970-01-01")
213+
}
214+
203215
@Test("Encode Attachment with fields")
204216
func encodeAttachmentWithFields() throws {
205217
// Arrange

0 commit comments

Comments
 (0)