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.

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(openapi): Remove lastMessage from TalkRoom object when empty #13891

Merged
merged 3 commits into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion lib/ResponseDefinitions.php
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@
* isFavorite: bool,
* lastActivity: int,
* lastCommonReadMessage: int,
* lastMessage: TalkRoomLastMessage|array<empty>,
* lastMessage?: TalkRoomLastMessage,
* lastPing: int,
* lastReadMessage: int,
* listable: int,
Expand Down
15 changes: 8 additions & 7 deletions lib/Service/RoomFormatter.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ public function formatRoomV4(
'lobbyTimer' => 0,
'lastPing' => 0,
'sessionId' => '0',
'lastMessage' => [],
'sipEnabled' => Webinary::SIP_DISABLED,
'actorType' => '',
'actorId' => '',
Expand Down Expand Up @@ -376,15 +375,17 @@ public function formatRoomV4(
}
}

$roomData['lastMessage'] = [];
$lastMessage = $room->getLastMessage();
if (!$room->isFederatedConversation() && $lastMessage instanceof IComment) {
$roomData['lastMessage'] = $this->formatLastMessage(
$lastMessageData = $this->formatLastMessage(
$responseFormat,
$room,
$currentParticipant,
$lastMessage,
);
if ($lastMessageData !== null) {
$roomData['lastMessage'] = $lastMessageData;
}
} elseif ($room->isFederatedConversation()) {
$roomData['lastCommonReadMessage'] = 0;
try {
Expand Down Expand Up @@ -414,25 +415,25 @@ public function formatRoomV4(
}

/**
* @return TalkRoomLastMessage|array<empty>
* @return TalkRoomLastMessage|null
*/
public function formatLastMessage(
string $responseFormat,
Room $room,
Participant $participant,
IComment $lastMessage,
): array {
): ?array {
$message = $this->messageParser->createMessage($room, $participant, $lastMessage, $this->l10n);
$this->messageParser->parseMessage($message);

if (!$message->getVisibility()) {
return [];
return null;
}

$now = $this->timeFactory->getDateTime();
$expireDate = $message->getComment()->getExpireDate();
if ($expireDate instanceof \DateTime && $expireDate < $now) {
return [];
return null;
}

return $message->toArray($responseFormat);
Expand Down
11 changes: 1 addition & 10 deletions openapi-backend-sipbridge.json
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,6 @@
"isFavorite",
"lastActivity",
"lastCommonReadMessage",
"lastMessage",
"lastPing",
"lastReadMessage",
"listable",
Expand Down Expand Up @@ -676,15 +675,7 @@
"format": "int64"
},
"lastMessage": {
"anyOf": [
{
"$ref": "#/components/schemas/RoomLastMessage"
},
{
"type": "array",
"maxItems": 0
}
]
"$ref": "#/components/schemas/RoomLastMessage"
},
"lastPing": {
"type": "integer",
Expand Down
11 changes: 1 addition & 10 deletions openapi-federation.json
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,6 @@
"isFavorite",
"lastActivity",
"lastCommonReadMessage",
"lastMessage",
"lastPing",
"lastReadMessage",
"listable",
Expand Down Expand Up @@ -730,15 +729,7 @@
"format": "int64"
},
"lastMessage": {
"anyOf": [
{
"$ref": "#/components/schemas/RoomLastMessage"
},
{
"type": "array",
"maxItems": 0
}
]
"$ref": "#/components/schemas/RoomLastMessage"
},
"lastPing": {
"type": "integer",
Expand Down
11 changes: 1 addition & 10 deletions openapi-full.json
Original file line number Diff line number Diff line change
Expand Up @@ -1184,7 +1184,6 @@
"isFavorite",
"lastActivity",
"lastCommonReadMessage",
"lastMessage",
"lastPing",
"lastReadMessage",
"listable",
Expand Down Expand Up @@ -1307,15 +1306,7 @@
"format": "int64"
},
"lastMessage": {
"anyOf": [
{
"$ref": "#/components/schemas/RoomLastMessage"
},
{
"type": "array",
"maxItems": 0
}
]
"$ref": "#/components/schemas/RoomLastMessage"
},
"lastPing": {
"type": "integer",
Expand Down
11 changes: 1 addition & 10 deletions openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -1071,7 +1071,6 @@
"isFavorite",
"lastActivity",
"lastCommonReadMessage",
"lastMessage",
"lastPing",
"lastReadMessage",
"listable",
Expand Down Expand Up @@ -1194,15 +1193,7 @@
"format": "int64"
},
"lastMessage": {
"anyOf": [
{
"$ref": "#/components/schemas/RoomLastMessage"
},
{
"type": "array",
"maxItems": 0
}
]
"$ref": "#/components/schemas/RoomLastMessage"
},
"lastPing": {
"type": "integer",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ describe('Conversation.vue', () => {
})

test('displays nothing when there is no last chat message', () => {
item.lastMessage = {}
delete item.lastMessage
testConversationLabel(item, 'No messages')
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,6 @@ export default {
type: 0,
displayName: '',
isFavorite: false,
lastMessage: {},
notificationLevel: PARTICIPANT.NOTIFY.DEFAULT,
notificationCalls: PARTICIPANT.NOTIFY_CALLS.ON,
canDeleteConversation: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ export default {
displayName: '',
isFavorite: false,
notificationLevel: 0,
lastMessage: {},
}
},
},
Expand Down
5 changes: 0 additions & 5 deletions src/components/LeftSidebar/LeftSidebar.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,36 +225,31 @@ describe('LeftSidebar.vue', () => {
isFavorite: false,
name: 'one',
displayName: 'the searched one by display name',
lastMessage: {},
}, {
id: 200,
token: 't200',
lastActivity: 80,
isFavorite: false,
name: 'searched by name',
displayName: 'another one',
lastMessage: {},
}, {
id: 300,
token: 't300',
lastActivity: 120,
isFavorite: true,
name: 'excluded',
displayName: 'excluded from results',
lastMessage: {},
}]

listedResults = [{
id: 1000,
name: 'listed one searched',
displayName: 'listed one searched',
lastMessage: {},
token: 'listed-token-1',
}, {
id: 1001,
name: 'listed two searched',
displayName: 'listed two searched',
lastMessage: {},
token: 'listed-token-2',
}]
usersResults = [{
Expand Down
2 changes: 1 addition & 1 deletion src/composables/useConversationInfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export function useConversationInfo({
})

const hasLastMessage = computed(() => {
return !!Object.keys(Object(item.value?.lastMessage)).length
DorraJaouad marked this conversation as resolved.
Show resolved Hide resolved
return !!item.value?.lastMessage && !!Object.keys(Object(item.value?.lastMessage)).length
})

/**
Expand Down
13 changes: 10 additions & 3 deletions src/store/conversationsStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,13 @@ const actions = {
updateConversationIfHasChanged(context, conversation) {
const oldConversation = context.state.conversations[conversation.token]

// Update conversation if the number of attributes differ
// (e.g. lastMessage was added or removed, because we don't keep lastMessage as an empty object)
if (Object.keys(oldConversation).length !== Object.keys(conversation).length) {
context.commit('updateConversation', conversation)
return true
}

// Update 1-1 conversation, if its status was changed
if (conversation.type === CONVERSATION.TYPE.ONE_TO_ONE
&& (oldConversation.status !== conversation.status
Expand All @@ -336,7 +343,7 @@ const actions = {
return true
}

// Check if any property were changed (no properties except status-related supposed to be added or deleted)
// Check if any property were changed (no properties except status-related and lastMessage supposed to be added or deleted)
for (const key of Object.keys(conversation)) {
// "lastMessage" is the only property with non-primitive (object) value and cannot be compared by ===
// If "lastMessage" was actually changed, it is already checked by "lastActivity"
Expand Down Expand Up @@ -760,8 +767,8 @@ const actions = {
}

const conversation = Object.assign({}, getters.conversations[token])
if (conversation.lastMessage.id === parseInt(messageId, 10)
|| conversation.lastMessage.timestamp >= Date.parse(notification.datetime) / 1000) {
if (conversation.lastMessage?.id === parseInt(messageId, 10)
|| conversation.lastMessage?.timestamp >= Date.parse(notification.datetime) / 1000) {
// Already updated from other source, skipping
return
}
Expand Down
10 changes: 5 additions & 5 deletions src/store/messagesStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ const getters = {
return false
}

return getters.getLastKnownMessageId(token) < conversation.lastMessage.id
return getters.getLastKnownMessageId(token) < conversation.lastMessage?.id
},

isMessagesListPopulated: (state) => (token) => {
Expand Down Expand Up @@ -552,7 +552,7 @@ const actions = {

if (message.systemMessage === 'message_edited' || message.systemMessage === 'message_deleted') {
// update conversation lastMessage, if it was edited or deleted
if (message.parent.id === context.getters.conversation(token).lastMessage.id) {
if (message.parent.id === context.getters.conversation(token).lastMessage?.id) {
context.dispatch('updateConversationLastMessage', { token, lastMessage: message.parent })
}
// Check existing messages for having a deleted / edited message as parent, and update them
Expand Down Expand Up @@ -1118,7 +1118,7 @@ const actions = {

// Overwrite the conversation.hasCall property so people can join
// after seeing the message in the chat.
if (conversation && conversation.lastMessage && message.id > conversation.lastMessage.id) {
if (conversation?.lastMessage && message.id > conversation.lastMessage.id) {
if (['call_started', 'call_ended', 'call_ended_everyone', 'call_missed'].includes(message.systemMessage)) {
context.dispatch('overwriteHasCallByChat', {
token,
Expand Down Expand Up @@ -1150,7 +1150,7 @@ const actions = {
id: parseInt(response.headers['x-chat-last-given'], 10),
})

if (conversation && conversation.lastMessage && lastMessage.id > conversation.lastMessage.id) {
if (conversation?.lastMessage && lastMessage.id > conversation.lastMessage.id) {
context.dispatch('updateConversationLastMessage', {
token,
lastMessage,
Expand Down Expand Up @@ -1237,7 +1237,7 @@ const actions = {

// update lastMessage and lastReadMessage
// do it conditionally because there could have been more messages appearing concurrently
if (conversation && conversation.lastMessage && message.id > conversation.lastMessage.id) {
if (conversation?.lastMessage && message.id > conversation.lastMessage.id) {
context.dispatch('updateConversationLastMessage', {
token,
lastMessage: message,
Expand Down
2 changes: 1 addition & 1 deletion src/types/openapi/openapi-backend-sipbridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ export type components = {
lastActivity: number;
/** Format: int64 */
lastCommonReadMessage: number;
lastMessage: components["schemas"]["RoomLastMessage"] | unknown[];
lastMessage?: components["schemas"]["RoomLastMessage"];
/** Format: int64 */
lastPing: number;
/** Format: int64 */
Expand Down
2 changes: 1 addition & 1 deletion src/types/openapi/openapi-federation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ export type components = {
lastActivity: number;
/** Format: int64 */
lastCommonReadMessage: number;
lastMessage: components["schemas"]["RoomLastMessage"] | unknown[];
lastMessage?: components["schemas"]["RoomLastMessage"];
/** Format: int64 */
lastPing: number;
/** Format: int64 */
Expand Down
2 changes: 1 addition & 1 deletion src/types/openapi/openapi-full.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2253,7 +2253,7 @@ export type components = {
lastActivity: number;
/** Format: int64 */
lastCommonReadMessage: number;
lastMessage: components["schemas"]["RoomLastMessage"] | unknown[];
lastMessage?: components["schemas"]["RoomLastMessage"];
/** Format: int64 */
lastPing: number;
/** Format: int64 */
Expand Down
2 changes: 1 addition & 1 deletion src/types/openapi/openapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1734,7 +1734,7 @@ export type components = {
lastActivity: number;
/** Format: int64 */
lastCommonReadMessage: number;
lastMessage: components["schemas"]["RoomLastMessage"] | unknown[];
lastMessage?: components["schemas"]["RoomLastMessage"];
/** Format: int64 */
lastPing: number;
/** Format: int64 */
Expand Down
4 changes: 2 additions & 2 deletions src/utils/getMessageIcon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ const iconSvgTemplate = (path: string) => {
}

export const getMessageIcon = (lastMessage: Conversation['lastMessage']): string => {
if (Array.isArray(lastMessage)) {
if (!lastMessage || Array.isArray(lastMessage)) {
return ''
}
const file = lastMessage?.messageParameters?.file
const file = lastMessage.messageParameters?.file
if (file) {
if (file.mimetype?.startsWith('video')) {
return iconSvgTemplate(mdiMovie) // Media - Videos
Expand Down
6 changes: 5 additions & 1 deletion tests/integration/features/bootstrap/FeatureContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,11 @@ private function assertRooms(array $rooms, TableNode $formData, bool $shouldOrde
$data['attendeePin'] = $room['attendeePin'] ? '**PIN**' : '';
}
if (isset($expectedRoom['lastMessage'])) {
$data['lastMessage'] = $room['lastMessage'] ? $room['lastMessage']['message'] : '';
if (isset($room['lastMessage'])) {
$data['lastMessage'] = $room['lastMessage'] ? $room['lastMessage']['message'] : '';
} else {
$data['lastMessage'] = 'UNSET';
}
}
if (isset($expectedRoom['lastMessageActorType'])) {
$data['lastMessageActorType'] = $room['lastMessage'] ? $room['lastMessage']['actorType'] : '';
Expand Down
6 changes: 3 additions & 3 deletions tests/integration/features/conversation-3/lobby.feature
Original file line number Diff line number Diff line change
Expand Up @@ -250,16 +250,16 @@ Feature: conversation/lobby
| room | the description | 3 | 2 | {actor} set the description |
And user "participant3" is participant of room "room" (v4)
| name | description | type | participantType | lastMessage |
| room | the description | 3 | 3 | |
| room | the description | 3 | 3 | UNSET |
And user "participant4" is participant of room "room" (v4)
| name | description | type | participantType | lastMessage |
| room | the description | 3 | 5 | |
| room | the description | 3 | 5 | UNSET |
And user "guest" is participant of room "room" (v4)
| name | description | type | participantType | lastMessage |
| room | the description | 3 | 6 | {actor} set the description |
And user "guest2" is participant of room "room" (v4)
| name | description | type | participantType | lastMessage |
| room | the description | 3 | 4 | |
| room | the description | 3 | 4 | UNSET |


# Not all the values are checked in the test, only the most relevant ones
Expand Down
Loading