Closed
Bug 954175
Opened 10 years ago
Closed 10 years ago
Fetch the tweets with @ mentions and tracked keywords when connecting a twitter account.
Categories
(Chat Core :: Twitter, defect)
Chat Core
Twitter
Tracking
(Not tracked)
RESOLVED
FIXED
0.3b1
People
(Reporter: florian, Assigned: florian)
References
Details
(Whiteboard: [0.3-wanted])
Attachments
(1 file, 2 obsolete files)
7.32 KB,
patch
|
clokep
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 741 at 2011-03-28 20:19:00 UTC *** Currently when (re)connecting a twitter account, the timeline that appears contains the history of tweets from the user and the followed users, but contains neither the @ mentions from not-followed users nor the messages containing tracked keywords. I think we need to fetch that data using the REST API. Possibly making several separate requests and merging (= remove duplicates + sort) the results before displaying them.
Assignee | ||
Updated•10 years ago
|
Whiteboard: [0.3-wanted]
Comment 1•10 years ago
|
||
*** Original post on bio 741 at 2011-03-29 01:28:37 UTC *** Not that we'll already get removing duplicates for free from the code that checks if an id is known already. http://lxr.instantbird.org/instantbird/source/purple/purplexpcom/src/twitter.js#218 How is the data for the "tracked keywords" actually fetched? Is that just a Twitter search? (If so, shouldn't we get the search lists from the Twitter site instead of storing them in our preferences?)
Comment 2•10 years ago
|
||
*** Original post on bio 741 as attmnt 575 at 2011-03-29 01:41:00 UTC *** This patch works but does not handle the "tracked keyword" part.
Attachment #8352317 -
Flags: review?(florian)
Updated•10 years ago
|
Assignee: nobody → clokep
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•10 years ago
|
||
Comment on attachment 8352317 [details] [diff] [review] WIP *** Original change on bio 741 attmnt 575 at 2011-03-30 13:48:21 UTC *** >diff --git a/purple/purplexpcom/src/twitter.js b/purple/purplexpcom/src/twitter.js >+ getTimelines: function() { > this.signAndSend("1/statuses/home_timeline.json", null, null, > this.onTimelineReceived, this.onError, this); >+ this.signAndSend("1/statuses/mentions.json", null, null, >+ this.onTimelineReceived, this.onError, this); >+ // Third one should get tracked keywords, is this just a search? >+ //this.signAndSend("1/statuses/home_timeline.json", null, null, >+ // this.onTimelineReceived, this.onError, this); Apparently this is "just a search" (with commas replaced by the OR keyword). The URL is: http://search.twitter.com/search.json?q=<URL encoded query> This seems to be a plain HTTP request, not an OAuth authenticated one. I'm a bit concerned by the effect of this code if some of these requests fail and some succeed. this.onError will disconnect the account. Shouldn't all the other pending requests be canceled then? Or should we just ignore the failed requests and just use the results we got from the others? > }, > > get timeline() this._timeline || (this._timeline = new Conversation(this)), >@@ -230,10 +240,25 @@ > // If the conversation already exists, notify it we are back online. > if (this._timeline) > this._timeline.notifyObservers(this._timeline, "update-buddy-status"); >- this.displayMessages(JSON.parse(aData).reverse()); >+ // Parse the return data >+ this._timelineBuffer = this._timelineBuffer.concat(JSON.parse(aData)); >+ >+ // If we've received everything, display it >+ if (++this._timelinesReceived == 2) { I don't really like this hardcoded value. If we want to be able to cancel pending requests, we will need to store the return value of signAndSend/doXHRequest, so maybe we can replace this test by checking that the store of pending requests is empty? >+ this._timelineBuffer.sort(this.sortByDate); >+ Components.utils.reportError(this._timelineBuffer); >+ >+ this.displayMessages(this._timelineBuffer); >+ // Reset in case we get disconnected >+ this._timelinesReceived = 0; I think you just wanted to delete this._timelinesReceived so that it falls back to the value in the prototype again. >+ this._timelineBuffer = []; Could do the same here (if you initialized it in the prototype instead of in the account contructor). You don't have to worry about the Array of the prototype being modified as Array.concat returns a new array.
Attachment #8352317 -
Flags: review?(florian) → review-
Comment 4•10 years ago
|
||
*** Original post on bio 741 as attmnt 588 at 2011-04-15 01:24:00 UTC *** This doesn't have as much of a hard coded number, but still kind of does. Let me know what you think of this, otherwise we'll have to discuss another way of doing this.
Attachment #8352330 -
Flags: review?(florian)
Comment 5•10 years ago
|
||
Comment on attachment 8352317 [details] [diff] [review] WIP *** Original change on bio 741 attmnt 575 at 2011-04-15 01:24:54 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352317 -
Attachment is obsolete: true
Assignee | ||
Comment 6•10 years ago
|
||
*** Original post on bio 741 as attmnt 595 at 2011-04-21 20:19:00 UTC *** This addresses the issue we discussed on IRC of being able to canceling the pending requests if the user clicks on "disconnect".
Attachment #8352337 -
Flags: review?(clokep)
Assignee | ||
Comment 7•10 years ago
|
||
Comment on attachment 8352330 [details] [diff] [review] v1.0 *** Original change on bio 741 attmnt 588 at 2011-04-21 20:19:06 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352330 -
Attachment is obsolete: true
Attachment #8352330 -
Flags: review?(florian)
Comment 8•10 years ago
|
||
Comment on attachment 8352337 [details] [diff] [review] Patch v3 *** Original change on bio 741 attmnt 595 at 2011-04-21 20:26:18 UTC *** Pretty confusing, but I think it's the best we can do for now. Thanks for finishing this up for me!
Attachment #8352337 -
Flags: review?(clokep) → review+
Assignee | ||
Comment 9•10 years ago
|
||
*** Original post on bio 741 at 2011-04-21 21:38:11 UTC *** https://hg.instantbird.org/instantbird/rev/14c07721ccdd
Assignee: clokep → florian
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 0.3b1
Updated•10 years ago
|
Component: General → Twitter
You need to log in
before you can comment on or make changes to this bug.
Description
•