Skip to content

Commit

Permalink
MDL-69411 core_message: Fix return structure for empty result set
Browse files Browse the repository at this point in the history
Passing a timefrom higher than last message timecreated needs to
return a formatted response so it does not break ws.
  • Loading branch information
dravek committed Nov 23, 2020
1 parent c8d33eb commit c99a148
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 1 deletion.
2 changes: 1 addition & 1 deletion message/classes/api.php
Expand Up @@ -1114,7 +1114,7 @@ public static function get_conversation_messages(int $userid, int $convid, int $
// The last known message time is earlier than the one being requested so we can
// just return an empty result set rather than having to query the DB.
if ($lastcreated && $lastcreated < $timefrom) {
return [];
return helper::format_conversation_messages($userid, $convid, []);
}
}

Expand Down
41 changes: 41 additions & 0 deletions message/tests/api_test.php
Expand Up @@ -6589,6 +6589,47 @@ public function test_delete_message_for_all_users_individual_conversation() {
$this->assertNotEquals($mid1, $convmessages2['messages'][0]->id);
}

/**
* Test retrieving conversation messages by providing a timefrom higher than last message timecreated. It should return no
* messages but keep the return structure to not break when called from the ws.
*/
public function test_get_conversation_messages_timefrom_higher_than_last_timecreated() {
// Create some users.
$user1 = self::getDataGenerator()->create_user();
$user2 = self::getDataGenerator()->create_user();
$user3 = self::getDataGenerator()->create_user();
$user4 = self::getDataGenerator()->create_user();

// Create group conversation.
$conversation = \core_message\api::create_conversation(
\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP,
[$user1->id, $user2->id, $user3->id, $user4->id]
);

// The person doing the search.
$this->setUser($user1);

// Send some messages back and forth.
$time = 1;
testhelper::send_fake_message_to_conversation($user1, $conversation->id, 'Message 1', $time + 1);
testhelper::send_fake_message_to_conversation($user2, $conversation->id, 'Message 2', $time + 2);
testhelper::send_fake_message_to_conversation($user1, $conversation->id, 'Message 3', $time + 3);
testhelper::send_fake_message_to_conversation($user3, $conversation->id, 'Message 4', $time + 4);

// Retrieve the messages from $time + 5, which should return no messages.
$convmessages = \core_message\api::get_conversation_messages($user1->id, $conversation->id, 0, 0, '', $time + 5);

// Confirm the conversation id is correct.
$this->assertEquals($conversation->id, $convmessages['id']);

// Confirm the message data is correct.
$messages = $convmessages['messages'];
$this->assertEquals(0, count($messages));

// Confirm that members key is present.
$this->assertArrayHasKey('members', $convmessages);
}

/**
* Helper to seed the database with initial state with data.
*/
Expand Down
46 changes: 46 additions & 0 deletions message/tests/externallib_test.php
Expand Up @@ -5727,6 +5727,52 @@ public function test_delete_message_for_all_users_private_conversation() {
core_message_external::delete_message_for_all_users($messageid, $user1->id);
}

/**
* Test retrieving conversation messages by providing a timefrom higher than last message timecreated. It should return no
* messages but keep the return structure to not break when called from the ws.
*/
public function test_get_conversation_messages_timefrom_higher_than_last_timecreated() {
$this->resetAfterTest(true);

// Create some users.
$user1 = self::getDataGenerator()->create_user();
$user2 = self::getDataGenerator()->create_user();
$user3 = self::getDataGenerator()->create_user();
$user4 = self::getDataGenerator()->create_user();

// Create group conversation.
$conversation = \core_message\api::create_conversation(
\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP,
[$user1->id, $user2->id, $user3->id, $user4->id]
);

// The person asking for the messages for another user.
$this->setUser($user1);

// Send some messages back and forth.
$time = 1;
testhelper::send_fake_message_to_conversation($user1, $conversation->id, 'Message 1', $time + 1);
testhelper::send_fake_message_to_conversation($user2, $conversation->id, 'Message 2', $time + 2);
testhelper::send_fake_message_to_conversation($user1, $conversation->id, 'Message 3', $time + 3);
testhelper::send_fake_message_to_conversation($user3, $conversation->id, 'Message 4', $time + 4);

// Retrieve the messages.
$result = core_message_external::get_conversation_messages($user1->id, $conversation->id, 0, 0, '', $time + 5);

// We need to execute the return values cleaning process to simulate the web service server.
$result = external_api::clean_returnvalue(core_message_external::get_conversation_messages_returns(), $result);

// Check the results are correct.
$this->assertEquals($conversation->id, $result['id']);

// Confirm the message data is correct.
$messages = $result['messages'];
$this->assertEquals(0, count($messages));

// Confirm that members key is present.
$this->assertArrayHasKey('members', $result);
}

/**
* Helper to seed the database with initial state with data.
*/
Expand Down

0 comments on commit c99a148

Please sign in to comment.