[tutorial] Explanation of code in webcompat.com

[karlcow]

@cch5ng started to ask question about the code. I will use this space to reply to the questions which might be useful for others.

Extract from #1628 (comment)

@karlcow this is the start to my questions about views.py (for /issues/new route) but eventually I'll cover the entire views.py. I might include some questions I had when trying to address a different issue later.

for https://github.com/webcompat/webcompat.com/blob/master/webcompat/views.py#L179
should set() be updated to frozenset()? I had to look up issubset() and saw a deprecation notice in the docs. https://docs.python.org/2/library/sets.html ... but am not totally clear about it

https://github.com/webcompat/webcompat.com/blob/master/config/__init__.py#L49
(this is something I looked at for a different issue) why do you use a namedtuple here? is it just to be more descriptive?

for the homepage view (/), I'm trying to understand the flow for how the triage issues list gets generated. in the source it seems to be coming from a backbone model but I'm not clear where in the code the request to the github api gets made. could I get a pointer to which source files I should be looking at?

in the future is it OK for me to include both you and MikeT as JS reviewers? I know both of you are very busy but your review concerns are a bit different and I think I learn from both forms.

[karlcow]

sets vs set/frozenset in views.py

Usually better to use a reference with a commit view so it doesn't get lost in the future.

webcompat.com/webcompat/views.py

Line 179 in eb8b2ac

must_parameters = set(['url', 'problem_category', 'description',

https://docs.python.org/2/library/sets.html

for https://github.com/webcompat/webcompat.com/blob/master/webcompat/views.py#L179
should set() be updated to frozenset()? I had to look up issubset() and saw a deprecation notice in the docs. https://docs.python.org/2/library/sets.html ... but am not totally clear about it

The module sets which has Set() is deprecated indeed, but we are not using that. We are using the built-in set(). It became a standard type. So we are good for this.

[karlcow]

namedtuple in config for labels

webcompat.com/config/__init__.py

Lines 46 to 62 in c613a78

	# Status categories used in the project
	# 'new', 'needsdiagnosis', 'needscontact', 'contactready' , 'sitewait', 'close'
	# Creating the model
	Category = namedtuple('Category', ['name', 'dataAttribute', 'label'])
	CATEGORIES = []
	cat_labels = [('needstriage', 'needstriage', 'Needs Triage'),
	('needsDiagnosis', 'needsdiagnosis', 'Needs Diagnosis'),
	('needsContact', 'needscontact', 'Needs Contact'),
	('ready', 'contactready', 'Ready for Outreach'),
	('sitewait', 'sitewait', 'Site Contacted'),
	('close', 'closed', 'Closed')]
	# populating the categories for issues status
	for cat_label in cat_labels:
	CATEGORIES.append(Category(name=cat_label[0],
	dataAttribute=cat_label[1],
	label=cat_label[2]))

https://github.com/webcompat/webcompat.com/blob/master/config/__init__.py#L49
(this is something I looked at for a different issue) why do you use a namedtuple here? is it just to be more descriptive?

Yes just to be more descriptive and easier to manipulate. We use them in templates. I did that initially to help the passing of arguments to @magsout This code will change soon when we switch to milestones (Issue #886 )

On GitHub, Blame is a good tool to find out the reason behind a change.

Though in this specific case it doesn't help that much because we moved the config to a new file and it destroyed the history. I wish there was a way to link history across file moves (but that's another topic.)

So Let's see, we can probably search in the commits and issues. We had a discussion about this in the past
#801 (diff)

@miketaylr was not very fond of it. He found it confusing. We could switch to something else. As I said this code will change anyway very soon. I'm happy to kill them with fire if it confuses more people. 🔥

[karlcow]

Homepage list of issues

for the homepage view (/), I'm trying to understand the flow for how the triage issues list gets generated. in the source it seems to be coming from a backbone model but I'm not clear where in the code the request to the github api gets made. could I get a pointer to which source files I should be looking at?

The issues are populated into the section js-lastIssue and this is the code sent by the server.

    <div id="js-lastIssue">
      <script type="text/template" id="needstriage-tmpl">
        <% if (issues.length) { %>
          <% _.each(issues, function(issue) { %>
            <article class="wc-IssueList wc-IssueList--<%= issue.stateClass %> wc-IssueList--large js-IssueList">
  <div class="wc-IssueList-section">
    <header class="wc-IssueList-header">
      <a class="wc-Link" href="/issues/<%= issue.number %>">
        <span class="wc-IssueList-count">Issue <%= issue.number %></span>: <%- issue.title %>
      </a>
    </header>
    <p>
      <span class="wc-IssueList-metadata">Opened: <time><%= issue.createdAt %></time></span>
      <span class="wc-IssueList-metadata">Comments: <%= issue.commentNumber %></span>
    </p>
  </div>
  <div class="wc-IssueList-section">
    <span class="wc-IssueList-label js-Issue-label">
    <% _.each(issue.labels, function(label) { %>
      <a href="/issues?q=label:<%= label.remoteName %>"
         class="wc-Labels" data-remotename="<%= label.remoteName %>"
         title="Labels : <%= label.name %>"><%= label.name %></a>
    <% }); %></span>
  </div>
</article>
          <% }); %>
        <% } else { %>
        <p>Nothing here currently, cool!</p>
      <% } %>
      </script>
      </div>

The HTTP request is made by the JavaScript as an XHR request asking for:
http://webcompat.com/api/issues/category/needstriage
This returns a JSON file.

Which means we hit this route in the code: @api.route('/issues/category/<issue_category>')

webcompat.com/webcompat/api/endpoints.py

Lines 107 to 145 in eb8b2ac

	@api.route('/issues/category/<issue_category>')
	@mockable_response
	def get_issue_category(issue_category):
	'''Return all issues for a specific category.
	issue_category can be of N types:
	* needstriage
	* closed
	* contactready
	* needscontact
	* needsdiagnosis
	* sitewait
	'''
	category_list = ['contactready', 'needscontact',
	'needsdiagnosis', 'needstriage', 'sitewait']
	issues_path = 'repos/{0}'.format(ISSUES_PATH)
	params = request.args.copy()
	if issue_category in category_list:
	# add "status-" before the filter param to match the naming scheme
	# of the repo labels.
	params.add('labels', 'status-' + issue_category)
	# Turns out the GitHub API considers &labels=x&labels=y an OR query
	# &labels=x,y is an AND query. Join the labels with a comma
	params['labels'] = ','.join(params.getlist('labels'))
	return api_request('get', issues_path, params=params)
	elif issue_category == 'closed':
	params['state'] = 'closed'
	return api_request('get', issues_path, params=params)
	# Note that 'needstriage' here is primarily used on the homepage.
	# For paginated results on the /issues page,
	# see /issues/search/needstriage.
	# We abort with 301 here because the new endpoint has
	# been replaced with needstriage.
	elif issue_category == 'new':
	abort(301)
	else:
	# The path doesn’t exist. 404 Not Found.
	abort(404)

which ends up doing:

webcompat.com/webcompat/api/endpoints.py

Line 132 in eb8b2ac

return api_request('get', issues_path, params=params)

 return api_request('get', issues_path, params=params)

aka

webcompat.com/webcompat/helpers.py

Lines 450 to 471 in eb8b2ac

	def api_request(method, path, params=None, data=None):
	'''Helper to abstract talking to the GitHub API.
	This method handles both logged-in and proxied requests.
	This returns a tuple for the convenience of being able to do:
	`return api_request('get', path, params=params)` directly from a view
	function. Flask will turn a tuple of the format
	(content, status_code, response_headers) into a Response object.
	'''
	request_headers = get_request_headers(g.request_headers)
	if g.user:
	request_method = github.raw_request
	else:
	request_method = proxy_request
	resource = request_method(method, path, headers=request_headers,
	params=params, data=data)
	if resource.status_code != 404:
	return (resource.content, resource.status_code,
	get_response_headers(resource))
	else:
	abort(404)

Two cases at this moment, depending if you are logged in or not.

---

Little tip here. If you do not remember what are the methods you can call on a function, you can use print(dir(function_name)) it will display all the method associated to a specific name.

---

so for example if you do that on resource
aka print(dir(resource)) you get in the terminal when loading the home page.

['__attrs__', '__bool__', '__class__', '__delattr__', '__dict__', '__doc__', '__format__',
 '__getattribute__', '__getstate__', '__hash__', '__init__', '__iter__', '__module__',
 '__new__', '__nonzero__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', 
'__setstate__', '__sizeof__', '__str__', '__subclasshook__', '__weakref__', '_content', 
'_content_consumed', 'apparent_encoding', 'close', 'connection', 'content',
 'cookies', 'elapsed', 'encoding', 'headers', 'history', 'is_permanent_redirect', 
'is_redirect', 'iter_content', 'iter_lines', 'json', 'links', 'ok', 'raise_for_status', 
'raw', 'reason', 'request', 'status_code', 'text', 'url']
127.0.0.1 - - [06/Jul/2017 09:50:18] "GET /api/issues/category/needstriage HTTP/1.1" 304 -

So let's print resource.url in helpers.py in the api_request function. and I get:

https://api.github.com/repos/webcompat/webcompat-tests/issues?labels=status-needstriage
127.0.0.1 - - [06/Jul/2017 09:54:11] "GET /api/issues/category/needstriage HTTP/1.1" 304 -

So this is the URL request that is sent to GitHub. GitHub will reply to webcompat.com which in return will send it back to the user. Basically webcompat.com proxy all requests to GitHub.

[karlcow]

About reviews

in the future is it OK for me to include both you and MikeT as JS reviewers? I know both of you are very busy but your review concerns are a bit different and I think I learn from both forms.

Yes. Perfectly fine. :)

[cch5ng]

@karlcow thanks for your time and answers. the app flow is particularly helpful.

Basically webcompat.com proxy all requests to GitHub.

is that the same as meaning the FE makes a request (like issues) to the BE api which makes a corresponding request to Github's api?

[karlcow]

@cch5ng what are FE and BE?

[zoepage]

@karlcow FE = Front end, BE = Back end

[karlcow]

is that the same as meaning the FE makes a request (like issues) to the BE api which makes a corresponding request to Github's api?

ok let me try to make a graph of what is happening? So it becomes clearer. The separation is not totally Front-end / Back-end. Or more exactly it's usually better to talk in terms of Request/Response because, when you "proxy", a server is at the same time front-end and back end.

[karlcow]

@cch5ng

It is something which looks a bit like this:

@startuml
Browser -> webcompat: HTTP Request 
webcompat -> Browser: HTTP Response (html/css/js/images)
Browser -> "webcompat API": HTTP XHR Request [by JS on client]
"webcompat API" -> "GitHub API": HTTP Request [by Python]
"GitHub API" -> "webcompat API": HTTP Response (json)
"webcompat API" -> Browser: HTTP Response (json) [by Python on server]
@enduml

[zoepage]

Would it be possible to move that to / reference that in our wiki as issues are not meant for documentation purpose? That would be great. Thanks!

[karlcow]

@zoepage Done

(updated)