Added separate ToS page (bug 1209226)

[Nolski]

Work in Progress:

Having issues with tests due to the fact that the user is populated somewhere in the unit tests which automatically causes the request to forward onto the next step (throwing assertion errors). Can't seem to figure out how to populate that request.user object.

[kumar303]

it looks like the only thing different here between the existing agreement page is the title, right? Can you maybe refactor them both to share a common base so there isn't any repeated content?

[kumar303]

r- because of missing api_key_agreement tests and also too much copy/paste of existing code

[Nolski]

I stubbed out the view for showing the API key but I was going to leave that for a separate PR. Everything else should be resolve aside from the one comment that I replied to.

[kumar303]

this template isn't used anymore so it can be deleted

[Nolski]

What do you mean? All sorts of templates extend from this.

[kumar303]

I meant apps/devhub/templates/devhub/api/agreement.html - where is this used?

[Nolski]

Ah right, so here is where I don't understand the UX of the current flow. Here is what happens in our hypothetical flow.

1. User logs into AMO
2. User clicks nav link to generate API key
3. User is redirected to this page
   
4. User accepts and is sent to actual API page

Now the issue here is that the page shown has nothing to do with API keys. it has a side bar that has a whole bunch of steps, none of which mean anything to someone trying to reach the API page. I must have missed this back in my last commit but this is why I suggested having a separate page for accepting the license.

[kumar303]

thanks, this is fixed now

[kumar303]

r- for some fixups needed to the test logic. Let me know if you get stuck or have questions.

[kumar303]

this still looks like unnecessary copy/paste to me. Why not just add the template as a parameter to submit()?

def api_key_agreement(request):
    return submit(request, next_step=..., 
                  template='devhub/api/agreement.html')

[kumar303]

Can't you just take off the follow argument and test it more directly?

response = self.client.get(reverse('devhub.submit.1'))
self.assertRedirects(response, reverse('devhub.submit.2'))

[kumar303]

The redirect chain test looks a little fragile so I offered a solution for that. Otherwise, r+ on the latest changes. Thanks!