Skip to content

Commit

Permalink
use true parent prop for replies
Browse files Browse the repository at this point in the history
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
  • Loading branch information
Antreesy authored and backportbot-nextcloud[bot] committed Aug 23, 2023
1 parent fe40ac8 commit 00758c1
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -295,10 +295,9 @@ describe('Message.vue', () => {
actorType: ATTENDEE.ACTOR_TYPE.USERS,
messageParameters: {},
token: TOKEN,
parentId: -1,
reactions: '',
}
messageProps.parent = 120
messageProps.parent = parentMessage

const messageGetterMock = jest.fn().mockReturnValue(parentMessage)
testStoreConfig.modules.messagesStore.getters.message = jest.fn(() => messageGetterMock)
Expand All @@ -311,9 +310,6 @@ describe('Message.vue', () => {
provide: injected,
})

// parent message got queried from the store
expect(messageGetterMock).toHaveBeenCalledWith(TOKEN, 120)

const quote = wrapper.findComponent(Quote)
expect(quote.exists()).toBe(true)
expect(quote.attributes('message')).toBe('quoted text')
Expand Down
14 changes: 5 additions & 9 deletions src/components/MessagesList/MessagesGroup/Message/Message.vue
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ the main body of the message as well as a quote.
class="message-body__main">
<div v-if="isSingleEmoji"
class="message-body__main__text">
<Quote v-if="parent" :parent-id="parent" v-bind="quote" />
<Quote v-if="parent" v-bind="parent" />
<div class="single-emoji">
{{ message }}
</div>
Expand Down Expand Up @@ -77,7 +77,7 @@ the main body of the message as well as a quote.
:reference-limit="0" />
</div>
<div v-else class="message-body__main__text message-body__main__text--markdown">
<Quote v-if="parent" :parent-id="parent" v-bind="quote" />
<Quote v-if="parent" v-bind="parent" />
<NcRichText :text="message"
:arguments="richParameters"
autolink
Expand Down Expand Up @@ -395,11 +395,11 @@ export default {
required: true,
},
/**
* The parent message's id.
* The parent message.
*/
parent: {
type: Number,
default: 0,
type: Object,
default: undefined,
},
/**
* Is message allowed to render in markdown
Expand Down Expand Up @@ -493,10 +493,6 @@ export default {
return moment(this.timestamp * 1000).format('LL')
},
quote() {
return this.parent && this.$store.getters.message(this.token, this.parent)
},
conversation() {
return this.$store.getters.conversation(this.token)
},
Expand Down
10 changes: 3 additions & 7 deletions src/components/NewMessage/NewMessage.vue
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,7 @@
</NcButton>
</div>
<div v-if="messageToBeReplied" class="new-message-form__quote">
<Quote is-new-message-quote
:parent-id="messageToBeReplied.id"
v-bind="messageToBeReplied" />
<Quote is-new-message-quote v-bind="messageToBeReplied" />
</div>
<NcRichContenteditable ref="richContenteditable"
v-shortkey.once="$options.disableKeyboardShortcuts ? null : ['c']"
Expand Down Expand Up @@ -548,11 +546,9 @@ export default {
// Restore the parent/quote message
if (temporaryMessage.parent) {
const temporaryParent = this.$store.getters.message(this.token, temporaryMessage.parent)
this.$store.dispatch('addMessageToBeReplied', {
token: temporaryParent.token,
id: temporaryParent.id,
token: this.token,
id: temporaryMessage.parent.id,
})
}
Expand Down
16 changes: 8 additions & 8 deletions src/components/Quote.vue
Original file line number Diff line number Diff line change
Expand Up @@ -139,13 +139,6 @@ export default {
type: Boolean,
default: false,
},
/**
* The parent message's id
*/
parentId: {
type: Number,
required: true,
},
},
computed: {
/**
Expand Down Expand Up @@ -251,7 +244,14 @@ export default {
},
handleQuoteClick() {
EventBus.$emit('focus-message', this.parentId)
const parentHash = '#message_' + this.id
if (this.$route.hash !== parentHash) {
// Change route to trigger message fetch, if not fetched yet
this.$router.replace(parentHash)
} else {
// Already on this message route, just trigger highlight
EventBus.$emit('focus-message', this.id)
}
},
},
}
Expand Down
4 changes: 2 additions & 2 deletions src/services/messagesService.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ const getMessageContext = async function({ token, messageId, limit = 50 }, optio
* @param {string} param0.message The message object
* @param {string} param0.actorDisplayName The display name of the actor
* @param {string} param0.referenceId A reference id to identify the message later again
* @param {number} param0.parent The id of the message to be replied to
* @param {object|undefined} param0.parent The message to be replied to
* @param {object} options request options
* @param {boolean} options.silent whether the message should trigger a notifications for the
* recipients or not
Expand All @@ -108,7 +108,7 @@ const postNewMessage = async function({ token, message, actorDisplayName, refere
message,
actorDisplayName,
referenceId,
replyTo: parent,
replyTo: parent?.id,
silent: options.silent,
}, options)
}
Expand Down
2 changes: 1 addition & 1 deletion src/services/messagesService.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ describe('messagesService', () => {
message: 'hello world!',
actorDisplayName: 'actor-display-name',
referenceId: 'reference-id',
parent: 111,
parent: { id: 111 },
}, {
dummyOption: true,
})
Expand Down
32 changes: 15 additions & 17 deletions src/store/messagesStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -505,25 +505,22 @@ const actions = {
* @param {object} message the message;
*/
processMessage(context, message) {
if (message.parent) {
// Handle cases for messages with replies and some system messages
if (message.systemMessage === 'message_deleted'
if (message.parent && message.systemMessage
&& (message.systemMessage === 'message_deleted'
|| message.systemMessage === 'reaction'
|| message.systemMessage === 'reaction_deleted'
|| message.systemMessage === 'reaction_revoked') {
// If parent message is presented in store already, we update it
const parentInStore = context.getters.message(message.token, message.parent.id)
if (Object.keys(parentInStore).length !== 0) {
context.commit('addMessage', message.parent)
context.dispatch('resetReactions', {
token: message.token,
messageId: message.parent,
})
}
// Quit processing
return
|| message.systemMessage === 'reaction_revoked')) {
// If parent message is presented in store already, we update it
const parentInStore = context.getters.message(message.token, message.parent.id)
if (Object.keys(parentInStore).length !== 0) {
context.commit('addMessage', message.parent)
context.dispatch('resetReactions', {
token: message.token,
messageId: message.parent,
})
}
message.parent = message.parent.id
// Quit processing
return
}

if (message.referenceId) {
Expand Down Expand Up @@ -607,6 +604,7 @@ const actions = {
*/
createTemporaryMessage(context, { text, token, uploadId, index, file, localUrl, isVoiceMessage }) {
const parentId = context.getters.getMessageToBeReplied(token)
const parent = parentId && context.getters.message(token, parentId)
const date = new Date()
let tempId = 'temp-' + date.getTime()
const messageParameters = {}
Expand Down Expand Up @@ -636,7 +634,7 @@ const actions = {
message: text,
messageParameters,
token,
parent: parentId ?? 0,
parent,
isReplyable: false,
sendingFailure: '',
reactions: {},
Expand Down
40 changes: 19 additions & 21 deletions src/store/messagesStore.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ describe('messagesStore', () => {
expect(store.getters.messagesList(TOKEN)).toHaveLength(0)
})

test('adds message with its parent to the list', () => {
test('adds user\'s message with included parent to the store', () => {
const parentMessage = {
id: 1,
token: TOKEN,
Expand All @@ -119,15 +119,11 @@ describe('messagesStore', () => {
id: 2,
token: TOKEN,
parent: parentMessage,
messageType: 'comment',
}

store.dispatch('processMessage', message1)
expect(store.getters.messagesList(TOKEN)[0]).toBe(parentMessage)
expect(store.getters.messagesList(TOKEN)[1]).toStrictEqual({
id: 2,
token: TOKEN,
parent: 1,
})
expect(store.getters.messagesList(TOKEN)).toMatchObject([message1])
})

test('deletes matching temporary message when referenced', () => {
Expand Down Expand Up @@ -358,7 +354,7 @@ describe('messagesStore', () => {
expect(getActorTypeMock).toHaveBeenCalled()
expect(getDisplayNameMock).toHaveBeenCalled()

expect(temporaryMessage).toStrictEqual({
expect(temporaryMessage).toMatchObject({
id: 'temp-1577908800000',
actorId: 'actor-id-1',
actorType: ATTENDEE.ACTOR_TYPE.USERS,
Expand All @@ -369,7 +365,6 @@ describe('messagesStore', () => {
message: 'blah',
messageParameters: {},
token: TOKEN,
parent: 0,
isReplyable: false,
sendingFailure: '',
reactions: {},
Expand All @@ -378,6 +373,14 @@ describe('messagesStore', () => {
})

test('creates temporary message with message to be replied', async () => {
const parent = {
id: 123,
token: TOKEN,
message: 'hello',
}

store.dispatch('processMessage', parent)

getMessageToBeRepliedMock.mockReset()
getMessageToBeRepliedMock.mockReturnValue(() => (123))

Expand All @@ -390,7 +393,7 @@ describe('messagesStore', () => {
localUrl: null,
})

expect(temporaryMessage).toStrictEqual({
expect(temporaryMessage).toMatchObject({
id: 'temp-1577908800000',
actorId: 'actor-id-1',
actorType: ATTENDEE.ACTOR_TYPE.USERS,
Expand All @@ -401,7 +404,7 @@ describe('messagesStore', () => {
message: 'blah',
messageParameters: {},
token: TOKEN,
parent: 123,
parent,
isReplyable: false,
sendingFailure: '',
reactions: {},
Expand All @@ -424,7 +427,7 @@ describe('messagesStore', () => {
localUrl: 'local-url://original-name.txt',
})

expect(temporaryMessage).toStrictEqual({
expect(temporaryMessage).toMatchObject({
id: expect.stringMatching(/^temp-1577908800000-upload-id-1-0\.[0-9]*$/),
actorId: 'actor-id-1',
actorType: ATTENDEE.ACTOR_TYPE.USERS,
Expand All @@ -446,7 +449,6 @@ describe('messagesStore', () => {
},
},
token: TOKEN,
parent: 0,
isReplyable: false,
sendingFailure: '',
reactions: {},
Expand All @@ -466,7 +468,7 @@ describe('messagesStore', () => {

store.dispatch('addTemporaryMessage', temporaryMessage)

expect(store.getters.messagesList(TOKEN)).toStrictEqual([{
expect(store.getters.messagesList(TOKEN)).toMatchObject([{
id: 'temp-1577908800000',
actorId: 'actor-id-1',
actorType: ATTENDEE.ACTOR_TYPE.USERS,
Expand All @@ -477,7 +479,6 @@ describe('messagesStore', () => {
message: 'blah',
messageParameters: {},
token: TOKEN,
parent: 0,
isReplyable: false,
sendingFailure: '',
reactions: {},
Expand All @@ -490,7 +491,7 @@ describe('messagesStore', () => {
temporaryMessage.message = 'replaced'
store.dispatch('addTemporaryMessage', temporaryMessage)

expect(store.getters.messagesList(TOKEN)).toStrictEqual([{
expect(store.getters.messagesList(TOKEN)).toMatchObject([{
id: 'temp-1577908800000',
actorId: 'actor-id-1',
actorType: ATTENDEE.ACTOR_TYPE.USERS,
Expand All @@ -501,7 +502,6 @@ describe('messagesStore', () => {
message: 'replaced',
messageParameters: {},
token: TOKEN,
parent: 0,
isReplyable: false,
sendingFailure: '',
reactions: {},
Expand All @@ -525,7 +525,7 @@ describe('messagesStore', () => {
reason: 'failure-reason',
})

expect(store.getters.messagesList(TOKEN)).toStrictEqual([{
expect(store.getters.messagesList(TOKEN)).toMatchObject([{
id: 'temp-1577908800000',
actorId: 'actor-id-1',
actorType: ATTENDEE.ACTOR_TYPE.USERS,
Expand All @@ -536,7 +536,6 @@ describe('messagesStore', () => {
message: 'blah',
messageParameters: {},
token: TOKEN,
parent: 0,
isReplyable: false,
sendingFailure: 'failure-reason',
reactions: {},
Expand Down Expand Up @@ -572,7 +571,7 @@ describe('messagesStore', () => {

store.dispatch('addTemporaryMessage', temporaryMessage)

expect(store.getters.getTemporaryReferences(TOKEN, temporaryMessage.referenceId)).toStrictEqual([{
expect(store.getters.getTemporaryReferences(TOKEN, temporaryMessage.referenceId)).toMatchObject([{
id: 'temp-1577908800000',
actorId: 'actor-id-1',
actorType: ATTENDEE.ACTOR_TYPE.USERS,
Expand All @@ -583,7 +582,6 @@ describe('messagesStore', () => {
message: 'blah',
messageParameters: {},
token: TOKEN,
parent: 0,
isReplyable: false,
sendingFailure: '',
reactions: {},
Expand Down

0 comments on commit 00758c1

Please sign in to comment.