Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update individual item for instance case #39

Merged
merged 4 commits into from Nov 19, 2015
Merged

Conversation

AlexRiina
Copy link
Collaborator

The docs have

    @takes_instance_or_queryset
    def tighten_lug_nuts(self, request, queryset):
        queryset.update(lugnuts=F('lugnuts') - 1)

but this updates all values since queryset seems to ignore the _result_cache (as proven by the test in my first commit). I'm not sure filter(pk=...) is the right way to implement this behavior, but @takes_instance_or_queryset seems dangerous as it is now.

fix use model or concrete model
@crccheck
Copy link
Owner

neat. looks like the new model meta api in Django 1.8 breaks things though. I was trying to avoid a db lookup, but looking at your solution, it seems worth having for simpler code. Performance in the Django admin is kind of a joke anyways.

@AlexRiina
Copy link
Collaborator Author

I copied the _meta lookup from the QuerySetIsh so while it doesn't work, it isn't a regression fortunately. I looked around a little and didn't see a great way of getting the model from the instance. Perhaps self.get_queryset(pk=queryset) is best.

@AlexRiina
Copy link
Collaborator Author

Is the requirements.txt file relevant? It isn't getting executed by the setup, contains a hard coded version of django and other dependencies, and isn't used by tox. I think it can be included in the setup.py and tox if you'd like to keep it. It would probably have to have pretty wide ranges like Django>=1.5

@crccheck
Copy link
Owner

requirements.txt is for developers only, and has extra stuff. There's duplication between that, tox.ini, and install_requires that I don't know how to get rid of in a way that doesn't create obfuscation.

@@ -1,6 +1,6 @@
Django==1.8.4
dj-database-url==0.3.0
django-extensions==1.5.5
django-extensions>=1.5.8
Copy link
Owner

Choose a reason for hiding this comment

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

I always pin exact versions in Python projects because writing I'm too lazy to write and don't like the look of django-extensions>=1.5.8,<1.6 and Python doesn't have 1.5.x, ^1.5.8 or ~1.5.8.

@AlexRiina
Copy link
Collaborator Author

updated to pin to django-extensions==1.5.9

crccheck added a commit that referenced this pull request Nov 19, 2015
Fix how queryset-ish wasn't queryset-ish enough
@crccheck crccheck merged commit 3d02a3e into crccheck:master Nov 19, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants