[Bug 1139127,1139128,1143941,1143945] Community top contributors

[mythmon]

This covers a lot of bugs, and a lot of functionality. I could split this up into multiple smaller PRs, but they would all have to be in a linear order, and I think it would just be more frustrating in the long run.

Some broad architecture notes

• The new pages are at http://localhost:8000/en-US/community/top-contributors/l10n/new and http://localhost:8000/en-US/community/top-contributors/questions/new They are not linked from anywhere just yet.
• They are written as a React component, driven by a controller, embedded in our normal template.
• Most of the new JavaScript is written in JSX and ES6. Babel (the Node tool) covers transpiling these to ES5 (modern-ish JS). The main ES6 features uses are arrow function (foo, bar) => foo + bar, classes, and modules.
• Browserify is used to make the modules work.
• This won't yet work on our lowest support platforms (I'm thinking <IE8). That will come later with some shims and shams and polyfills. This is because Babel and Browserify target ES5 by default.
• Files that export a single React component are named like ComponentName.jsx. They use export default class ComponentName extends React.Component.
• Files that export multiple things or no things are named with lower case.
• Files that end in .browserify.js. Are treated as ES6 and as Browserify entry points for the module system. They and all the files they import will be packaged as a single file, compiled to ES5.

React docs: http://facebook.github.io/react/docs/getting-started.html

Details of the feature in particular:

The main view loads a barebones view which simply renders the React component. It also embeds the needed data into the page via script tags. This avoids and extra round trip. All filtering on the client happens by modifying the querystring parameters, and essentially reloading the page. React is very efficient, and analyzes the before and after of the DOM, and only re-renders the parts that actually changed. It is magic. Both the backend view and the frontend renderer read from the query string, so this is the one source of truth for what data should be shown on the page.

I expect this to be a high touch review. I imagine I will have questions to answer, and that there needs to be more comments in some places. I also expect we will want to change some things about this, or at least do them differently in the future.

r?

[mythmon]

More helpful links

• Moment docs - Used for datetime handling.
• Pikaday docs - The calendar component in the date range picker.
• The UX mockups
• The React announcement of class based components, used extensively in the PR. (The blog post talks about React 0.13 beta, but this has now been released in 0.13 stable, which we are using).
• Better info about ES6

[willkg]

Actually, there is with ElasticUtils--you can do .everything():

https://github.com/mozilla/elasticutils/blob/master/elasticutils/__init__.py#L1566

It does an ES request for the count, then does another ES request to get count number of things.

I don't know if that helps you or not.

Also, "This should be the higher..." should probably be "This should be higher...".

[mythmon]

Ooh, nice. I stole that comment, and the general approach of getting metric objects, and then getting user objects, from some stuff r1cky wrote for the first iteration of this feature.

[mythmon]

I did this for one place, but I think that we still need to use this trick for facets. Let me know if there is something clever I'm missing there too.

[willkg]

Seems like there's a blank kitsune/community/static/js/QuestionsContributors.jsx and a blank kitsune/community/static/js/community.js. Does that sound right?

[mythmon]

QuestionsContributors.jsx is my fault, left over from some refactor. It seems community.js has just always been empty though.

[mythmon]

I wrote some docs about all this jazz.

[willkg]

There's no bower/ directory, so I think these need to be de-bower-ified.

[rehandalal]

+1

[willkg]

The title in the titlebar for http://localhost:8000/en-US/community/top-contributors/l10n/new doesn't match the title on the page:

http://bluesock.org/~willkg/images/link/1427390887.png

[willkg]

Using the filters feels kind of stuttery, but it's possible that's just when it's running on my machine.

Everything else seems to work fine as far as I can tell.

I can't really speak to the React code--I have no idea what that's supposed to look like or how it works, so it's hard to review it.

Mmm... I think I've covered everything I can think of here.

[rehandalal]

thanks for adding this. i totally dropped the ball on adding bower docs!

[mythmon]

I added tests and fixed some of the issues brought up in review. ^

[mythmon]

@willkg I know that I am going to be asked to be able to sort this data by last_contribution_date, but I don't really see a way to do that without getting every single user object instead of paginating just the IDs. This sounds quite slow to me. Do you have any clever ideas?

[willkg]

So, I'm not entirely sure I understand the question. Why can't you do another ES search ordered by last_contribution_date and just retrieve that page of ids?

Generally, SUMO bugs tend to be under-specified, so I'd treat everything as a "first pass", land it and then create new bugs for optimization work or changes in requirements. The theory being that they need to see a prototype to help them figure out what it is they need and spending the least amount of time on the prototype gets it out there faster and "wastes" less time.

Going on that theory, I'd just leave it as is for now and push off additional work to new bugs.

[mythmon]

Huh. What you said here actually makes a lot of sense, so I can imagine being able to do this query without it being very inefficient. I'm not sure what my problem was earlier. Yay for more sets of eyes on a problem.

That being said, I think the idea of landing this and then following up is a good one. I'll do that.

[willkg]

Tests look ok to me.

[mythmon]

@rehandalal Any opinions on the front end of this? If not, I'm inclined to land this so I can get unblocked.

[rehandalal]

@mythmon looks fine to me. I don't see anything glaringly obvious. Took me a bit to wrap my head around some of it.

[mythmon]

@rehandalal I'd be really interested in hearing about what parts were hard to understand. There is a lot of code here, so that is understandable, but I'm curious if we can improve any of it.

I'm going to merge this as is, but I'd love to get feedback on it to improve this.