Conversation
…ket.Chat and Livechat.
# Conflicts: # packages/rocketchat-livechat/server/lib/Livechat.js # packages/rocketchat-livechat/server/methods/getInitialData.js
|
RocketChat / Rocket.Chat Code Issues 1,805 Pull requests 179 Projects 9 Pulse Jump to bottom Xarkam opened this issue Expected behavior: 👍 TwizzyDizzy commented 1 day ago • edited 1 day ago 👍 1 day ago maih43538 👍 maih43538 👍 maih43538 👍 1 👍 maih43538 👍 maih43538 👍 maih43538 👍 Comment You’re not receiving notifications from this thread. Desktop version |
|
Hi @maih43538, I didn't understand why you mentioned the #10552 in this PR. |
|
when this will be merge to master? :) |
| livechatUrl = `${ livechatUrl }&${ query }`; | ||
| } | ||
|
|
||
| return Meteor.absoluteUrl().replace(/\/$/, '') + livechatUrl; |
There was a problem hiding this comment.
you can change this to Meteor.absoluteUrl(livechatUrl)
| }; | ||
|
|
||
| Template.messageAttachment.helpers({ | ||
| fixLivechatUpload, |
There was a problem hiding this comment.
pls fix the indentation (change the spaces by a tab)
| </div> | ||
| <div class="buttons"> | ||
| <svg class="send-button" aria-label="{{_ "Send"}}" viewBox="0 0 1792 1792" xmlns="http://www.w3.org/2000/svg"><path d="M1764 11q33 24 27 64l-256 1536q-5 29-32 45-14 8-31 8-11 0-24-5l-453-185-242 295q-18 23-49 23-13 0-22-4-19-7-30.5-23.5t-11.5-36.5v-349l864-1059-1069 925-395-162q-37-14-40-55-2-40 32-59l1664-960q15-9 32-9 20 0 36 11z"/></svg> | ||
| {{#if fileUploadEnabled}} |
There was a problem hiding this comment.
please add a logic to hide the button if the textarea is not empty.
and what about changing the position of the button to above the textarea? like this:
| @@ -1,6 +1,7 @@ | |||
| /* globals Livechat, LivechatVideoCall, MsgTyping */ | |||
| /* globals Livechat, LivechatFileUpload, LivechatVideoCall, MsgTyping */ | |||
There was a problem hiding this comment.
looks like you're not using LivechatFileUpload on this file
| enableQuery: { _id: 'Jitsi_Enabled', value: true } | ||
| }); | ||
|
|
||
| RocketChat.settings.add('Livechat_fileupload_enabled', false, { |
There was a problem hiding this comment.
I guess this can be enabled by default
| closeOnCancel: true, | ||
| closeOnConfirm: true | ||
| }, (isConfirm) => { | ||
| if (isConfirm) { |
There was a problem hiding this comment.
we should try writing more plain functions, it helps readability and maintainability. this one could be rewrite like this:
if (!isConfirm) {
return;
}
const roomId = visitor.getRoom(true);
if (visitor.getId()) {
return sendFileMessage(file.file, roomId);
}
const guest = {
token: visitor.getToken()
};
if (Livechat.department) {
guest.department = Livechat.department;
}
Meteor.call('livechat:registerGuest', guest, (error, result) => {
if (error) {
return showError(error.reason);
}
visitor.setId(result.userId);
sendFileMessage(file.file, roomId);
});|
|
||
| const match = /^\/([^\/]+)\/(.*)/.exec(req.url); | ||
|
|
||
| if (match[1]) { |
There was a problem hiding this comment.
same as before.. please rewrite this to be more plain.. also rewrite in a way that the success flow is not inside an if statement.
| } | ||
|
|
||
| fileUpload = function(file) { | ||
| Meteor.call('livechat:checkTypeFileUpload', file.file.type, (error, result) => { |
There was a problem hiding this comment.
IMO both checks (type and size) should be done by the same method. so instead of calling multiple methods for multiples validations, I would rather then call a single livechat:validateFileUpload and that method would than perform all validations needed.
| return getUploadPreview(file, function(file, preview) { | ||
| let text = ''; | ||
| if (file.type === 'audio') { | ||
| text = `\ |
There was a problem hiding this comment.
any reason to add this line breaks?
| } | ||
|
|
||
| const room = RocketChat.models.Rooms.findOneById(file.rid); | ||
| return room && room.t === 'l' && file && file.visitorToken && room.v && room.v.token === file.visitorToken; |
There was a problem hiding this comment.
you don't need to check file && file.visitorToken here again, you checked them before already
|
Hi @sampaiodiego! |
# Conflicts: # packages/rocketchat-livechat/server/lib/Livechat.js # packages/rocketchat-livechat/server/methods/getInitialData.js
|
Hi guys, I was trying to run this branch on OSX High Sierra 10.13.3 with npm 6.1.0 and meteor 1.6, I have encountered a very strange bug with live chat. The strange thing is it does not always occur. Here are the steps to reproduce:
Step 1 seems to fix the issue but only for that time, if I build again then the error re-appears. I wonder if has anyone faced the same issues or any idea how to fix it? Many thanks. |
|
I had the same issue, and it seemed that putting "readable-stream": "^2.3.3" into packages/rocketchat-livechat/.app/package.json make it work |
# Conflicts: # packages/rocketchat-livechat/.app/package.json
|
@vodkhang, about your comments:
|
|
I still think your string is not in the correct location? Should the string be inside livechat i18n file? |
|
The display is still wrong when I have done meteor reset. The livechat app seems to be independent so I think you need to copy the i18n to inside |
| }; | ||
|
|
||
| Template.messageAttachment.helpers({ | ||
| fixLivechatUpload, |
There was a problem hiding this comment.
what about change this to a cookie approach just like rocket.chat client does?
| } | ||
|
|
||
| let { rc_uid, rc_token } = query; | ||
| const { rc_room_type } = query; |
There was a problem hiding this comment.
this will need to change if we change to cookies
| const isAuthorizedByCookies = rc_uid && rc_token && RocketChat.models.Users.findOneByIdAndLoginToken(rc_uid, rc_token); | ||
| const isAuthorizedByHeaders = headers['x-user-id'] && headers['x-auth-token'] && RocketChat.models.Users.findOneByIdAndLoginToken(headers['x-user-id'], headers['x-auth-token']); | ||
| return isAuthorizedByCookies || isAuthorizedByHeaders; | ||
| const isAuthorizedByQuery = rc_room_type && RocketChat.roomTypes.getConfig(rc_room_type).allowFileUpload(query); |
There was a problem hiding this comment.
allowFileUpload is not the best name to use here, since it is not allowing upload but the file access.. so I think canAccessUploadedFile or canAccessFile would be better.
| */ | ||
| allowFileUpload(allowData) { | ||
| const { rc_uid, rc_token } = allowData; | ||
| return rc_uid && rc_token && RocketChat.models.Users.findOneByIdAndLoginToken(rc_uid, rc_token); |
There was a problem hiding this comment.
since this is being done already previously, I think this default function can return false only.
| {{{body}}} | ||
| {{#if hasAttachments}} | ||
| {{#each attachments}} | ||
| {{injectIndex . @index}} {{> messageAttachment}} |
There was a problem hiding this comment.
is this index from injectIndex being used somewhere? or what is its purpose?
There was a problem hiding this comment.
My bad.. I was probably thinking of something like ReactJS approach.
Fixed.
| </span> | ||
| <div class="body" dir="auto"> | ||
| {{{body}}} | ||
| {{#if hasAttachments}} |
There was a problem hiding this comment.
I think this {{#if ... }} can be removed.
instead it should always show {{{body}}} and then have the {{#each attachments}}.. this will allow it showing replies from rocket.chat (somewhat).
There was a problem hiding this comment.
Fixed, but we will need to thing about improving markdown on Livechat, I guess.. =)
| } | ||
|
|
||
| canAccessUploadedFile(accessData) { | ||
| const { rc_token, rc_rid } = accessData; |
There was a problem hiding this comment.
you can move this to function definition, like:
canAccessUploadedFile({ rc_token, rc_rid } = {}) {
// ...
}| connected = connectionStatus.connected; | ||
| document.cookie = `rc_rid=${ visitor.getRoom() }; path=/`; | ||
| document.cookie = `rc_token=${ visitor.getToken() }; path=/`; | ||
| document.cookie = `rc_room_type=${ 'l' }; path=/`; |
There was a problem hiding this comment.
you don't need to use interpolation for fixed values:
document.cookie = `rc_room_type=l; path=/`;| } | ||
|
|
||
| const user = Meteor.user(); | ||
| const user = (file.userId) ? Meteor.user() : null; |
There was a problem hiding this comment.
you don't need these parenthesis:
const user = file.userId ? Meteor.user() : null;| return false; | ||
| } | ||
|
|
||
| const language = (user) ? user.language : 'en'; |
There was a problem hiding this comment.
you don't need these parenthesis:
const language = user ? user.language : 'en';
|
@sampaiodiego, I just submitted the improvements on codebase. |
8 participants
Closes #4124
Closes #3686
Closes #6516
Closes #5449
This PR adds a important feature allowing to share files between Rocket.Chat and Livechat Widget.