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

Added new page for tagged content by a single author with '/tag/:tagname/author/:authorname' #1872

Merged
merged 14 commits into from Jan 12, 2018

Conversation

SidharthBansal
Copy link
Member

@SidharthBansal SidharthBansal commented Dec 11, 2017

fixes #1854

This'll allow:

http://publiclab.org/tag/replication:*/author/warren would show all replications by author warren!

Progress:

  • a config/routes.rb entry
  • a re-use of the /app/views/tag/show.html.erb template with a custom title
  • a new action in /app/controllers/tag_controller.rb for, maybe def show_for_author ? which uses the new Tag.tagged_nodes_by_author() method here
  • a functional test in /test/functionals/tag_controller_test.rb to show that the new show_for_author action works

@PublicLabBot
Copy link

PublicLabBot commented Dec 11, 2017

1 Warning
⚠️ It looks like you merged from master in this pull request. Please rebase to get rid of the merge commits – you may want to rewind the master branch and rebase instead of merging in from master, which can cause problems when accepting new code!
1 Message
📖 @SidharthBansal Thank you for your pull request! I’m here to help with some tips and recommendations. Please take a look at the list provided and help us review and accept your contribution! And don’t be discouraged if you see errors – we’re here to help.

Generated by 🚫 Danger

@jywarren
Copy link
Member

This old code tried to do this!

# notes for given comma-delimited tags params[:topic] for author
def author_topic
@user = DrupalUser.find_by(name: params[:author])
@tagnames = params[:topic].split('+')
@title = @user.name + " on '" + @tagnames.join(', ') + "'"
@notes = @user.notes_for_tags(@tagnames)
@unpaginated = true
render template: 'notes/index'
end

Let's update this or replace it with your new code. Great!

@SidharthBansal
Copy link
Member Author

@jywarren I have made a commit can you please view it. At my localhost:3000 it is working properly.
All previous tests are passing.
Please tell me the required changes if any.
Thanks

@SidharthBansal
Copy link
Member Author

SidharthBansal commented Jan 5, 2018

@jwarren I am little confused about this commit. What are we trying to build?
I guess we want to have a page like tag/show for the tags with authors, as you wrote in the comment. In that page, we have buttons for the research notes, questions, wiki pages, maps, and contributors. So, what will they link to in this navbar? I checked the routes file but I could not find any route linking to questions tagged with the tag name and author name and same for other fields. Are we suppose to create them later on? Also, I tried to understand what wiki leads are but can't understand. Can you please help me in understanding them..? Curious to know about them.
I have created the notes view which is supposed to be wrong if we need to use the tag/show. Otherwise, it is working fine with the notes view.
Also, I found out that the contributor's page on the navbar links to another page which is completely different from the tag/show view. So, does it needs to be changed?
One more thing, maybe not important. But I felt a little odd to have space at the end of the search of publiclab.org/tag/balloon*. I think there should be either 28 search results or 32 search results or say 20 like on https://publiclab.org/profile/warren/likes. So, that there is no extra space. I know by default will_paginate has 30 records. We can change it. Also this is happening not only on this but on many pages of the publiclab.org.

@wildcard = true
end
@tagname = params[:id]
@user = DrupalUser.find_by(name: params[:author])
Copy link
Member

Choose a reason for hiding this comment

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

Can we just search User.find_by here, not DrupalUser which we're trying to reduce usage of (and eliminate)? Thanks!

@@ -239,7 +239,6 @@ def self.trending(limit = 5 , start_date = DateTime.now - 1.month , end_date = D
#select nodes by tagname and user_id
def self.tagged_nodes_by_author(tagname, user_id)
if tagname[-1..-1] == '*'
@wildcard = true
Copy link
Member

Choose a reason for hiding this comment

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

Ah, we use this to disable some things that don't currently work with wildcard searches... so maybe preserve this unless we're sure that the whole page works identically for normal tags and wildcard queries?

@@ -0,0 +1,43 @@
<% if params[:controller] == "tag" || params[:controller] == "search" %>
Copy link
Member

Choose a reason for hiding this comment

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

Was there not an efficient way to use the same show.html.erb template, and adapt it using one of these if/else statements for displaying posts by a single author?

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps anything that doesn't currently work with just one author, like the other tabs, could be disabled by checking the params[:action] to see if this is an instance of an author-specific tag show page and not just a normal one?

Copy link
Member Author

Choose a reason for hiding this comment

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

I will try to change it

@@ -185,6 +185,19 @@ def setup
assert_select '#wiki-content', 1
end

test 'show note with author and tagname without wildcard' do
Copy link
Member

Choose a reason for hiding this comment

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

These look great. Awesome job on the tests.

@jywarren
Copy link
Member

jywarren commented Jan 8, 2018

Sorry, just getting to this after the backlog over the new year. I made some requests above.

The bigger goal here is to make it possible to list completed activities on peoples' profiles.

When you complete an activity and post a response, it's tagged replication:______ and so my hope was to develop a query we can use to display any post that is:

  1. by a given author
  2. tagged with replication:______ -- a wildcard search

Then, we can:

  1. display the count of such posts on peoples' profiles and
  2. have a consistent page and route to view a list of such posts (this route could have other uses too!)

Does this make sense?

understand what wiki leads are

Do you mean wiki likes? Sorry, what section of code?

Regarding pagination of 30, i agree! We can change it to 24 if you want to open a new PR.

@SidharthBansal
Copy link
Member Author

SidharthBansal commented Jan 9, 2018

This
image
on https://publiclab.org/tag/balloon. Can you explain about this feature? I tried to find a page leading to wiki pages but cant. I also tried to click on the button add one now but it was then redirecting the login
page eventhough I was logged in.

@jywarren
Copy link
Member

jywarren commented Jan 9, 2018

The idea of the tag page wiki button is that if there's a wiki page whose URL matches the tagname, like /wiki/gsoc for tag gsoc, then we show the first paragraph and the lead image of that page on the tag page, as a kind of "intro". If it doesn't exist, then we prompt people to create such a wiki page to provide a nice intro for the page.

but it was then redirecting the login page eventhough I was logged in.

Are you sure? i just tried this for /wiki/abcdef and it worked for me -- presented me a blank wiki page form.

It looks like your _header.html.erb is out of sync due to other changes we've since merged. You can probably rebase all your changes over the latest master -- see https://publiclab.org/wiki/developers for some guidance! -- or you can use the GitHub interface button to Resolve conflicts. I'm happy to help if you get in trouble! Thanks!

@SidharthBansal
Copy link
Member Author

SidharthBansal commented Jan 10, 2018

Ok, got it. I checked it again after logging out and then logging in again. Then, it worked. It is not displaying the login page. Thanks...
I will change the view soon
Regarding conflicts --yeah I will resolve all of them
Just thinking, it will be better not to display "has no lead wiki " for power tags. Because even though Gsoc wiki page exists, searching /tag/Gsoc* gives us that it leads to no wiki page. I think, if a person is searching for power tag, then he/ she should get a message that "Enter a tag instead of power tag to get related wiki pages" or we can remove the lead wiki section when there is a power tag type search

@SidharthBansal
Copy link
Member Author

SidharthBansal commented Jan 10, 2018

image
image
image
image
I think it is working fine. @jywarren As per your suggestion I have disabled the buttons which are not needed in this with a tooltip.
I have used the google translator for the locale files. I don't know Dutch. Hope it works.

@jywarren
Copy link
Member

jywarren commented Jan 10, 2018 via email

@jywarren
Copy link
Member

Indeed you did! This looks great; is it ready? Want to throw in a functional test to confirm this at least loads successfully in tests? This will protect your code from future issues too.

Awesome!

@@ -18,3 +18,4 @@ de:
no_results_found: "Keine Ergebnisse gefunden; versuchen, für '<b>%{tag}</b>' Suche"
try_advanced_search: "Oder versuchen Sie eine <a href='%{url1}'>erweiterte Suche</a>"
contributors: "Mitwirkende"
tagged_by: "Notities getagd met \"%{tags}\" door <a href='%{url1}'>%{user_name}</a>"
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove this, and let it default to English, ok? Thanks!

@jywarren
Copy link
Member

Just one more tweak to leave the German translation blank, and we'll be good to merge!

@jywarren
Copy link
Member

Thanks, this is exciting!

@jywarren jywarren changed the title Added new page for tagged content by a single author(Resolves #1854) Added new page for tagged content by a single author with '/tag/:tagname/author/:authorname' Jan 12, 2018
@jywarren
Copy link
Member

Amazing. This is a great implementation. 🎉 🎈

@jywarren jywarren merged commit 432a7ce into publiclab:master Jan 12, 2018
@jywarren
Copy link
Member

I hope we'll deploy it today and then we can check http://publiclab.org/tag/replication:*/author/warren -- and then make a link to it from the profile page. Thank you!!!

@SidharthBansal
Copy link
Member Author

@jywarren Just thinking, it will be better not to display "has no lead wiki " for power tags. Because even though Gsoc wiki page exists, searching /tag/Gsoc* gives us that it leads to no wiki page. I think, if a person is searching for power tag, then he/ she should get a message that "Enter a tag instead of power tag to get related wiki pages" or we can remove the lead wiki section when there is a power tag type search.
One ambiguity with power tag search is a user may assume that if no g* page exists this implies gsoc cant be a wiki page.
Another way can be for power tags it will display all wiki pages related to that tag. But this would be ugly.

@jywarren
Copy link
Member

I think i agree -- we could could truncate the * and just lead to /wiki/replication for replication* -- and then be sure we make explanatory pages for those sorts of things. If you'd like to open a new issues for that, go for it!

Same with the link from profile page. I think we could show a count and link with a fourth column where it displays questions, answers, and comments. What do you think?

@SidharthBansal
Copy link
Member Author

I think truncating is not a better option. Reason -- if a lazy person who knows a little code writes replica instead of replication because he knows replication is derived from the replica this will again end up in a chaos.
One more thing, power tags eg * itself by the code(regex) means all tags. I think, we should optimise the condition of power tags too, so the user will enter first part fully.

@Fastie
Copy link

Fastie commented Jan 14, 2018

This is really great. There have been many times when I have wanted to use something like this. I added some links to my profile with this type of search. Thanks much.
https://publiclab.org/profile/cfastie

@Fastie
Copy link

Fastie commented Jan 14, 2018

The link to the author's profile at the author's tag result page is broken, e.g., "Notes on ndvi by cfastie" on this page: https://publiclab.org/tag/ndvi/author/cfastie

@SidharthBansal
Copy link
Member Author

@Fastie thanks, I will change it

SrinandanPai pushed a commit to SrinandanPai/plots2 that referenced this pull request May 5, 2019
…ame/author/:authorname' (publiclab#1872)

* Added new page for tagged content by a single author(Resolves publiclab#1854)

* Added show_for_author view and action

* Added Tests

* Modified controller and model

* Modified controller and view

* Changed the view

* Views corrected

* Fixed I18n

* Controller test modified

* Testing improved

* Tests improved

* Localisation removed
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.

Add a new page for tagged content by a single author
4 participants