Re: Current user in model.save() context

[Bartłomiej Górny]

Joshua Russo wrote:
> On Mon, Jul 20, 2009 at 9:23 AM, Matthias Kestenholz 
> > 
> wrote:
> 
> thread locals have all the problems which are generally associated
> with global variables. They might be overwritten by other code, they
> make testing extremely difficult because the behavior of methods may
> depend on non-obvious circumstances and it's not clear from the method
> signatures which variables are really used -- in short, it makes code
> maintenance much harder than it needs to be.
> 
> I'll try giving a few examples:
> 
> - You have a field on your model called last_modified_by, and you
> automatically assign request.user in the save() method. You made this
> piece of code much harder to test in an unittest than it needs to be,
> because now you have to provide the complete environment including the
> variables from the threadlocals space in your testsuite. More code is
> needed, and the probability that the code and the testsuite (and
> eventual documentation) get out of sync gets bigger and bigger.
> 
> - Something which I've done for a CRM system which we've developped
> for internal use together with another company was giving Tasks access
> levels, so that we were able to make certain tasks viewable only by
> the management. I built a manager which uses the user information from
> thread local space to filter the queryset automatically according to
> the profile of the currently authenticated user. With this seemingly
> simple move I've made it impossible to use loaddata/dumpdata, because
> no user exists, and I made it very non-obvious that the list of
> viewable tasks depends on the user. You would not get the idea that
> filtering is going on in the templates or even in a big part of the
> code. This is not a big problem when developping alone, but it can
> really hamper progress if someone else takes over the codebase.
> 
> In short: It's easy and it gets the job done, but will come back and
> bite you later for sure. Don't give in to the temptation -- it's much
> better to write custom template tags where you can pass the user
> explicitly to the function, which makes it very clear that the
> output/the result of a certain function depends on the user. It also
> makes testing a breeze, because the behavior of methods is only
> determined by their arguments (as it should be), not by other
> circumstances or the current weather.
> 
> 
> Matthias, thanks. I really appreciate you taking the time to explain 
> that. I always wondered why the objected oriented world crucified 
> global variables. :o)
> 

In this case, I'd rather call it "environment variable", it sounds much 
better ;)

Bartek

> 
> > 


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django users" group.
To post to this group, send email to django-users@googlegroups.com
To unsubscribe from this group, send email to 
django-users+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-users?hl=en
-~--~~~~--~~--~--~---

Re: Current user in model.save() context

[Bartłomiej Górny]

Matthias Kestenholz wrote:
> On Mon, Jul 20, 2009 at 9:26 AM, Bartłomiej Górny wrote:
>> [...]
>>> there is a cookbook recipe for achieving this sort of thing:
>>>
>>> http://code.djangoproject.com/wiki/CookBookThreadlocalsAndUser
>> Yep, that's exactly what I did :)
>>
>>> That's deep in the category of 'give them rope to hang themselves
>>> with' though. You should understand the implications and design
>>> decisions involved before hacking your way around it.
>> I give +1 to Joshua's request - plz explain.
>>
> 
> thread locals have all the problems which are generally associated
> with global variables. They might be overwritten by other code, they
> make testing extremely difficult because the behavior of methods may
> depend on non-obvious circumstances and it's not clear from the method
> signatures which variables are really used -- in short, it makes code
> maintenance much harder than it needs to be.
> 

Thanks for explaining that - I see your point, and I generally agree, 
though I'd like to comment on a couple of things:

> I'll try giving a few examples:
> 
> - You have a field on your model called last_modified_by, and you
> automatically assign request.user in the save() method. You made this
> piece of code much harder to test in an unittest than it needs to be,
> because now you have to provide the complete environment including the
> variables from the threadlocals space in your testsuite. More code is
> needed, and the probability that the code and the testsuite (and
> eventual documentation) get out of sync gets bigger and bigger.

Actually, not necessarily - in testing environment you can simply 
replace the function that gives you current user. Though I understand 
that some people may find it disgusting.

> - Something which I've done for a CRM system which we've developped
> for internal use together with another company was giving Tasks access
> levels, so that we were able to make certain tasks viewable only by
> the management. I built a manager which uses the user information from
> thread local space to filter the queryset automatically according to
> the profile of the currently authenticated user. With this seemingly
> simple move I've made it impossible to use loaddata/dumpdata, because
> no user exists, and I made it very non-obvious that the list of
> viewable tasks depends on the user. You would not get the idea that
> filtering is going on in the templates or even in a big part of the
> code. This is not a big problem when developping alone, but it can
> really hamper progress if someone else takes over the codebase.
> 

True. For getting/filtering, user should be passed explicitly.

> In short: It's easy and it gets the job done, but will come back and
> bite you later for sure. Don't give in to the temptation -- it's much
> better to write custom template tags where you can pass the user
> explicitly to the function, which makes it very clear that the
> output/the result of a certain function depends on the user. It also
> makes testing a breeze, because the behavior of methods is only
> determined by their arguments (as it should be), not by other
> circumstances or the current weather.

There are two sides to that - for view functions, you can write 
templatetags. But you have also to create and save objects, and then you 
have to set or pass the user manually every time you create or save 
anything anywhere in your code. This means more typing, and it is also 
easy to forget about it, so you get another kind of trouble. As far as 
I'm concerned, the main use of this way of getting user is creating 
objects, not filtering.

Bartek

> 



> 
> Matthias
> 
> > 


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django users" group.
To post to this group, send email to django-users@googlegroups.com
To unsubscribe from this group, send email to 
django-users+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-users?hl=en
-~--~~~~--~~--~--~---

Re: Current user in model.save() context

[Joshua Russo]

On Mon, Jul 20, 2009 at 9:23 AM, Matthias Kestenholz <
matthias.kestenh...@gmail.com> wrote:

> thread locals have all the problems which are generally associated
> with global variables. They might be overwritten by other code, they
> make testing extremely difficult because the behavior of methods may
> depend on non-obvious circumstances and it's not clear from the method
> signatures which variables are really used -- in short, it makes code
> maintenance much harder than it needs to be.
>
> I'll try giving a few examples:
>
> - You have a field on your model called last_modified_by, and you
> automatically assign request.user in the save() method. You made this
> piece of code much harder to test in an unittest than it needs to be,
> because now you have to provide the complete environment including the
> variables from the threadlocals space in your testsuite. More code is
> needed, and the probability that the code and the testsuite (and
> eventual documentation) get out of sync gets bigger and bigger.
>
> - Something which I've done for a CRM system which we've developped
> for internal use together with another company was giving Tasks access
> levels, so that we were able to make certain tasks viewable only by
> the management. I built a manager which uses the user information from
> thread local space to filter the queryset automatically according to
> the profile of the currently authenticated user. With this seemingly
> simple move I've made it impossible to use loaddata/dumpdata, because
> no user exists, and I made it very non-obvious that the list of
> viewable tasks depends on the user. You would not get the idea that
> filtering is going on in the templates or even in a big part of the
> code. This is not a big problem when developping alone, but it can
> really hamper progress if someone else takes over the codebase.
>
> In short: It's easy and it gets the job done, but will come back and
> bite you later for sure. Don't give in to the temptation -- it's much
> better to write custom template tags where you can pass the user
> explicitly to the function, which makes it very clear that the
> output/the result of a certain function depends on the user. It also
> makes testing a breeze, because the behavior of methods is only
> determined by their arguments (as it should be), not by other
> circumstances or the current weather.


Matthias, thanks. I really appreciate you taking the time to explain that. I
always wondered why the objected oriented world crucified
global variables. :o)

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django users" group.
To post to this group, send email to django-users@googlegroups.com
To unsubscribe from this group, send email to 
django-users+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-users?hl=en
-~--~~~~--~~--~--~---

Re: Current user in model.save() context

[Matthias Kestenholz]

On Mon, Jul 20, 2009 at 9:26 AM, Bartłomiej Górny wrote:
> [...]
>> there is a cookbook recipe for achieving this sort of thing:
>>
>> http://code.djangoproject.com/wiki/CookBookThreadlocalsAndUser
>
> Yep, that's exactly what I did :)
>
>>
>> That's deep in the category of 'give them rope to hang themselves
>> with' though. You should understand the implications and design
>> decisions involved before hacking your way around it.
>
> I give +1 to Joshua's request - plz explain.
>

thread locals have all the problems which are generally associated
with global variables. They might be overwritten by other code, they
make testing extremely difficult because the behavior of methods may
depend on non-obvious circumstances and it's not clear from the method
signatures which variables are really used -- in short, it makes code
maintenance much harder than it needs to be.

I'll try giving a few examples:

- You have a field on your model called last_modified_by, and you
automatically assign request.user in the save() method. You made this
piece of code much harder to test in an unittest than it needs to be,
because now you have to provide the complete environment including the
variables from the threadlocals space in your testsuite. More code is
needed, and the probability that the code and the testsuite (and
eventual documentation) get out of sync gets bigger and bigger.

- Something which I've done for a CRM system which we've developped
for internal use together with another company was giving Tasks access
levels, so that we were able to make certain tasks viewable only by
the management. I built a manager which uses the user information from
thread local space to filter the queryset automatically according to
the profile of the currently authenticated user. With this seemingly
simple move I've made it impossible to use loaddata/dumpdata, because
no user exists, and I made it very non-obvious that the list of
viewable tasks depends on the user. You would not get the idea that
filtering is going on in the templates or even in a big part of the
code. This is not a big problem when developping alone, but it can
really hamper progress if someone else takes over the codebase.

In short: It's easy and it gets the job done, but will come back and
bite you later for sure. Don't give in to the temptation -- it's much
better to write custom template tags where you can pass the user
explicitly to the function, which makes it very clear that the
output/the result of a certain function depends on the user. It also
makes testing a breeze, because the behavior of methods is only
determined by their arguments (as it should be), not by other
circumstances or the current weather.


Matthias

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django users" group.
To post to this group, send email to django-users@googlegroups.com
To unsubscribe from this group, send email to 
django-users+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-users?hl=en
-~--~~~~--~~--~--~---

Re: Current user in model.save() context

[Bartłomiej Górny]

Matthias Kestenholz wrote:
> On Fri, Jul 17, 2009 at 4:24 PM, Bartłomiej Górny wrote:
>> Phil wrote:
>>> Hi Josh,
>>>
>>> unfortunately it seems that there is no way to do so. As you've
>>> noticed that correctly you can use request (request.user) in any place
>>> but model save.
>> Yes, I bumped into the same problem and came to the same conclusion. No
>> way. Unless you hack out something along the lines of
>> translation.activate/get_language - using a global dictionary and
>> current thread. It works, but is hardly in line with Django spirit.
>>
>> I think it is a design decision, to keep MVC structure clean. Though it
>> might be arguable if it would really seriously breach the MVC principle.
>> Maybe it is worthwhile.
>>
> 
> If you really need to do that (or if you are just extremely lazy)

I just don't like copy-and-paste programming. You can call it laziness, 
if you like.

> there is a cookbook recipe for achieving this sort of thing:
> 
> http://code.djangoproject.com/wiki/CookBookThreadlocalsAndUser

Yep, that's exactly what I did :)

> 
> That's deep in the category of 'give them rope to hang themselves
> with' though. You should understand the implications and design
> decisions involved before hacking your way around it.

I give +1 to Joshua's request - plz explain.

Bartek

> 
> Matthias
> 
> > 


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django users" group.
To post to this group, send email to django-users@googlegroups.com
To unsubscribe from this group, send email to 
django-users+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-users?hl=en
-~--~~~~--~~--~--~---

Re: Current user in model.save() context

[Joshua Russo]

On Sat, Jul 18, 2009 at 8:06 AM, Matthias Kestenholz <
matthias.kestenh...@gmail.com> wrote:

> If you really need to do that (or if you are just extremely lazy)
> there is a cookbook recipe for achieving this sort of thing:
>
> http://code.djangoproject.com/wiki/CookBookThreadlocalsAndUser
>
> That's deep in the category of 'give them rope to hang themselves
> with' though. You should understand the implications and design
> decisions involved before hacking your way around it.


I appreciate the concern in regards to breaking concepts. I'm not sure I
understand the concept (the portion of MVC) that would be broken. Could you
(or anyone for that matter) explain the ramifications?

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django users" group.
To post to this group, send email to django-users@googlegroups.com
To unsubscribe from this group, send email to 
django-users+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-users?hl=en
-~--~~~~--~~--~--~---

Re: Current user in model.save() context

[Matthias Kestenholz]

On Fri, Jul 17, 2009 at 4:24 PM, Bartłomiej Górny wrote:
>
> Phil wrote:
>> Hi Josh,
>>
>> unfortunately it seems that there is no way to do so. As you've
>> noticed that correctly you can use request (request.user) in any place
>> but model save.
>
> Yes, I bumped into the same problem and came to the same conclusion. No
> way. Unless you hack out something along the lines of
> translation.activate/get_language - using a global dictionary and
> current thread. It works, but is hardly in line with Django spirit.
>
> I think it is a design decision, to keep MVC structure clean. Though it
> might be arguable if it would really seriously breach the MVC principle.
> Maybe it is worthwhile.
>

If you really need to do that (or if you are just extremely lazy)
there is a cookbook recipe for achieving this sort of thing:

http://code.djangoproject.com/wiki/CookBookThreadlocalsAndUser


That's deep in the category of 'give them rope to hang themselves
with' though. You should understand the implications and design
decisions involved before hacking your way around it.


Matthias

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django users" group.
To post to this group, send email to django-users@googlegroups.com
To unsubscribe from this group, send email to 
django-users+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-users?hl=en
-~--~~~~--~~--~--~---

Re: Current user in model.save() context

[Shawn Milochik]

I think you still have to pass the user, but you can tell if the item  
is modified or new by whether its id has a value. If it's brand new  
and has never been saved, then it won't have one.

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django users" group.
To post to this group, send email to django-users@googlegroups.com
To unsubscribe from this group, send email to 
django-users+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-users?hl=en
-~--~~~~--~~--~--~---

Re: Current user in model.save() context

[Bartłomiej Górny]

Phil wrote:
> Hi Josh,
> 
> unfortunately it seems that there is no way to do so. As you've
> noticed that correctly you can use request (request.user) in any place
> but model save.

Yes, I bumped into the same problem and came to the same conclusion. No
way. Unless you hack out something along the lines of
translation.activate/get_language - using a global dictionary and 
current thread. It works, but is hardly in line with Django spirit.

I think it is a design decision, to keep MVC structure clean. Though it 
might be arguable if it would really seriously breach the MVC principle. 
Maybe it is worthwhile.

Bartek

> 
> most likely that will be view:
> 
> ---
> if form.is_valid():
> newObj = form.save(commit=False)
> newObj.user = request.user
> newObj.save()
> ---
> 
> Cheers,
> Phil
> 
> On Jul 17, 7:19 am, Joshua Russo  wrote:
>> I think I might be overlooking something simple here. I have a set of
>> 4 fields in almost every table (user create & modified and date create
>> & modified). The date (datetime) is easy with auto_now and
>> auto_now_add options.
>>
>> The user has been much trickier. I can't figure out how (if possible)
>> to get access to the current user information in the context of a
>> model. The work around that I've implemented is a method that takes
>> the model, user id, and a boolean indicating modification or new. I
>> then use this in location where I still have a request object (and
>> thus request.user.id) like my views or save_model() of the admin form.
>>
>> Is this as good as I can do, or can I access the current user (or
>> request object) from within the model?
>>
>> Thanks
>> Josh
> > 



--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django users" group.
To post to this group, send email to django-users@googlegroups.com
To unsubscribe from this group, send email to 
django-users+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-users?hl=en
-~--~~~~--~~--~--~---

Re: Current user in model.save() context

[Phil]

Hi Josh,

unfortunately it seems that there is no way to do so. As you've
noticed that correctly you can use request (request.user) in any place
but model save.

most likely that will be view:

---
if form.is_valid():
newObj = form.save(commit=False)
newObj.user = request.user
newObj.save()
---

Cheers,
Phil

On Jul 17, 7:19 am, Joshua Russo  wrote:
> I think I might be overlooking something simple here. I have a set of
> 4 fields in almost every table (user create & modified and date create
> & modified). The date (datetime) is easy with auto_now and
> auto_now_add options.
>
> The user has been much trickier. I can't figure out how (if possible)
> to get access to the current user information in the context of a
> model. The work around that I've implemented is a method that takes
> the model, user id, and a boolean indicating modification or new. I
> then use this in location where I still have a request object (and
> thus request.user.id) like my views or save_model() of the admin form.
>
> Is this as good as I can do, or can I access the current user (or
> request object) from within the model?
>
> Thanks
> Josh
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django users" group.
To post to this group, send email to django-users@googlegroups.com
To unsubscribe from this group, send email to 
django-users+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-users?hl=en
-~--~~~~--~~--~--~---