Parse Link header in issue model.

[miketaylr]

We don't use it yet, but this allows us to keep doing pagination the way we do--and will allow us to use Link URIs to request new pages.

r? @karlcow because it relates to HTTP-ish things.

[karlcow]

So I ran the

→ git checkout remotes/upstream/370/2
[…]
HEAD is now at 2188dd6... Issue #370 - Remove unused getLastPageNumber method.
→ python run.py
 * Running on http://127.0.0.1:5000/
 * Restarting with reloader
127.0.0.1 - - [13/Nov/2014 14:38:26] "GET / HTTP/1.1" 200 -

Everything is working, but as you said because it's not used. It's difficult to know what it does in terms of requests.

Looking at jqXHR and can't find the reference for this.
I find http://api.jquery.com/jQuery.ajax/#jqXHR
ok. Trying on the console:

$.ajax({url: '/api/issues?page=1'})

receiving a JSON object which has 16 entries, and still no pagination links. I wonder when the pagination is starting. Any idea?

[miketaylr]

I wonder when the pagination is starting. Any idea?

GitHub only sends pagination links for resources which need paginating--I think by default that begins after 30 issues. (I had assumed they would always send it.)

You should be able to request the issues of a popular repo like https://api.github.com/repos/jquery/jquery/issues.

[miketaylr]

If the request goes through webcompat.com. Would not it be easier to send directly this information from Webcompat.com? Just asking.

That's another way we can do it, but would require rewriting how we handle responses from the server for Issue Collections. Right now we assume we get back an array of Issue objects and render. Maybe I'll end up re-writing it when I start using the pagination links.

[miketaylr]

Since there's no strong objections, I'm going to merge this so I can continue working towards the other issues. Thanks for the suggestion to parse the Link header via Python and send the parsed object in the response--I might end up doing that.

[miketaylr]

(If I'm abusing reviews, let me know--I think this particular case is OK because it's sort of in flux and doesn't change any existing functionality).

[karlcow]

About the piece of code.

I was asking what it was doing, but I didn't get the answer ;) Maybe I'm too polite in my questions.
Running manually I can see it create yet another request, which seems to have been done already before. Aka

• a 1st request for displaying the issue
• a 2nd request for parsing the link

but these same to be the same two requests. I might be missing something. So the fact to see that it should be done on the python side is that if there are two requests already it's better to grasp the information already. We want less dependencies on JS, not more. Specifically for things which are related to requests and API.

As for the execution of the code.

I'm a bit wary when we introduce codes without a way to test it. Be a test case or at least demo code for using it.

About the review process

hehe. kind of abusing.

• Feeling: I'm being asked to review. I ask question. I don't get answers and the PR is both merged and closed without the discussions being finished.
• Practical: Is there a purpose that it should be here now, more than sitting in a branch on when the code will be used? As I said it could be perfectly valid for example if there was a test case testing this piece of code.

[miketaylr]

I was asking what it was doing, but I didn't get the answer.

Sorry, I did not understand that from:

If the request goes through webcompat.com. Would not it be easier to send directly this information from Webcompat.com? Just asking.

It's OK to be less polite and be more direct in your questions. No hard feelings.

As to the actual question,

a 2nd request for parsing the link

If it's requesting again, that's a bug. The code that parses the Link header gets the Link header from the original jqXHR object (a jquery-fied XHR object) from the request to the /issues endpoint. Double checking just now there's only a single request to /issues?page=1.

I'm a bit wary when we introduce codes without a way to test it.

Tests are on their way. ^_^

I'm being asked to review. I ask question. I don't get answers and the PR is both merged and closed without the discussions being finished.

Again, sorry about screwing up the review process. It seemed like you were giving me a hint about how you would have done it, I didn't realize it was a question about the code at hand.

Is there a purpose that it should be here now, more than sitting in a branch on when the code will be used?

Well, it is being used internally in the IssueCollection. This PR introduced a parseHeader method, which is used to create the linkHeader property. We used that parsed header /linkHeader in the getPageFromRel method which is used to control pagination requests.

The next step will be to use the linkHeader property externally to set the url property of the IssueListView. Which will be an improvement over current hacks.

[karlcow]

Thanks @miketaylr for taking the time to explain.

It's OK to be less polite and be more direct in your questions. No hard feelings.

That will take efforts. A lot. Because I have strong ideas of what should be done and not done. I'm always afraid to push too hard my own will. I want to preserve the nice part of the project: the collective collaboration.

Back to the core:

The code that parses the Link header gets the Link header from the original jqXHR object (a jquery-fied XHR object) from the request to the /issues endpoint.

Ah cool. I didn't know that.

Well, it is being used internally in the IssueCollection. This PR introduced a parseHeader method, which is used to create the linkHeader property. We used that parsed header /linkHeader in the getPageFromRel method which is used to control pagination requests.

So all these requests are going through webcompat (which is cool). OK let's be more affirmative. I think we should (must? ;) still too polite ) not do it through JS. But let me know if my logic is flawed:

1. The first request brings the issues list through webcompat.com.
2. webcompat.com knows the links before the JS client.
3. webcompat.com sends the JSON and the link headers. We can already generates the links for the page from webcompat.com

Note that the JS code and python code could be associated here for a performance gain. During the first request we can imagine to prefetch the JSON (by webcompat.com) the next pages using the links in the HTTP headers). In that way when the user clicks on next/prev, the request doesn't go to github. It's already here in webcompat.com.

[miketaylr]

I think we should (must? ;) still too polite ) not do it through JS.

Hehe. Your POV doesn't surprise me. I don't agree with it 100%, but I hope you'll keep sharing it.

But let me know if my logic is flawed

Nope, that's correct. The server knows about the Links before the client.

Note that the JS code and python code could be associated here for a performance gain.

👍 This would be a cool enhancement. But let's keep in mind that "next" can mean a lot of things. For issues, there are filter,state,label,sort,directionandsince` parameters that may get changed on the client by the user (not currently, but that's what #371, #372, #373 are about). Similar for search. But it may be that pre-fetching "next" of the current state is always useful.