[Django] #18148: django.contrib.sites.managers.CurrentSiteManager should be able to span multiple models
Django
multiple models
-------------------------------+--------------------
Reporter: rgeoghegan | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.4
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
The current implementation of CurrentSiteManager can only be used with
models that have a ForeignKey or ManyToManyField reference to a Site
model. Because get_query_set uses the built-in filter syntax, you could
specify remote model references by using a double underscore, except that
the validation code breaks.
For example, you should be able to do:
{{{#!python
class Foo(model.Model):
bar = models.ForeignKey('Bar')
on_site = CurrentSiteManager('bar__site')
}}}
and it should do the right thing.
--
Ticket URL: <https://code.djangoproject.com/ticket/18148>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
multiple models
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.4
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by sfllaw):
* cc: sfllaw@… (added)
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/18148#comment:1>
Django
multiple models
Reporter: rgeoghegan | Owner: nobody
Type: New feature | Status: new
Component: contrib.sites | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Changes (by jezdez):
* component: Uncategorized => contrib.sites
* type: Uncategorized => New feature
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/18148#comment:2>
Django
multiple models
Reporter: rgeoghegan | Owner: nobody
Component: contrib.sites | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Changes (by krzysiumed):
* needs_better_patch: 0 => 1
Comment:
The patch looks really well, but some error messages could be better:
If `ArticleComment` model (from regressiontests.sites_framework.models
module) is changed in this way:
{{{
on_site = CurrentSiteManager("parent_article__non_existing_field")
}}}
then the error message is
{{{
ValueError: CurrentSiteManager couldn't find a field named
parent_article__non_existing_field in ExclusiveArticle.
}}}
The field name is `non_existing_field`, but the message says it's
`parent_article__non_existing_field`.
The other situation when the error message is not meaningful enough is
when the field exists but it's neither `ForeignKey` nor `ManyToManyField`.
See example:
{{{
TypeError: invalid_field must be a ForeignKey or ManyToManyField.
}}}
The message doesn't say name of the model where `invalid_field` was
defined.
I also suggest using `first_article` and `second_article` instead of
`article` and `article2`. It's not more meaningful, but it's easier to see
the difference.
--
Ticket URL: <https://code.djangoproject.com/ticket/18148#comment:3>
Django
multiple models
Reporter: rgeoghegan | Owner: nobody
Component: contrib.sites | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Changes (by krzysiumed):
* cc: krzysiumed@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/18148#comment:4>
Django
multiple models
Reporter: rgeoghegan | Owner: nobody
Component: contrib.sites | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Changes (by rgeoghegan):
* cc: rgeoghegan (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/18148#comment:5>
Django
multiple models
Reporter: rgeoghegan | Owner: nobody
Component: contrib.sites | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Comment (by rgeoghegan):
@krzysiumed: I updated the patch after your comments with the changes you
requested.
--
Ticket URL: <https://code.djangoproject.com/ticket/18148#comment:6>
Django
multiple models
Reporter: rgeoghegan | Owner: nobody
Component: contrib.sites | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Comment (by krzysiumed):
The patch seems to be ready to check in, but I would like to suggest
some ideas. I attached patch including them.
The documentation is slightly unclear -- it's not obvious to which
paragraph refers the '`Changed in Django 1.5`'. I added subsections
which make it less ambiguous, but the titles (especially '`Spanning
relationship`')
could be better. I also changed the section to which
the only link points. The previous target was '`Field lookups`' section
which describes `lte` (e. g. `pub_date__lte`), `exact`
(e. g. `headline__exact`) and other features. I think that '`Lookups that
span relationships`'
section is more appropriate. Finally, I
would be happy if the patch were reviewed by a native speaker.
The changes in code (and tests) are usually renamings and following
`PEP8`. I renamed `first_article` and `second_article` to
`article_inside` and `article_outside` (I did the same about `comment`
and `comment2`). The idea is to make the names more meaningful but I'm
not sure if the goal was achieved.
These ideas are propositions so feel free to reject them if you want.
--
Ticket URL: <https://code.djangoproject.com/ticket/18148#comment:7>
Django
multiple models
Reporter: rgeoghegan | Owner: nobody
Component: contrib.sites | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Changes (by krzysiumed):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/18148#comment:8>
Django
multiple models
Reporter: rgeoghegan | Owner: nobody
Type: New feature | Status: new
Component: contrib.sites | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/18148#comment:9>