Opened 12 years ago

Closed 12 years ago

Last modified 11 years ago

#1132 closed enhancement (duplicate)

[patch] CurrentUserField and manipulator changes

Reported by: jkocherhans <jkocherhans@…> Owned by: Adrian Holovaty
Component: Core (Other) Version:
Severity: normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:


This isn't as high level as I would like, but it works. It's also still usable without access to a request object as well. It feels like a hack to me, but no more so than DateField's auto_now and auto_add_now. I played around with using events, and even with RuleDispatch, but this was the simplest change that worked. It doesn't work for inline objects yet though. I figured I'd get some feedback before I waste my time. Here's how to use it:

class Article(meta.Model):
    title = meta.CharField(maxlength=100)
    body = meta.TextField()
    added_by = meta.CurrentUserField(update_on_edit=False) # only save the current user in add_stage
    edited_by = meta.CurrentUserField() # update_on_edit defaults to True

Attachments (3)

current_user_field_patch.diff (3.5 KB) - added by jkocherhans <jkocherhans@…> 12 years ago.
current_user_field_patch.2.diff (8.0 KB) - added by jkocherhans <jkocherhans@…> 12 years ago.
updated with ideas from rjwittams
current_user_field_patch.3.diff (9.2 KB) - added by jkocherhans <jkocherhans@…> 12 years ago.
patch for magic-removal

Download all attachments as: .zip

Change History (15)

Changed 12 years ago by jkocherhans <jkocherhans@…>

comment:1 Changed 12 years ago by jkocherhans <jkocherhans@…>

Type: defectenhancement

comment:2 Changed 12 years ago by ian@…

Component: Admin interfaceCore framework

I'm not sure I like just appending the 'user' to the save function.
shouldn't this be a dictionary so that the view can pass a set of different name/value pairs?
as I can see this being of use for other things besides from the user-id


comment:3 Changed 12 years ago by jkocherhans <jkocherhans@…>

I don't really like appending 'user' to the save function either. I hadn't thought of passing in a dict, but I think I like the idea. It'll make even more since once the new manipulators in the magic-removal branch land in the trunk and custom manipulators become straightforward to do. I thought about adding the request as an argument rather than just the user id. Would that cover the other things you're thinking of? I think anything more would require custom views/manipulators anyhow, and could be done there.


comment:4 Changed 12 years ago by rjwittams

Yeah, this is really horrible in its present form.
If anything was going to be passed in, it would need to be something general - we don't want to have arbitrary hacks for every bit of functionality.
The least bad thing I can come up with is optionally passing in a mapping - which would usually be the context (but there is no tight coupling, it can just be any mapping). This way processors can be reused, and custom fields can be packaged with the processors required for their functioning.

This would also mean coming up with a much better way of doing this than random isinstance checks inside the manipulator. The field class itself would have to be passed the context ( or whatever ), probably in the get_manipulator_new_data call. Then the field can check to see what is in the post data and in the context.

comment:5 Changed 12 years ago by anonymous

Thanks for the comments rjwittams. You've given me quite a few more ideas to try. Just to clarify though, what do you mean by 'context' above? The template context? Would this be something like the new TEMPLATE_CONTEXT_PROCESSORS stuff?

comment:6 Changed 12 years ago by hugo

#714 would be solved with this ticket, too.

comment:7 Changed 12 years ago by rjwittams

Yeah, the template context. It might be better to think about it more generally than being just for the template system.

Changed 12 years ago by jkocherhans <jkocherhans@…>

updated with ideas from rjwittams

comment:8 Changed 12 years ago by jkocherhans <jkocherhans@…>

I've got a new version of the patch (current_user_field_patch.2.diff) that's a lot more generic. I'm not entriely happy with all of it, but it's closer. fields like added_by vs edited_by don't work yet, and I haven't tested that edit_inline objects work with this though. Generic views would also need to be updated.

I passed the context into the manipulator on init, and that context is passed to fields in get_manipulator_new_data and get_default calls. Comments?

comment:9 Changed 12 years ago by rjwittams

This would need to be done against magic-removal. Manipulators have been cleaned up quite a bit, and this wouldn't get applied to trunk.

Changed 12 years ago by jkocherhans <jkocherhans@…>

patch for magic-removal

comment:10 Changed 12 years ago by jkocherhans <jkocherhans@…>

This patch makes auto_add_now stuff in the manipulator a little more general and does everything the previous ones do. Still no tests and no changes for generic views. If this has a good chance of being applied, I'll finish it up. I still don't like passing the context around in field.get_default and field.get_manipulator_new_data, but I think it's the best way. Suggestions for a different approach are welcome.

comment:11 Changed 12 years ago by jkocherhans <jkocherhans@…>

Most of the issues here are taken care of in #1164

comment:12 Changed 12 years ago by hugo

Resolution: duplicate
Status: newclosed

superseeded by #1268

Note: See TracTickets for help on using tickets.
Back to Top