Refactor: migrated some apps to class views

[jespino]

migrated auth_handlers/dbauth, badges, categories, contact, courses and portal

[oscarcp]

@jespino nice work, but I think you should be moving the permission validations (p.e. get_courses_available_for_user) to the dispatch method of the classes. Don't worry, request can be passed to dispatch() as a parameter to get the request data (it's not documented). We can do it too if you want :)

[jespino]

Why in the dispatch? I'ts a query, probably in other class method called from the get or/and post, but i don't see it in the dispatch.

On the other hand, i'm trying to do a migration to class view without big changes, the idea is make a "Translation" from function views to class views, and then, when all is working, refactor this classes.

[pitbulk]

I agree with you that with View and TemplateView you can reuse and refactor code. In the pull request I don't see any reuse of code (maybe you plan to do it later). The change seems pointless.

I can't find an advantage in switching each function view by a class view. Maybe in other projects with similar views the use of classes has more sense, but I can't find clear examples of reuse in OpenMOOC.

[jespino]

There are some really big views in one function, and in some cases are helper functions thats split the code, but in an unstructured way. I think that are two good reasons to refactor it to classes with smaller, clear and well separated methods, and to include the helper functions only used by one view in the same class.

I don't want to refactor the code in the same commits of the funtion views to class views trasnformation, because this can add a lot of confusion and probably some bugs.

[pitbulk]

If you are planning to refactor some of the views to reduce complexity, then it makes sense to do the migration to CBV. But there is no sense in doing this migration without that refactorizations, and because of that we won't merge this pull request yet.

Please, add new commits to this pull request with the main changes you are planning to do, and then we will be more than happy to merge it. Don't worry about merging conflicts, there are no major developments ongoing right now.

[jespino]

I have pushed some refactoring examples, but it's really hard to refactor without good unit tests.

[oscarcp]

Hi @jespino I've been testing the PR and it has some stuff missing, for example in courses/views.py:321-327 the request and badge variables are not declared. And I've been also unable to access the main course view, I don't know if you have the same problem. I leave a screenshot.

[jespino]

I have no time to finish this refactor, I close this PR.