| View previous topic :: View next topic |
| Author |
Message |
jens
Administrator
Joined: 12 Oct 2005
Posts: 972
Location: duisburg, germany
|
|
| Back to top |
|
 |
rantaaho
Joined: 09 Jan 2008
Posts: 47
Location: Kuopio, Finland
|
 Posted: Mon 19 May, 2008 20:48 Post subject: |
 |
|
I noticed the old comment, but I still wanted to commit an easy bugfix. 
I'm planning to look page permissions, using model manager could be a nice solution for that too. |
|
| Back to top |
|
|
jens
Administrator
Joined: 12 Oct 2005
Posts: 972
Location: duisburg, germany
|
| Posted: Mon 19 May, 2008 21:17 Post subject: |
|
|
| rantaaho wrote: | | I noticed the old comment, but I still wanted to commit an easy bugfix. |
Thats ok. It wasn't a criticism directed to you
The question is, how many code should be go into a model manager? I don't know.
I should look at other big django projects...
_________________
http://www.jensdiemer.de | http://www.htfx.de | http://www.python-forum.de
|
|
| Back to top |
|
|
rantaaho
Joined: 09 Jan 2008
Posts: 47
Location: Kuopio, Finland
|
| Posted: Wed 21 May, 2008 20:07 Post subject: |
|
|
I started this in http://trac.pylucid.net/changeset/1580 I implemented functions provided in detect_page in Page model manager. Since models.py is already quite big I put PageManager to managers.py. Now:
- Default page is property Page.objects.default_page
- Function get_current_page_obj(request,url) is replaced by Page.objects.get_by_shortcut(url,request.user).
Page.objects.get_by_shortcut raises exception in case of invalid shortcuts at the end of the url and if no page is found corresponding to given shortcuts. These exceptions are now handled in index.py. Previously get_current_page_obj returned HttpResponse in these situations. I also changed input arguments, since I thought that it's clearer to provide user instead of request as the user object is the only thing function needs from request.
I think tree functions from db/page.py could form PageTreeManager. So that they would be available as Page.tree.get_menu(), or something like that.
I will move PluginManager to managers.py, if Jens doesn't do it before me. However, I think that unit tests for Preferences should be updated before that.
Oh, and now http://pylucid.org/Contribute/developer-documentation/render-workflow is again out of date.. |
|
| Back to top |
|
|
jens
Administrator
Joined: 12 Oct 2005
Posts: 972
Location: duisburg, germany
|
| Posted: Thu 22 May, 2008 12:15 Post subject: |
|
|
Yes, the models.py is big. But i don't know if a ./PyLucid/managers.py is a good idea. Put all managers in this file, if will be grow big, too.
Whats about to make model.py as a packages? Don't know if we get unresolved dependencies?!?
Or we leave all model classes in models.py and put only the manager part out there. But not into one file. Group by manager types. We should look into the future. With i18n pages we get more classes around the page content...
_________________
http://www.jensdiemer.de | http://www.htfx.de | http://www.python-forum.de
|
|
| Back to top |
|
|
rantaaho
Joined: 09 Jan 2008
Posts: 47
Location: Kuopio, Finland
|
| Posted: Thu 22 May, 2008 12:43 Post subject: |
|
|
I looked models.py quickly and it might be possible to divide it to different files without too many dependency problems. So, something like:
models/__init__.py - which will import everything
models/Page.py - with Page and PageArchiv at least
models/Plugin.py
etc.
And manager in same file with corresponding model? Or different files: PageManager.py PluginManager.py etc.? |
|
| Back to top |
|
|
jens
Administrator
Joined: 12 Oct 2005
Posts: 972
Location: duisburg, germany
|
| Posted: Thu 22 May, 2008 13:51 Post subject: |
|
|
| rantaaho wrote: | something like:
models/__init__.py - which will import everything
models/Page.py - with Page and PageArchiv at least
models/Plugin.py
etc. |
This looks good.
Also this doesn't break existing imports like "from PyLucid.models import Page"
| rantaaho wrote: | | And manager in same file with corresponding model? Or different files: PageManager.py PluginManager.py etc.? |
With this, i don't know. Put managers and models together can give us a better compendium, isn't it? But i don't know if this will be result in to biggest files.
We should find a compromise between a good summary and not to big files
We can start with the scheme from above and put managers and models together. If the files grows to big in the feature, we can split them later...
_________________
http://www.jensdiemer.de | http://www.htfx.de | http://www.python-forum.de
|
|
| Back to top |
|
|
rantaaho
Joined: 09 Jan 2008
Posts: 47
Location: Kuopio, Finland
|
| Posted: Fri 23 May, 2008 13:58 Post subject: |
|
|
| jens wrote: | | rantaaho wrote: | something like:
models/__init__.py - which will import everything
models/Page.py - with Page and PageArchiv at least
models/Plugin.py
etc. |
This looks good.
|
Yes, it looks good but it didn't work that easily.  See http://trac.pylucid.net/changeset/1584
First of all models/__init__py really must contain lines such as "from Page import Page" otherwise we should import models.Page.Page everywhere, IMHO not pretty but Ok. However, after that Django wants to have Table named as 'your_site.models_page' thus we need to specify db_table in model metadata. Then it took me a while before I found that we must also specify 'app_label' in metadata.
My own test site works, unit tests gave same errors than before refractoring . Overall, not as pretty as I hoped but it seems to work... of course YMMV |
|
| Back to top |
|
|
jens
Administrator
Joined: 12 Oct 2005
Posts: 972
Location: duisburg, germany
|
|
| Back to top |
|
|
jens
Administrator
Joined: 12 Oct 2005
Posts: 972
Location: duisburg, germany
|
|
| Back to top |
|
|
|
FAQ
Search
Memberlist
Usergroups
Register
Profile
Log in to check your private messages
Log in
