Opened 11 years ago
Closed 10 years ago
#686 closed enhancement (fixed)
Create User.url_for_self() method and use it
Reported by: | Elrond | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | |
Component: | programming | Keywords: | bitesized |
Cc: | ben@… | Parent Tickets: |
Description
The User
object should have a .url_for_self
method like the media entries or collections already have.
With the new redirect_obj
it should then be as easy as
return redirect_obj(request, user)
to redirect to the user's main page.
So use redirect_obj
or at least the .url_for_self
around the sources.
Attachments (2)
Change History (12)
comment:1 by , 11 years ago
Status: | new → review |
---|
by , 11 years ago
Attachment: | trac#686-user.url_for_self.diff added |
---|
Add url_for_self function to User.
comment:2 by , 11 years ago
Cc: | added |
---|
comment:3 by , 11 years ago
It is useful to have **kwargs
because you could add qualified=true
or query strings.
This looks good to me, but I would say that this should go in mediagoblin.db.mixin.py instead of models.py
comment:4 by , 11 years ago
Status: | review → accepted |
---|
by , 11 years ago
Attachment: | trac#686-usermixin.url_for_self.diff added |
---|
Add url_for_self function to UserMixin.
follow-up: 7 comment:5 by , 11 years ago
I've attached an updated patch moving the url_for_self
function to UserMixin
.
This appears to be all that's needed and existing tests pass, but I'm unclear on the best way to verify that it does indeed work, since it needs the urlgen
parameter. Is there a way to get hold of urlgen
without running the whole stack?
PS. My username is Sturm rather than Strum, but I appreciate the mention in previous commits anyway. :)
comment:6 by , 11 years ago
Status: | accepted → review |
---|
comment:7 by , 11 years ago
Replying to Sturm:
This appears to be all that's needed and existing tests pass, but I'm unclear on the best way to verify that it does indeed work, since it needs the
urlgen
parameter. Is there a way to get hold ofurlgen
without running the whole stack?
I should point out that I have verified that the new url_for_self
method works, but only by adding a PDB call inside a view like user_home
and running user.url_for_self(request.urlgen)
. That produces /u/ben/
, which is the expected value. I just wondered if there was a neater way to do this.
comment:8 by , 11 years ago
Owner: | set to |
---|---|
Status: | review → in_progress |
So this looks good.
It would be nice if you could write a test for this. I think it could go in test_modelmethods.py
. I think you would need to do a GET request and then pull the request object out of the response, which you could then pass to the url_for_self method and verify that it does the correct thing. You can take a look at test_submission.py
for an example of a test using request.urlgen
.
If you could write that test, I will merge this.
comment:9 by , 11 years ago
Owner: | removed |
---|---|
Status: | in_progress → review |
I've created issue686-url_for_self branch for this issue on Gitorious. The patch also has a test.
comment:10 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | review → closed |
Merged. Great job berker!
Also added a small note to the merge:
*Note from cwebber on merge of this branch:* Thanks also to Sturm who provided an early version of this patch.
I've attached a patch to add the
User.url_for_self
function. This is my first patch to MediaGoblin, so I'd appreciate some feedback.I'm a bit unsure about the
urlgen
function. I can see that it's a reference to thebuild_proxy
function of aRequest
inmediagoblin.app.call_backend
, but it's not clear to me what it actually is.build_proxy
could do with some docs in this regard.I'm also not sure whether it's useful to allow
**kwargs
onurl_to_self
- I'm doing this because this is how it's done inmixin.MediaEntryMixin.url_for_self
.