Bug 1168381 - PR1 - Created a view that unites all releases, changed tables to makes ajax calls, added pagination, added new MethodView, created scripts on migrate_repo, created a new getReleases function

[allan-silva]

No description provided.

[sylvestre]

As you probably saw, travis showed some coding style issues, could you fix those? Thanks

[allan-silva]

Finnaly, after four commits. :)

[sylvestre]

Well done, this is much faster than the previous version! However, clicking on the title of a column should change the order, it seems broken on my system ?!

[allan-silva]

You're right, I forgot this feature!
Soon I will make a new PR. Now I'm finishing the HTML to the Bug 1234832.

The sorting will be made on server side. I think that in future, we can add some relevant filters on this page.

[allan-silva]

Sort feature was added. In the previous version of this page, by default, "ready", "complete" and "SubmitedAt" was added to "order by" expression always, this behavior was maintained, however with my database dump the results seem confusing. If I remove this fields(see releases.py, line 113), the results makes sense (for me).

[sylvestre]

Are you sure it is fixed? I don't see any results when clicking on the columns ?

I see this in the terminal:

INFO:werkzeug:127.0.0.1 - - [26/Jan/2016 12:04:33] "GET /releases/releaseslist?sEcho=14&iColumns=17&sColumns=&iDisplayStart=0&iDisplayLength=10&mDataProp_0=status&mDataProp_1=name&mDataProp_2=description&mDataProp_3=submitter&mDataProp_4=submittedAt&mDataProp_5=branch&mDataProp_6=mozillaRevision&mDataProp_7=mozillaRelbranch&mDataProp_8=commRevision&mDataProp_9=commRelbranch&mDataProp_10=dashboardCheck&mDataProp_11=name&mDataProp_12=partials&mDataProp_13=promptWaitTime&mDataProp_14=comment&mDataProp_15=shippedAt&mDataProp_16=status&sSearch=&bRegex=false&sSearch_0=&bRegex_0=false&bSearchable_0=true&sSearch_1=&bRegex_1=false&bSearchable_1=true&sSearch_2=&bRegex_2=false&bSearchable_2=true&sSearch_3=&bRegex_3=false&bSearchable_3=true&sSearch_4=&bRegex_4=false&bSearchable_4=true&sSearch_5=&bRegex_5=false&bSearchable_5=true&sSearch_6=&bRegex_6=false&bSearchable_6=true&sSearch_7=&bRegex_7=false&bSearchable_7=true&sSearch_8=&bRegex_8=false&bSearchable_8=true&sSearch_9=&bRegex_9=false&bSearchable_9=true&sSearch_10=&bRegex_10=false&bSearchable_10=true&sSearch_11=&bRegex_11=false&bSearchable_11=true&sSearch_12=&bRegex_12=false&bSearchable_12=true&sSearch_13=&bRegex_13=false&bSearchable_13=true&sSearch_14=&bRegex_14=false&bSearchable_14=true&sSearch_15=&bRegex_15=false&bSearchable_15=true&sSearch_16=&bRegex_16=false&bSearchable_16=true&iSortCol_0=6&sSortDir_0=asc&iSortingCols=1&bSortable_0=true&bSortable_1=true&bSortable_2=true&bSortable_3=true&bSortable_4=true&bSortable_5=true&bSortable_6=true&bSortable_7=true&bSortable_8=true&bSortable_9=true&bSortable_10=true&bSortable_11=true&bSortable_12=true&bSortable_13=true&bSortable_14=true&bSortable_15=true&bSortable_16=true&_=1453806146630 HTTP/1.1" 200

By the way, having some JS & Python tests to make sure this feature doesn't regress would be great!
(this patch moved the code coverage from 73 to 72 % https://coveralls.io/github/mozilla/ship-it)

[allan-silva]

The request is right, you tried sort by "Mozilla Revision". Probably I found the problem, now my tests makes sense. Try to test again, please.

[allan-silva]

The code coverage decreases, because, I created new functions and original getReleases() function is utilized by a single request now, I guess....
How can I increase the code coverage ?

[sylvestre]

By writing tests :) Example: https://github.com/mozilla/ship-it/blob/master/kickoff/test/views/test_releases.py

[allan-silva]

Ah, OK!
Soon I will create test for new class.
I need go to work now :(

Take a look the last commit, you is able to sort. If any something else is wrong, I fix all together.

[sylvestre]

This is great, thanks!
@bhearsum As you are a great reviewer, could you review it?
We still need to tests to approve it but this will make release management much easier.

[sylvestre]

Many of this function is duplicate from getReleases. We don't like code duplication, could you factorize this? Thanks

[allan-silva]

You're right, I kept the logic, because, I did not know if any other place (or other system) use getReleases() function. Can I change getReleases() function to use only the new View ?

[sylvestre]

Sure, just add a argument to change it behavior depending on the context

[allan-silva]

Hi, I made some changes, please, take a look, again.

[sylvestre]

very happy with your changes, thanks!
@bhearsum could you review and merge it if you are ok/happy with it? Merci!

[sylvestre]

By the way, don't hesitate to merge all patches into a single commit with a clear description, thanks!

[allan-silva]

After I made the first commit and some new commits to the same PR, how I can merge all commits?

Should I use squash command described in following doc?
https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History#Squashing-Commits

[allan-silva]

I think I got it. Cool... haha

[sylvestre]

I have now permissions ! :) Many thanks! this is great
@rail @bhearsum Could you deploy the update ? It needs database changes (a new view) Merci!