Posting from tag pages not tagging properly

[steviepubliclab]

Hello!
I was trying to post a new research note from a tag page (https://publiclab.org/tag/oil-and-gas-water-trio) using this button:

But the note it created didn't have the tag for the page:

Might be a bug?

[nicoleiocana]

Can you offer some insight as to how you replicated the error? When I go to the url you provided: https://publiclab.org/tag/oil-and-gas-water-trio my screen does how have the "New Post + " button. It shows this:

[steviepubliclab]

Are you logged into a Public Lab account? You might not see the button if you're not logged in.

…

On Sat, Nov 2, 2019 at 11:30 PM nicoleiocana ***@***.***> wrote: Can you offer some insight as to how you replicated the error? When I go to the url you provided: https://publiclab.org/tag/oil-and-gas-water-trio <http://url> my screen does how have the "New Post + " button. It shows this: [image: publiclab_url] <https://user-images.githubusercontent.com/44947175/68079960-8f7dc800-fdaf-11e9-9d07-4d3b88fa3db6.PNG> — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6592?email_source=notifications&email_token=ABZEXLQC7GPSK6FDLWIXHX3QRZAWJA5CNFSM4JH6HNSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC5KDLA#issuecomment-549101996>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABZEXLUIGSGBNNZHFP3FJLDQRZAWJANCNFSM4JH6HNSA> .

[nicoleiocana]

Yes. I just clicked on the link on my mobile, logged in, then refreshed the page. The page still doesn't show the "New Post +" button.

…

On Mon, Nov 4, 2019, 7:46 AM Stevie ***@***.***> wrote: Are you logged into a Public Lab account? You might not see the button if you're not logged in. On Sat, Nov 2, 2019 at 11:30 PM nicoleiocana ***@***.***> wrote: > Can you offer some insight as to how you replicated the error? When I go > to the url you provided: https://publiclab.org/tag/oil-and-gas-water-trio > <http://url> my screen does how have the "New Post + " button. It shows > this: > > [image: publiclab_url] > < https://user-images.githubusercontent.com/44947175/68079960-8f7dc800-fdaf-11e9-9d07-4d3b88fa3db6.PNG > > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > < #6592?email_source=notifications&email_token=ABZEXLQC7GPSK6FDLWIXHX3QRZAWJA5CNFSM4JH6HNSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC5KDLA#issuecomment-549101996 >, > or unsubscribe > < https://github.com/notifications/unsubscribe-auth/ABZEXLUIGSGBNNZHFP3FJLDQRZAWJANCNFSM4JH6HNSA > > . > — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#6592?email_source=notifications&email_token=AKW5NZYJLZDYTZNDNZE7K4LQSA7TVA5CNFSM4JH6HNSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC7WLGI#issuecomment-549414297>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKW5NZ63OLXN3VHDDHSUHS3QSA7TVANCNFSM4JH6HNSA> .

[steviepubliclab]

Ok, I'm not sure then. I still see it. If it's helpful, I'm using chrome on a desktop. On Mon, Nov 4, 2019 at 10:49 AM nicoleiocana <notifications@github.com> wrote:

…

Yes. I just clicked on the link on my mobile, logged in, then refreshed the page. The page still doesn't show the "New Post +" button. On Mon, Nov 4, 2019, 7:46 AM Stevie ***@***.***> wrote: > Are you logged into a Public Lab account? You might not see the button if > you're not logged in. > > On Sat, Nov 2, 2019 at 11:30 PM nicoleiocana ***@***.***> > wrote: > > > Can you offer some insight as to how you replicated the error? When I go > > to the url you provided: > https://publiclab.org/tag/oil-and-gas-water-trio > > <http://url> my screen does how have the "New Post + " button. It shows > > this: > > > > [image: publiclab_url] > > < > https://user-images.githubusercontent.com/44947175/68079960-8f7dc800-fdaf-11e9-9d07-4d3b88fa3db6.PNG > > > > > > — > > You are receiving this because you authored the thread. > > Reply to this email directly, view it on GitHub > > < > #6592?email_source=notifications&email_token=ABZEXLQC7GPSK6FDLWIXHX3QRZAWJA5CNFSM4JH6HNSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC5KDLA#issuecomment-549101996 > >, > > or unsubscribe > > < > https://github.com/notifications/unsubscribe-auth/ABZEXLUIGSGBNNZHFP3FJLDQRZAWJANCNFSM4JH6HNSA > > > > . > > > > — > You are receiving this because you commented. > Reply to this email directly, view it on GitHub > < #6592?email_source=notifications&email_token=AKW5NZYJLZDYTZNDNZE7K4LQSA7TVA5CNFSM4JH6HNSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC7WLGI#issuecomment-549414297 >, > or unsubscribe > < https://github.com/notifications/unsubscribe-auth/AKW5NZ63OLXN3VHDDHSUHS3QSA7TVANCNFSM4JH6HNSA > > . > — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6592?email_source=notifications&email_token=ABZEXLX5327ZUUCEJG3C33LQSBACNA5CNFSM4JH6HNSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC7WZGQ#issuecomment-549416090>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABZEXLU7TZPK3ZJ6NYF4GSLQSBACNANCNFSM4JH6HNSA> .

[jywarren]

Hi all - you can see the "New Post" button only if you are following the tag. So, if you press Follow then reload the original page, you should see it. The New post button should have the params "/post?tags=oil-and-gas-fellowship-trio" -- maybe that's not set up right? Thanks!

…

On Mon, Nov 4, 2019 at 11:14 AM Stevie ***@***.***> wrote: Ok, I'm not sure then. I still see it. If it's helpful, I'm using chrome on a desktop. On Mon, Nov 4, 2019 at 10:49 AM nicoleiocana ***@***.***> wrote: > Yes. I just clicked on the link on my mobile, logged in, then refreshed the > page. The page still doesn't show the "New Post +" button. > > On Mon, Nov 4, 2019, 7:46 AM Stevie ***@***.***> wrote: > > > Are you logged into a Public Lab account? You might not see the button if > > you're not logged in. > > > > On Sat, Nov 2, 2019 at 11:30 PM nicoleiocana ***@***.*** > > > wrote: > > > > > Can you offer some insight as to how you replicated the error? When I > go > > > to the url you provided: > > https://publiclab.org/tag/oil-and-gas-water-trio > > > <http://url> my screen does how have the "New Post + " button. It > shows > > > this: > > > > > > [image: publiclab_url] > > > < > > > https://user-images.githubusercontent.com/44947175/68079960-8f7dc800-fdaf-11e9-9d07-4d3b88fa3db6.PNG > > > > > > > > > — > > > You are receiving this because you authored the thread. > > > Reply to this email directly, view it on GitHub > > > < > > > #6592?email_source=notifications&email_token=ABZEXLQC7GPSK6FDLWIXHX3QRZAWJA5CNFSM4JH6HNSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC5KDLA#issuecomment-549101996 > > >, > > > or unsubscribe > > > < > > > https://github.com/notifications/unsubscribe-auth/ABZEXLUIGSGBNNZHFP3FJLDQRZAWJANCNFSM4JH6HNSA > > > > > > . > > > > > > > — > > You are receiving this because you commented. > > Reply to this email directly, view it on GitHub > > < > #6592?email_source=notifications&email_token=AKW5NZYJLZDYTZNDNZE7K4LQSA7TVA5CNFSM4JH6HNSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC7WLGI#issuecomment-549414297 > >, > > or unsubscribe > > < > https://github.com/notifications/unsubscribe-auth/AKW5NZ63OLXN3VHDDHSUHS3QSA7TVANCNFSM4JH6HNSA > > > > . > > > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > < #6592?email_source=notifications&email_token=ABZEXLX5327ZUUCEJG3C33LQSBACNA5CNFSM4JH6HNSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC7WZGQ#issuecomment-549416090 >, > or unsubscribe > < https://github.com/notifications/unsubscribe-auth/ABZEXLU7TZPK3ZJ6NYF4GSLQSBACNANCNFSM4JH6HNSA > > . > — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#6592?email_source=notifications&email_token=AAAF6JYLKBCHJSX3KSCVDELQSBC3DA5CNFSM4JH6HNSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC7ZONQ#issuecomment-549426998>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6J2BYVPWT4HFGS6LKJDQSBC3DANCNFSM4JH6HNSA> .

[jywarren]

This is suffering from the same problem as #6675! I've put a solution there, and this would make a great FTO!

Short version: we should replace tag.name with params[:id]!

---

This has been marked as a good candidate for becoming a first-timers-only issue like these, meaning that it's simple, self-contained, and with some extra formatting, could be a great entry point for a new contributor. If you're familiar enough with this code, please consider reformatting or reposting it as a first-timers-only issue, and then ping @publiclab/reviewers to get it labelled. Or, if this is not your first time, try to solve it yourself!

[nicoleiocana]

Can this be assigned to me? I am currently working on the fix, except to simplify the code, I:

replaced tag.name with @title since it is referenced in the if-statement.

This fix passed my first test (the current issue #6592). I am now testing to see if my refactored code passes the following:

Since we didn't use tag.name and there are no references to the @tag instance variable in the show.html.erb file, I am certain that we do not need the @tag in the tag_controller.rb.

Let me know if you will allow me to go ahead with my fix (or have any criticism) please? Thanks in advance.

[jywarren]

Hi, `@title` may not be the correct capitalization, i think... and it was originally intended to be used as the page title. Would you mind switching to `params[:id]`? I would like to remove @tag in the tag controller but there may be other places it's used. Can we do that in a separate PR so as not to mix that change up with this one? Thanks! We'd love to have your help, thanks a lot!

…

On Mon, Nov 4, 2019 at 2:01 PM nicoleiocana ***@***.***> wrote: Can this be assigned to me? I am currently working on the fix, except to simplify the code, I: replaced tag.name with @title <https://github.com/title> since it is referenced in the if-statement. This fix passed my first test (the current issue #6592 <#6592>). I am now testing to see if my refactored code passes the following: Since we didn't use tag.name and there are no references to the @tag <https://github.com/tag> instance variable in the show.html.erb file, I am certain that we do not need the @tag <https://github.com/tag> in the tag_controller.rb. Let me know if you will allow me to go ahead with my fix (or have any criticism) please? Thanks in advance. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#6592?email_source=notifications&email_token=AAAF6J6OG3AIJEV26POSNZLQSBWPPA5CNFSM4JH6HNSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDAK6ZQ#issuecomment-549498726>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6JZPRPLYJYFPR6OVBHLQSBWPPANCNFSM4JH6HNSA> .

[nicoleiocana]

Hello. The @title = params[:id] from the tag_controller.rb in the show method (sorry, I should've specified this) returns the exact tag name string, so it is the correct capitalization (or not depending on the tag name). Therefore, regarding only the show method, it would make sense to delete the @tag = Tag.find_by(name: params[:id]) in the tag_controller.rb because:

1. It was returning nil; hence, why were seeing in this issue and
2. because the @tag instance variable was nowhere in the show.html.erb file.

And yeah, in the #6675 issue, I will refactor the code and then test the file where @tag is deleted from only the show method. It already worked for this isssue (and I already submitted a PR), so I am confident it will work in the other issue.

[nicoleiocana]

@jywarren
You were totally right. Deleting the @tag instance variable didn't pass the tests. I kept it, only added the @title in the show.html.erb file for the "New Post +" issue and submitted that PR. Thanks for the insight, I appreciate it.

I'll make a PR for #6675 and refactor the code; in particular, editing out all of the tag.name's and replacing them with @title's.

[jywarren]

This sounds good, however I don't want people to begin assuming @title means tagname here, as that is not true for the rest of the codebase. I'd really appreciate if you could use params[:id] as that is more generic and will create less confusion, and if you'd like, we can develop a new naming scheme that'll make even more sense in a separate PR. My other concern is that I'd like to be sure any refactoring of the @title variable (for setting HTML page title) won't be entangled with this section of code - @title should primarily be used in our /app/views/layouts/ files. I hope this makes sense! I'm for a more readable usage of variable names, i just don't want to do it all within this PR, so that we keep each PR to a single purpose. Thanks so much!

…

On Mon, Nov 4, 2019 at 3:24 PM nicoleiocana ***@***.***> wrote: @jywarren <https://github.com/jywarren> You were totally right. Deleting the @tag <https://github.com/tag> instance variable didn't pass the tests. I kept it, only added the @title in the show.html.erb file for the "New Post +" issue and submitted that PR. Thanks for the insight, I appreciate it. I'll make a PR for #6675 <#6675> and refactor the code; in particular, editing out all of the tag.name's and replacing them with @title's. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6592?email_source=notifications&email_token=AAAF6J5LBEKHLW565YZUBATQSCAD7A5CNFSM4JH6HNSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDASYYY#issuecomment-549530723>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6J4SBQGLH4DP7XUBP73QSCAD7ANCNFSM4JH6HNSA> .

[nicoleiocana]

I completely agree with you on all points. Let me break down my logic to see if it is parallel with your notions:

The confusion occurs for @title when the original code for the show.html.erb in the tag folder utilizes here:
<h1 style="margin-top:6px;"><%= @title %></h1>
versus
<% if current_user && current_user.following(@title) %>
Like you mentioned, we should keep the logic of the header title different from the parameter passed in from the previous link. Therefore, all of the code referring to the parameter passed in with be changed to params[:id], all references to the tag name as the title in sections of the page will remain @title. This solution will be solved in a PR for issue #6675.

[jywarren]

Yes, that's exactly right! Thank you so much for working through this! I also think this applies for #6677, what do you think?

[nicoleiocana]

Yes, you are right. Since I submitted a PR for the similar issue more recent than this one, my comment above applies as of now:

the code referring to the parameter passed in with be changed to params[:id], all references to the tag name as the title in sections of the page will remain @title.

In particular, the @title instance variable only refers to the HTML headers where applicable.

You are very much welcome. =)