-
Notifications
You must be signed in to change notification settings - Fork 405
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
[Migration] Upgrade to new version of Instagram APIs #1250
Conversation
4c224fd
to
b007922
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work Vineet! 👍 I dropped some comments here and there. Also this branch has diverged from master, so it can't be merged without conflicts. Can you maybe rebase with master in order to fix this?
} | ||
session = new Client.Session(device, storage); | ||
igClient.state.serialize().then((cookies)=>{ | ||
delete cookies.constants; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the implication of this deletion? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this deletes the version info. So, it always use the version provided by the library.
Suggested here :
https://github.com/dilame/instagram-private-api/blob/master/examples/session.example.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it 👍
main/instagram.js
Outdated
const feed = new Client.Feed.Inbox(session, 10); | ||
feed.all().then(resolve).catch(reject); | ||
exports.startCheckpoint = () => { | ||
//return Client.Web.Challenge.resolve(error, 'email'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did you mean to delete this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Will remove the comment. Kept for a doubt which resolved now. 👍
return new Promise((resolve, reject) => { | ||
Client.Thread.configureText(session, recipients, message).then(resolve).catch(reject); | ||
const directThread = igClient.entity.directThread(recipients); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sendNewChatMessage
function was originally meant for starting new conversations (where there's no threadId/chatId yet). Looking at the instagram private api code, it seems the directThread
function only accepts threadIds and not usernames. Are we certain that this function still works for starting new conversations? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I have tested the APIs. If we pass Array of recipients then it will start a new conversation. else if its an string of threadID, it will send to thread.
instagram.sendNewChatMessage(data.message, data.users)
main/main.js
Outdated
@@ -234,8 +231,8 @@ electron.ipcMain.on('login', (evt, data) => { | |||
|
|||
const getErrorMsg = (error) => { | |||
let message = 'An unknown error occurred.'; | |||
if (error.message) { | |||
message = error.message; | |||
if (error.text) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this if (...)
check is redundant, no? 🤔
instagram.getUser(session, userId).then((user) => { | ||
mainWindow.webContents.send('getDisplayPictureUrl', { userId: userId, url: user._params.profilePicUrl }); | ||
instagram.getUser(userId).then((user) => { | ||
mainWindow.webContents.send('getDisplayPictureUrl', { userId: userId, url: user.profile_pic_url }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this profile_pic also possible available in the authenticatedUser
object? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, should be. We can remove this method, Agree. checking 💯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just checked, This profile_pic is fetched on the base of userId
which is rendered in renderMessageReactions
. So, authenticatedUser only consists of the loggedIn user's info.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah! that's right
main/utils.js
Outdated
if (canUseFileStorage()) { | ||
const storagePath = buildAndGetStoragePath(); | ||
const filePath = `${username}.json`; | ||
fs.writeFileSync(storagePath+'/'+filePath, JSON.stringify(cookies)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fs.writeFileSync(storagePath+'/'+filePath, JSON.stringify(cookies)); | |
fs.writeFileSync(`${storagePath}/${filePath}`, JSON.stringify(cookies)); |
storage = new Client.CookieMemoryStorage(); | ||
} | ||
|
||
if (filePath) storage = fs.readFileSync(`${buildAndGetStoragePath()}/${filePath}`, 'utf8'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we supposed to JSON.parse(...)
this data before sending it back?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, based on the new behaviour of this function, I think we should rename it to getCookieStorage geStoredCookie
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JSON is deserializing in,
instagram.js Line 19
function loadCookieInSession () {
return new Promise((resolve, reject) => {
const savedCookie = utils.getStoredCookie();
if (savedCookie) {
igClient.state.deserialize(savedCookie).then(() => {
resolve(true);
}).catch(reject);
} else {
reject('No session saved');
}
});
}
So, we might need not to parse.
browser/js/renderers.js
Outdated
let post = message.media_share; | ||
let img = ''; | ||
if (post.image_versions2) { | ||
img = dom(`<img style='max-height:400px;' src="${post.image_versions2.candidates[0].url}">`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this style='max-height:400px;'
be handled in css instead? 🤔 Same for the other cases in this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Sounds Perfect. 💯
@@ -333,31 +338,25 @@ function renderChatHeader (chat_) { | |||
chatTitleContainer.appendChild(b); | |||
} | |||
|
|||
function renderChat (chat_, loadingMore) { | |||
function renderChat (chat_) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are we dropping the loadingMore
flag here? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, In the first API call for fetching chat-lists we are getting 10 messages for each chat in the List. Previously, We used to get One message in each chat, and we were loading more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Provide your suggestion should we keep loading ? 🔑
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, I see what you mean. I'd say we don't need it anymore indeed then.
main/instagram.js
Outdated
Client.Thread.getById(session, chatId).then((thread)=>{ | ||
thread.hide().then(resolve); | ||
chatsFeed.items().then((chats)=>{ | ||
const thread = chats.find(chat => chat.thread_id === chatId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trying to optimize this O(n) of extra computation. This will delay the chat to render every time we de-queue a sent message in front-end, which we merged in previous PR.
browser/js/funcs.js
Outdated
queue = queue.filter((messageQueued) => messageQueued.trackerKey !== trackerKey); | ||
const la = queue.length; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this value needed for anything? Also the one above?
optimized-get-chat
thank you for the work Vineet! ❤️ |
can I add my thanks here as well? <3 @vineetvk01 |
Tasks:
- Updating to the latest "instagram-private-api" repository (migration)
- Updating Two-Factor-Authentication (fix)
After entering the code in the OTP window, still login was not happening
- Send Image in chat (fix)
We were not able to send .jpg images in the chat previously. Fixed the issue
- Chat gets stuck while loading old messages when reaches last message (fix)
When we go to the very top of the chat, when there is no more old messages, the chat seems to be stuck there sometimes
- Limiting image height in chat view (update)
The image renders with the original size, which captures a lot of space of the chat screen, limited the height to 300px for better chat view
Closes: #1245 , #1244, #1243 , #1241