From e9deb577e037890c3e2be6588366c2e6ccd12110 Mon Sep 17 00:00:00 2001 From: Diego Mello Date: Fri, 16 Nov 2018 09:06:29 -0200 Subject: [PATCH] Regression: Resend messages with error (#532) --- __mocks__/react-native-gesture-handler.js | 1 + .../__snapshots__/Storyshots.test.js.snap | 632 ++++++++++++++++-- app/containers/MessageErrorActions.js | 8 +- app/containers/message/Message.js | 96 +-- app/containers/message/styles.js | 22 +- app/lib/methods/canOpenRoom.js | 2 +- app/lib/methods/getRooms.js | 2 +- app/lib/methods/loadMessagesForRoom.js | 2 +- app/lib/methods/loadMissedMessages.js | 4 +- app/lib/methods/readMessages.js | 2 +- app/lib/methods/sendMessage.js | 42 +- app/lib/methods/subscriptions/room.js | 2 +- app/lib/methods/subscriptions/rooms.js | 6 +- app/lib/rocketchat.js | 5 +- app/sagas/selectServer.js | 2 +- app/views/RoomView/index.js | 4 +- package-lock.json | 6 +- 17 files changed, 691 insertions(+), 147 deletions(-) diff --git a/__mocks__/react-native-gesture-handler.js b/__mocks__/react-native-gesture-handler.js index 4701ac260..da7b586dd 100644 --- a/__mocks__/react-native-gesture-handler.js +++ b/__mocks__/react-native-gesture-handler.js @@ -1,3 +1,4 @@ export const RectButton = () => 'View'; export const State = () => 'View'; export const LongPressGestureHandler = () => 'View'; +export const BorderlessButton = () => 'View'; diff --git a/__tests__/__snapshots__/Storyshots.test.js.snap b/__tests__/__snapshots__/Storyshots.test.js.snap index 40c926b09..326454c38 100644 --- a/__tests__/__snapshots__/Storyshots.test.js.snap +++ b/__tests__/__snapshots__/Storyshots.test.js.snap @@ -228,7 +228,15 @@ exports[`Storyshots Message list 1`] = ` } > - View + + View + Simple - View + + View + Long message - View - View - View - View - View + + View + + + View + + + View + + + View + + + View + Grouped messages - View + + View + Without header - View + + View + With alias - View + + View + Edited - View + + View + Static avatar - View + + View + Full name - View + + View + Mentions - View + + View + Emojis - View + + View + Custom Emojis - View + + View + Time format - View + + View + Reactions - View + + View + Multiple reactions - View - View - View - View + + View + + + View + + + View + + + View + Intercalated users - View + + View + - View + + View + - View - View + + View + + + View + - View + + View + Date and Unread separators - View - View + + View + + + View + With image - View + + View + With video - View + + View + With audio - View + + View + URL - View + + View + Custom fields - View + + View + Two short custom fields - View + + View + Broadcast - View + + View + Archived - View - View + + View + View + + + View + View + Error - View + + View + Temp - View + + View + Editing - View + + View + Removed - View + + View + Joined - View + + View + Room name changed - View + + View + Message pinned - View + + View + Has left the channel - View + + View + User removed - View + + View + User added - View + + View + User muted - View + + View + User unmuted - View + + View + Role added - View + + View + Role removed - View + + View + Changed description - View + + View + Changed announcement - View + + View + Changed topic - View + + View + Changed type - View + + View + Custom style - View + + View + Markdown emphasis - View + + View + Markdown headers - View + + View + Markdown links - View + + View + Markdown image - View + + View + Markdown code - View + + View + Markdown quote - View + + View + { + if (this.actionSheet && this.actionSheet.show) { + this.actionSheet.show(); + } + }); } handleResend = protectedFunction(() => { diff --git a/app/containers/message/Message.js b/app/containers/message/Message.js index e83f92acd..b2f32dc02 100644 --- a/app/containers/message/Message.js +++ b/app/containers/message/Message.js @@ -6,7 +6,9 @@ import { import Icon from 'react-native-vector-icons/MaterialIcons'; import moment from 'moment'; import { KeyboardUtils } from 'react-native-keyboard-input'; -import { State, RectButton, LongPressGestureHandler } from 'react-native-gesture-handler'; +import { + State, RectButton, LongPressGestureHandler, BorderlessButton +} from 'react-native-gesture-handler'; import Image from './Image'; import User from './User'; @@ -252,7 +254,11 @@ export default class Message extends PureComponent { return null; } const { onErrorPress } = this.props; - return ; + return ( + + + + ); } renderReaction = (reaction) => { @@ -337,48 +343,58 @@ export default class Message extends PureComponent { const accessibilityLabel = I18n.t('Message_accessibility', { user: author.username, time: moment(ts).format(timeFormat), message: msg }); return ( - nativeEvent.state === State.ACTIVE && onLongPress()} - > - + {this.renderError()} + nativeEvent.state === State.ACTIVE && onLongPress()} > - - - {this.renderError()} - {this.renderAvatar()} - - {this.renderUsername()} - {this.renderContent()} - {this.renderAttachment()} - {this.renderUrl()} - {this.renderReactions()} - {this.renderBroadcastReply()} + + + {this.renderAvatar()} + + {this.renderUsername()} + {this.renderContent()} + {this.renderAttachment()} + {this.renderUrl()} + {this.renderReactions()} + {this.renderBroadcastReply()} + + {reactionsModal + ? ( + + ) + : null + } - {reactionsModal - ? ( - - ) - : null - } - - - + + + ); } } diff --git a/app/containers/message/styles.js b/app/containers/message/styles.js index b6bbc3598..b74e9c3c8 100644 --- a/app/containers/message/styles.js +++ b/app/containers/message/styles.js @@ -1,16 +1,24 @@ import { StyleSheet, Platform } from 'react-native'; export default StyleSheet.create({ + root: { + flexDirection: 'row' + }, container: { - paddingVertical: 5 + paddingVertical: 5, + flexDirection: 'row', + width: '100%' }, messageContent: { flex: 1, marginLeft: 51 }, - hasHeader: { + messageContentWithHeader: { marginLeft: 15 }, + messageContentWithError: { + marginLeft: 0 + }, flex: { flexDirection: 'row', flex: 1 @@ -35,6 +43,9 @@ export default StyleSheet.create({ height: 20 }, temp: { opacity: 0.3 }, + marginBottom: { + marginBottom: 10 + }, codeStyle: { ...Platform.select({ ios: { fontFamily: 'Courier New' }, @@ -92,10 +103,9 @@ export default StyleSheet.create({ width: 17, height: 17 }, - errorIcon: { - paddingRight: 12, - paddingLeft: 0, - alignSelf: 'center' + errorButton: { + paddingHorizontal: 15, + paddingVertical: 5 }, broadcastButton: { width: 107, diff --git a/app/lib/methods/canOpenRoom.js b/app/lib/methods/canOpenRoom.js index f576b2e15..d37e8e933 100644 --- a/app/lib/methods/canOpenRoom.js +++ b/app/lib/methods/canOpenRoom.js @@ -48,7 +48,7 @@ export default async function canOpenRoom({ rid, path }) { const [type, name] = path.split('/'); try { - const data = await (SDK.driver.ddp ? canOpenRoomDDP.call(this, { rid, type, name }) : canOpenRoomREST.call(this, { type, rid })); + const data = await (this.connected() ? canOpenRoomDDP.call(this, { rid, type, name }) : canOpenRoomREST.call(this, { type, rid })); return data; } catch (e) { log('canOpenRoom', e); diff --git a/app/lib/methods/getRooms.js b/app/lib/methods/getRooms.js index e1fbf2993..acf7ed01c 100644 --- a/app/lib/methods/getRooms.js +++ b/app/lib/methods/getRooms.js @@ -36,7 +36,7 @@ export default function() { return new Promise(async(resolve, reject) => { try { - const { subscriptions, rooms } = await (SDK.driver.ddp ? getRoomDpp.apply(this) : getRoomRest.apply(this)); + const { subscriptions, rooms } = await (this.connected() ? getRoomDpp.apply(this) : getRoomRest.apply(this)); const data = rooms.map(room => ({ room, sub: database.objects('subscriptions').filtered('rid == $0', room._id) })); diff --git a/app/lib/methods/loadMessagesForRoom.js b/app/lib/methods/loadMessagesForRoom.js index 7f724de7d..e53c73e5b 100644 --- a/app/lib/methods/loadMessagesForRoom.js +++ b/app/lib/methods/loadMessagesForRoom.js @@ -38,7 +38,7 @@ export default function loadMessagesForRoom(...args) { const { database: db } = database; return new Promise(async(resolve, reject) => { try { - const data = (await (SDK.driver.ddp + const data = (await (this.connected() ? loadMessagesForRoomDDP.call(this, ...args) : loadMessagesForRoomRest.call(this, ...args))).map(buildMessage); diff --git a/app/lib/methods/loadMissedMessages.js b/app/lib/methods/loadMissedMessages.js index 4bcdd0e36..69ae0f18b 100644 --- a/app/lib/methods/loadMissedMessages.js +++ b/app/lib/methods/loadMissedMessages.js @@ -9,6 +9,8 @@ async function loadMissedMessagesRest({ rid: roomId, lastOpen }) { let lastUpdate; if (lastOpen) { lastUpdate = new Date(lastOpen).toISOString(); + } else { + return []; } const { result } = await SDK.api.get('chat.syncMessages', { roomId, lastUpdate, count: 50 }); return result; @@ -29,7 +31,7 @@ export default function loadMissedMessages(...args) { const { database: db } = database; return new Promise(async(resolve, reject) => { try { - const data = (await (SDK.driver.ddp ? loadMissedMessagesDDP.call(this, ...args) : loadMissedMessagesRest.call(this, ...args))); + const data = (await (this.connected() ? loadMissedMessagesDDP.call(this, ...args) : loadMissedMessagesRest.call(this, ...args))); if (data) { if (data.updated && data.updated.length) { diff --git a/app/lib/methods/readMessages.js b/app/lib/methods/readMessages.js index 4cce4ea7f..7b298d7ce 100644 --- a/app/lib/methods/readMessages.js +++ b/app/lib/methods/readMessages.js @@ -19,7 +19,7 @@ export default async function readMessages(rid) { const ls = new Date(); const { database: db } = database; try { - const data = await (SDK.driver.ddp ? readMessagesDDP.call(this, rid) : readMessagesREST.call(this, rid)); + const data = await (this.connected() ? readMessagesDDP.call(this, rid) : readMessagesREST.call(this, rid)); const [subscription] = db.objects('subscriptions').filtered('rid = $0', rid); db.write(() => { subscription.open = true; diff --git a/app/lib/methods/sendMessage.js b/app/lib/methods/sendMessage.js index aced6738d..a21211128 100644 --- a/app/lib/methods/sendMessage.js +++ b/app/lib/methods/sendMessage.js @@ -31,26 +31,22 @@ export const getMessage = (rid, msg = {}) => { return message; }; -function sendMessageByRest(message) { - const { _id, rid, msg } = message; - return SDK.api.post('chat.sendMessage', { message: { _id, rid, msg } }); +function sendMessageByRest(args) { + return SDK.api.post('chat.sendMessage', { message: args }); } -function sendMessageByDDP(message) { - const { _id, rid, msg } = message; - return SDK.driver.asyncCall('sendMessage', { _id, rid, msg }); +function sendMessageByDDP(...args) { + try { + return SDK.driver.asyncCall('sendMessage', ...args); + } catch (error) { + return sendMessageByRest.call(this, ...args); + } } export async function _sendMessageCall(message) { - try { - const data = await (SDK.driver.ddp ? sendMessageByDDP.call(this, message) : sendMessageByRest.call(this, message)); - return data; - } catch (e) { - database.write(() => { - message.status = messagesStatus.ERROR; - database.create('messages', message, true); - }); - } + const { _id, rid, msg } = message; + const data = await (this.connected() ? sendMessageByDDP.call(this, { _id, rid, msg }) : sendMessageByRest.call(this, { _id, rid, msg })); + return data; } export default async function(rid, msg) { @@ -63,11 +59,17 @@ export default async function(rid, msg) { room.lastMessage = message; }); - const ret = await _sendMessageCall.call(this, message); - // TODO: maybe I have created a bug in the future here <3 - db.write(() => { - db.create('messages', buildMessage({ ...message, ...ret }), true); - }); + try { + const ret = await _sendMessageCall.call(this, message); + db.write(() => { + db.create('messages', buildMessage({ ...message, ...ret }), true); + }); + } catch (e) { + database.write(() => { + message.status = messagesStatus.ERROR; + database.create('messages', message, true); + }); + } } catch (e) { log('sendMessage', e); } diff --git a/app/lib/methods/subscriptions/room.js b/app/lib/methods/subscriptions/room.js index 9a6a23440..c718cdcfb 100644 --- a/app/lib/methods/subscriptions/room.js +++ b/app/lib/methods/subscriptions/room.js @@ -43,7 +43,7 @@ export default function subscribeRoom({ rid, t }) { }, 5000); }; - if (!SDK.driver.ddp && SDK.driver.userId) { + if (!this.connected()) { loop(); } else { SDK.driver.on('logged', () => { diff --git a/app/lib/methods/subscriptions/rooms.js b/app/lib/methods/subscriptions/rooms.js index 752f8deff..e12cdc37b 100644 --- a/app/lib/methods/subscriptions/rooms.js +++ b/app/lib/methods/subscriptions/rooms.js @@ -30,7 +30,7 @@ export default async function subscribeRooms(id) { }, 5000); }; - if (!SDK.driver.ddp && SDK.driver.userId) { + if (!this.connected()) { loop(); } else { SDK.driver.on('logged', () => { @@ -44,13 +44,13 @@ export default async function subscribeRooms(id) { }); SDK.driver.on('disconnected', () => { - if (this._login) { + if (SDK.driver.userId) { loop(); } }); SDK.driver.on('stream-notify-user', protectedFunction((e, ddpMessage) => { - if (ddpMessage.msg === 'added') { + if (!this.ddp || ddpMessage.msg === 'added') { return; } const [type, data] = ddpMessage.fields.args; diff --git a/app/lib/rocketchat.js b/app/lib/rocketchat.js index 5cee9833d..a03230199 100644 --- a/app/lib/rocketchat.js +++ b/app/lib/rocketchat.js @@ -421,6 +421,9 @@ const RocketChat = { log('SDK.connect catch', e); }); }, + connected() { + return SDK.driver.ddp && SDK.driver.ddp._logged; + }, register({ credentials }) { return call('registerUser', credentials); @@ -534,7 +537,7 @@ const RocketChat = { message.status = messagesStatus.TEMP; database.create('messages', message, true); }); - return _sendMessageCall(JSON.parse(JSON.stringify(message))); + return _sendMessageCall.call(this, JSON.parse(JSON.stringify(message))); }, async search({ text, filterUsers = true, filterRooms = true }) { diff --git a/app/sagas/selectServer.js b/app/sagas/selectServer.js index 9a3d50b4e..82f8d94cb 100644 --- a/app/sagas/selectServer.js +++ b/app/sagas/selectServer.js @@ -20,6 +20,7 @@ let LoginView = null; const handleSelectServer = function* handleSelectServer({ server }) { try { yield database.setActiveDB(server); + yield put(connectRequest()); yield call([AsyncStorage, 'setItem'], 'currentServer', server); const token = yield AsyncStorage.getItem(`${ RocketChat.TOKEN_KEY }-${ server }`); if (token) { @@ -36,7 +37,6 @@ const handleSelectServer = function* handleSelectServer({ server }) { return result; }, {}))); - yield put(connectRequest()); yield put(selectServerSuccess(server)); } catch (e) { log('handleSelectServer', e); diff --git a/app/views/RoomView/index.js b/app/views/RoomView/index.js index 4766d8b34..f893df2aa 100644 --- a/app/views/RoomView/index.js +++ b/app/views/RoomView/index.js @@ -116,7 +116,7 @@ export default class RoomView extends LoggedView { const { room, loaded, joined, end } = this.state; - const { showActions } = this.props; + const { showActions, showErrorActions } = this.props; if (room.ro !== nextState.room.ro) { return true; @@ -130,6 +130,8 @@ export default class RoomView extends LoggedView { return true; } else if (showActions !== nextProps.showActions) { return true; + } else if (showErrorActions !== nextProps.showErrorActions) { + return true; } return false; } diff --git a/package-lock.json b/package-lock.json index b8ea1670c..2812dedd5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2578,9 +2578,9 @@ "integrity": "sha512-FWR7QB7EqBRq1s9BMk0ccOSOuRLfVEWYpHQYpFPaXtCoqN6dJx2ttdsdQbUxLLnAlKpYeVjveGGhQ3583TTa7g==" }, "@types/node": { - "version": "9.6.35", - "resolved": "https://registry.npmjs.org/@types/node/-/node-9.6.35.tgz", - "integrity": "sha512-h5zvHS8wXHGa+Gcqs9K8vqCgOtqjr0+NqG/DDJmQIX1wpR9HivAfgV8bjcD3mGM4bPfQw5Aneb2Pn8355L83jA==" + "version": "9.6.36", + "resolved": "https://registry.npmjs.org/@types/node/-/node-9.6.36.tgz", + "integrity": "sha512-Fbw+AdRLL01vv7Rk7bYaNPecqmKoinJHGbpKnDpbUZmUj/0vj3nLqPQ4CNBzr3q2zso6Cq/4jHoCAdH78fvJrw==" }, "@types/react": { "version": "16.4.6",