Plots2: Tagging input appears without logging in

Created on 3 Aug 2018 · 6Comments · Source: publiclab/plots2

Hi, this is a first-timers-only issue. This means we've worked to make it more legible to folks who either haven't contributed to our codebase before, or even folks who haven't contributed to open source before.

If that's you, we're interested in helping you take the first step and can answer questions and help you out as you do. Note that we're especially interested in contributions from people from groups underrepresented in free and open source software!

If you have contributed before, consider leaving this one for someone new, and looking through our general help wanted issues. Thanks!

The problem

The textbox for adding tags appears on the sidebar, even when you aren't logged in. Here's an example.

What's supposed to happen:
The "enter tags" textbox shouldn't appear unless you are logged in and either the author of the post, or an admin.

What actually does happen (screenshot if helpful!):

tagging-logged-out

Solution

The logic that decides when to render the textbox is off. It's comparing the value of a local variable parent to :profile and if they are not equal, then the textbox is displayed. However, in most cases parent is nil, so we're seeing the textbox everywhere this is being checked. This comparison should only worry about the status of current_user and whether it's the author, or an admin.

Where to find the relevant lines of code:

https://github.com/publiclab/plots2/blob/116db0cbaa8df72ea6cdb5e5115bff9eb5d6ee03/app/views/tag/_tagging.html.erb#L43

What to change them to:

Remove the unneeded parent check and parentheses that will be rendered redundant around the user check.

This is what the line should look like after you're done.

<% current_user && (current_user.id == @user.id || current_user.role == "admin") %>

EDIT:

The line https://github.com/publiclab/plots2/blob/116db0cbaa8df72ea6cdb5e5115bff9eb5d6ee03/app/views/tag/_tagging.html.erb#L43

is used for displaying tag form in notes as well as at profile pages, so if parent != :profile is removed then it would check for current_user.id == @user.id but @ user is available/passed only on profile pages, so to ignore second comparison this parent != :profile check is required!

But as we want to ensure that user is present so, changing the above line i.e., https://github.com/publiclab/plots2/blob/116db0cbaa8df72ea6cdb5e5115bff9eb5d6ee03/app/views/tag/_tagging.html.erb#L43

<% if current_user && (parent != :profile || (current_user.id == @user.id || current_user.role == "admin")) %> would do the job.

Steps to Fix

[ ] claim this issue with a comment here, below, and ask any clarifying questions you need
[ ] set up a repository locally following the README instructions, and make sure that all tests pass
[ ] try to fix the issue following the steps above
[ ] commit your changes and start a pull request (see contributing to Public Lab software) but mark it as "in progress" if you have questions or if you haven't finished
[ ] alert someone via the developers list (see below) to have your pull request merged. This may take a few extra steps depending on when you do it, but we'll help you out!

Please email the developers list (see https://publiclab.org/wiki/developers) or go to the chatroom if you have questions, and take a look at our first-timers landing page for more information!

Ruby first-timers-only help wanted

Source

derek-allen

All 6 comments

@jywarren @publiclab/reviewers I've updated the issue to fit the FTO template. Let me know if anything else needs to be done to get this tagged as FTO. Thanks!

derek-allen on 9 Aug 2018

👍1

Awesome, just did this! I'll delete some of our comments to make it a bit cleaner. 👍