Opened 11 years ago
Closed 10 years ago
#430 closed enhancement (fixed)
Filter By Tags Under User
Reported by: | Rob Myers | Owned by: | Christopher Allan Webber |
---|---|---|---|
Priority: | major | Milestone: | 0.3.3 |
Component: | programming | Keywords: | bitesized, review |
Cc: | deletesoftware@… | Parent Tickets: |
Description
Please can it be made possible to search for tags *under a username*.
i.e. something like:
http://mediagoblin.com/u/rob/tag/makerbot/
would list all my images tagged with "makerbot".
Subtickets
Change History (16)
comment:1 Changed 11 years ago by
Keywords: | bitesized added |
---|
comment:2 Changed 11 years ago by
Owner: | changed from somebody to NattilyPidgin |
---|---|
Status: | new → assigned |
comment:3 Changed 11 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
My changes are included here:
https://gitorious.org/~npigeon/mediagoblin/npigeons-mediagoblin/commits/addusertags
You can read my change files, but a short summary follows...
in order to create the URL I:
modified mediagoblin/user_pages/routing.py
created mediagoblin/user_pages/listing/views.py (vbased off of but distinct from mediagoblin/listing/views.py)
created mediagoblin/user_pages/listing/init.py (identical to mediagoblin/listing/init.py)
and in order to place the page properly with links I:
modified mediagoblin/templates/mediagoblin/utils/tags.html
created mediagoblin/templates/mediagoblin/listings/user_tag.html
and I also added a link to the old tag/{tag}/ page from the new /u/{user}/tag/{tag} page
created mediagoblin/templates/mediagoblin/utils/tag_link.html
one file was minorly edited by accident and made it into the commit although no real changes happened to it (mediagoblin/templates/mediagoblin/listings/tag.html)
comment:4 Changed 11 years ago by
Keywords: | needs-review added |
---|---|
Resolution: | fixed |
Status: | closed → reopened |
comment:5 Changed 11 years ago by
Owner: | changed from NattilyPidgin to Elrond |
---|---|
Status: | reopened → assigned |
Ok, will review (and merge).
comment:6 Changed 11 years ago by
I just wanted to check in and make sure that everything was okay with this bug and was wondering if it was still to be committed into the trunk and otherwise if there was something I could do to clean up my work or anything.
comment:7 Changed 11 years ago by
Hi,
excuse my delay!
I am totally busy with other stuff. But this IS on my todo list.
If there is a hurry, you might ask Chris webber or Joar. But otherwise, I'm ttying to get this done next weekend.
comment:8 Changed 11 years ago by
Oh don't worry at all, I just was doing this for an independent study at school and wanted to make sure that everything was working out fine... I don't think there's any real rush though, thanks for responding!
comment:9 Changed 11 years ago by
Component: | component1 → programming |
---|---|
Milestone: | → 0.3.1 |
I'm marking this in milestone 0.3.1 because it looks like it may be pretty close?
That may be abusing milestones, however ;)
comment:10 Changed 11 years ago by
Keywords: | needs-review removed |
---|---|
Owner: | changed from Elrond to NattilyPidgin |
Hi,
finally got to review this. There are some things you should fix, I think.
I have to admit, I didn't try it, just read the code.
So if I'm plain wrong, tell me!
- In none of the db queries (hidden behind media_entries_for_tag_slug) I can see a reference to the user. So how do you only show entries for a specific user? I guess, we need to enhance media_entries_for_tag_slug to actually do that. Or append an appropiate
.filter(MediaEntry.uploader==user.id)
to each cursor. _get_tag_name_from_entries
feels copied from somewhere. Please refactor this into some function that can be called from both places.mediagoblin/templates/mediagoblin/utils/tags.html
: You changed two of the three parts to include theuser=
... thing. But for the last you only changed the urlgen target but did not include theuser=
...
If you need any help with my comments, best contact me on irc. :-)
That said, if you have fixed the things, just re assign back to me and add the needs-review keyword.
comment:11 Changed 11 years ago by
Cc: | deletesoftware@… added |
---|
comment:12 Changed 11 years ago by
npigeon: How're you doing with this issue? Is this something you think you can finish up in the next two weeks for 0.3.2?
comment:13 Changed 10 years ago by
Milestone: | 0.3.1 → 0.3.3 |
---|
I also have a branch implementing this. The hard part is not providing the view, but to decide where to put links to global and where to a user's tags...
comment:14 Changed 10 years ago by
Keywords: | review added |
---|---|
Owner: | changed from NattilyPidgin to Christopher Allan Webber |
Okay, I'm tacking this onto my review queue and assigning to myself!
comment:15 Changed 10 years ago by
I actually think the way this should be handled is that tags by default should point to a *user's* tags, but on the tag page, it should have a link to the global version of that tag? What do people think?
comment:16 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Just merged the branch and made a couple of fixes. FYI: for the future, it's best not to do:
{% trans %}start of a sentence{% endtrans %} {% if foo %}{% trans %}and conditionally, the end of that sentence{% endtrans %}{% endif %}
This is called lego translations, and it doesn't really work because syntactic structure isn't shared across languages! It's best to do:
{% if foo %} {% trans %}start of a sentence and conditionally, the end of that sentence{% endtrans %} {% else %} {% trans %}start of a sentence{% endtrans %} {% endif %}
that's a bit annoyingly verbose, but our translators will thank us for it! :)
This was a really great branch though, and I'm thrilled to have this functionality in 0.3.3! Woot! Thanks very much Spaetz (and npigeon for the previous work)!
This is an easy task for a new contributor who is reasonably experienced with python or web programming.