Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: latestAction -> action renaming + test #85

Merged
merged 1 commit into from
Dec 6, 2024
+36 −13
Merged

fix: latestAction -> action renaming + test #85

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions chat-android/src/main/java/com/ably/chat/ChatApi.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ internal class ChatApi(
method = "GET",
params = params,
) {
val latestActionName = it.requireJsonObject().get("latestAction")?.asString
val latestActionName = it.requireJsonObject().get("action")?.asString
sacOO7 marked this conversation as resolved.
Show resolved Hide resolved
val latestAction = latestActionName?.let { name -> messageActionNameToAction[name] }

latestAction?.let { action ->
Expand All @@ -47,7 +47,7 @@ internal class ChatApi(
createdAt = it.requireLong("createdAt"),
metadata = it.asJsonObject.get("metadata"),
headers = it.asJsonObject.get("headers")?.toMap() ?: mapOf(),
latestAction = action,
action = action,
)
}
}
Expand Down Expand Up @@ -87,7 +87,7 @@ internal class ChatApi(
createdAt = it.requireLong("createdAt"),
metadata = params.metadata,
headers = params.headers ?: mapOf(),
latestAction = MessageAction.MESSAGE_CREATE,
action = MessageAction.MESSAGE_CREATE,
)
} ?: throw AblyException.fromErrorInfo(ErrorInfo("Send message endpoint returned empty value", HttpStatusCode.InternalServerError))
}
Expand Down
2 changes: 1 addition & 1 deletion chat-android/src/main/java/com/ably/chat/Message.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,5 +73,5 @@ data class Message(
/**
* The latest action of the message. This can be used to determine if the message was created, updated, or deleted.
*/
val latestAction: MessageAction,
val action: MessageAction,
)
2 changes: 1 addition & 1 deletion chat-android/src/main/java/com/ably/chat/Messages.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ internal class DefaultMessages(
text = data.text,
metadata = data.metadata,
headers = pubSubMessage.extras.asJsonObject().get("headers")?.toMap() ?: mapOf(),
latestAction = MessageAction.MESSAGE_CREATE,
action = MessageAction.MESSAGE_CREATE,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Remaining instances of latestAction found in the codebase

The verification revealed that latestAction is still being used in two files:

  • ChatApi.kt: Used as local variables (latestActionName, latestAction) for processing message actions
  • MainActivity.kt in the example app: Still using the old property name

The local variables in ChatApi.kt are fine as they serve a different purpose, but the example app needs to be updated to maintain consistency.

  • example/src/main/java/com/ably/chat/example/MainActivity.kt: Update latestAction to action in Message construction
🔗 Analysis chain

LGTM: Property renamed from latestAction to action

The renaming aligns with the PR objectives to match changes in the chat-js repository. The hardcoded MessageAction.MESSAGE_CREATE value is consistent with the message listener's action check.

Let's verify the consistent renaming across the codebase:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify consistent renaming from latestAction to action across the codebase

# Check for any remaining instances of latestAction
echo "Checking for remaining instances of 'latestAction'..."
rg --type kotlin --type java "latestAction"

# Check for consistent usage of 'action' property in Message construction
echo "Checking Message construction patterns..."
ast-grep --pattern 'Message($$$, action = $_, $$$)'

Length of output: 7784

)
Comment on lines 292 to 294
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Update needed in example app's MainActivity

The Message class constructor has been properly updated to use action parameter, but there's an inconsistency in the example app's MainActivity.kt which still uses the old latestAction parameter name. This needs to be updated to match the current Message class definition.

  • example/src/main/java/com/ably/chat/example/MainActivity.kt:323: Change latestAction = MessageAction.MESSAGE_CREATE to action = MessageAction.MESSAGE_CREATE
🔗 Analysis chain

Verify Message class constructor parameter update

While the property renaming here looks correct, we should ensure the Message class constructor has been updated to accept action instead of latestAction.

Let's verify the Message class changes:

Let me generate another verification script to check the Message class definition since the previous pattern didn't match.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify Message class constructor parameter update

# Check Message class definition
echo "Checking Message class definition..."
ast-grep --pattern 'class Message($$$, action: MessageAction, $$$)'

# Check all Message instantiations
echo "Checking Message instantiations..."
ast-grep --pattern 'Message($$$)'

Length of output: 8257

Script:

#!/bin/bash
# Search for Message class definition with a more flexible pattern
echo "Checking Message class definition..."
rg -A 10 "^data class Message\b" --type kotlin

# Also check for any remaining references to latestAction
echo -e "\nChecking for latestAction references..."
rg "latestAction" --type kotlin

Length of output: 1534

Script:

#!/bin/bash
# Get more context around Message class definition
echo "Getting full Message class definition..."
rg -B 5 -A 20 "^data class Message\b" chat-android/src/main/java/com/ably/chat/Message.kt

# Get context around latestAction usage in MainActivity
echo -e "\nChecking MainActivity latestAction context..."
rg -B 5 -A 5 "latestAction = MessageAction" example/src/main/java/com/ably/chat/example/MainActivity.kt

Length of output: 1164

listener.onEvent(MessageEvent(type = MessageEventType.Created, message = chatMessage))
}
Expand Down
8 changes: 4 additions & 4 deletions chat-android/src/test/java/com/ably/chat/ChatApiTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class ChatApiTest {
addProperty("clientId", "clientId")
addProperty("text", "hello")
addProperty("createdAt", 1_000_000)
addProperty("latestAction", "message.create")
addProperty("action", "message.create")
},
),
)
Expand All @@ -49,7 +49,7 @@ class ChatApiTest {
createdAt = 1_000_000L,
metadata = null,
headers = mapOf(),
latestAction = MessageAction.MESSAGE_CREATE,
action = MessageAction.MESSAGE_CREATE,
),
),
messages.items,
Expand All @@ -66,7 +66,7 @@ class ChatApiTest {
listOf(
JsonObject().apply {
addProperty("foo", "bar")
addProperty("latestAction", "message.create")
addProperty("action", "message.create")
},
),
)
Expand Down Expand Up @@ -103,7 +103,7 @@ class ChatApiTest {
createdAt = 1_000_000L,
headers = mapOf(),
metadata = null,
latestAction = MessageAction.MESSAGE_CREATE,
action = MessageAction.MESSAGE_CREATE,
),
message,
)
Expand Down
4 changes: 2 additions & 2 deletions chat-android/src/test/java/com/ably/chat/MessagesTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class MessagesTest {
createdAt = 1_000_000,
metadata = JsonObject().apply { addProperty("meta", "data") },
headers = mapOf("foo" to "bar"),
latestAction = MessageAction.MESSAGE_CREATE,
action = MessageAction.MESSAGE_CREATE,
),
sentMessage,
)
Expand Down Expand Up @@ -135,7 +135,7 @@ class MessagesTest {
text = "some text",
metadata = null,
headers = mapOf("foo" to "bar"),
latestAction = MessageAction.MESSAGE_CREATE,
action = MessageAction.MESSAGE_CREATE,
),
messageEvent.message,
)
Expand Down
23 changes: 23 additions & 0 deletions chat-android/src/test/java/com/ably/chat/SandboxTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,29 @@ class SandboxTest {
)
}

@Test
fun `should be able to send and retrieve messages from history`() = runTest {
val chatClient = sandbox.createSandboxChatClient()
val roomId = UUID.randomUUID().toString()

val room = chatClient.rooms.get(roomId)

room.attach()

room.messages.send("hello")

lateinit var messages: List<Message>

assertWaiter {
messages = room.messages.get().items
messages.isNotEmpty()
}

assertEquals(1, messages.size)
assertEquals("hello", messages.first().text)
assertEquals("sandbox-client", messages.first().clientId)
}

companion object {

private lateinit var sandbox: Sandbox
Expand Down
2 changes: 1 addition & 1 deletion chat-android/src/test/java/com/ably/chat/TestUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ fun Occupancy.subscribeOnce(listener: Occupancy.Listener) {
}
}

suspend fun assertWaiter(timeoutInMs: Long = 10_000, block: () -> Boolean) {
suspend fun assertWaiter(timeoutInMs: Long = 10_000, block: suspend () -> Boolean) {
withContext(Dispatchers.Default) {
withTimeout(timeoutInMs) {
do {
Expand Down
2 changes: 1 addition & 1 deletion example/src/main/java/com/ably/chat/example/MainActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ fun MessageBubblePreview() {
createdAt = System.currentTimeMillis(),
metadata = null,
headers = mapOf(),
latestAction = MessageAction.MESSAGE_CREATE,
action = MessageAction.MESSAGE_CREATE,
),
)
}
Expand Down
Loading