Skip to content
This repository has been archived by the owner on Dec 31, 2021. It is now read-only.

Django 1.5 - DO NOT MERGE #135

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Django 1.5 - DO NOT MERGE #135

wants to merge 10 commits into from

Conversation

ksurya
Copy link
Member

@ksurya ksurya commented Aug 3, 2013

The below are the changes made (some minor changes might have been missed but can be found clearly in commit messages)

  1. Upgrade {% url %} template tag
  2. Use django-haystack 2.1.0, xapian-haystack 2.0.0, Whoosh 2.5.1 -- make respective changes in search_settings.py and search_indexes.py
  3. Use django-registration 1.0 -- Also move registration.backends.default.urls from scipy_central.urls to scipy_central.person.urls
  4. DVCS (windows specific) error fixed
  5. Use django transaction middleware, clickjacking middleware
  6. Make pages.views.page_404_error default 404 handler
  7. Add wsgi.py, change manage.py, upgrade slugify import, upgrade url conf import
  8. Fix os.path.normpath import
  9. Use paginator module for finding "next" or "prev" objects in Revision, Submission module
  10. Make UserProfile model compatible with django-1.5

[5] Django transaction middleware raises #132

Note: A deprecation warning is observed when starting the server regarding hashlib import. This is caused in django transactions middleware (It will be deprecated in 1.7. However, we require it at the moment until we have our own Middleware)

ksurya added 5 commits August 5, 2013 01:50
{% url view_name %} --> {% url 'view_name' %}

https://docs.djangoproject.com/en/1.5/releases/1.5/ (Overview Notes)
This commit requires following
1. django-haystack==2.1.0
2. xapian-haystack==2.0.0
3. xapian-python-bindings-for-python-2.7.0==1.2.8
4. Whoosh==2.5.1

This code is NOT backward compatible with older versions of
django-haystack, xapian-haystack
Kindly install xapian-haystack from its repository and NOT with `pip`
https://github.com/notanumber/xapian-haystack/ (master)
Changes:

1. move built-in registration.backends.default.urls into person.urls (from
scipy_central.urls to scipy_central.person.urls)

2. django-registration latest version NOT compatible with previous
versions. Its majorly written from scratch using class-based `views`
(registration.backends.default.views.RegistrationView)

3. SciPyRegistrationBackend() is inherited from class-based view [2] and
moved into views.py

required packages:
django-registration==1.0
django.conf.urls.defaults import * --> django.conf.urls import *
@lomegor
Copy link

lomegor commented Aug 28, 2013

@ksurya I think you included a lot of changes, that although probably necessary, are not at all related to Django 1.5. Why don't you create more PRs, each with their commits, neatly separated?

@ksurya
Copy link
Member Author

ksurya commented Aug 29, 2013

I was thinking django-1.5 was more like improving existing code. Its why I added lots of code and simultaneously fixed very necessary bugs found.

Since each of the individual bug-fixing or improving code are quite small and confined to upgrading the packages and fixing relevant code, I added them in the same branch. Moreover, I discovered them when I started reading django-docs and other respective docs.

@lomegor
Copy link

lomegor commented Aug 29, 2013

No, django-1.5 is updating the code to work correctly with Django-1.5. Remove all commits that are not part of that, and create a PR for each one of them so it can be decided separately if they are needed and when to merge.

@lomegor
Copy link

lomegor commented Sep 7, 2013

Problems:

  1. The error I told you about in Search pages. Are you sure you are not getting it? Why could it be that I see it?
  2. I can't upload images on submissions. I get error "Error on server, please try again decoder jpeg not available". Please fix both the error message, and whatever is causing this.

I will look at the code now.

@ksurya
Copy link
Member Author

ksurya commented Sep 7, 2013

yeah, you can't upload images. This is fixed in #141 PR

@lomegor
Copy link

lomegor commented Sep 7, 2013

I think commits 30c73e8 and a320450 shouldn't be here. Create new PRs for those.

deploy/wsgi.py Outdated
might make sense to replace the whole Django WSGI application with a custom one
that later delegates to the Django one. For example, you could introduce WSGI
middleware here, or combine a Django application with an application of another
framework.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this previous paragraph, it's not needed.

@lomegor
Copy link

lomegor commented Sep 7, 2013

Also, there are conflicts between this PR and master, which means it can't be automatically merged. Fix them.

@ksurya
Copy link
Member Author

ksurya commented Sep 8, 2013

@lomegor I have found why you were getting server error on search query.

django-haystack requires another module xapian-haystack and xapian-haystack bindings for python.
If you install those, the search is going to work.

pip install xapian-haystack installs 1st module I mentioned. Unfortunately, xapian bindings are highly system dependent and we need to install packages based on it.

As requirements.txt never mentioned these packages, quickstart.py does not install and thus you get this error

1. deploy/wsgi.py
from django.template.defaultfilters import slugify -->
from django.utils.text import slugify

change string type to unicode in tests
`execute_manager` is deprecated -- mentioned in django-1.4 release notes
update manage.py according to django-1.5
admin_media_prefix is deprecated in django-1.4
Its used from static_url
Django 1.5 deprecates AUTH_PROFILE_MODULE settings which enable to extend User model.

scipy central UserProfile fields are not used in User authentication or Registration! So, a OneToField with User module fits our requirement. It is described at https://docs.djangoproject.com/en/1.5/topics/auth/customizing/#extending-the-existing-user-model

The following are the changes done:

1. remove AUTH_PROFILE_MODULE variable in settings
2. Improve UserProfile admin interface
3. change `User` model import
@ksurya
Copy link
Member Author

ksurya commented Sep 8, 2013

I have tested the commit you mentioned a320450
The new changes in slugify() are causing UnicodeDecode error. I moved these changes to 983c851

These changes are required in the PR

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants