Plots2: Posting from tag pages not tagging properly
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?
steviepubliclab
All 14 comments
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:
nicoleiocana
on 3 Nov 2019
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 notifications@github.com
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
https://github.com/publiclab/plots2/issues/6592?email_source=notifications&email_token=ABZEXLQC7GPSK6FDLWIXHX3QRZAWJA5CNFSM4JH6HNSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC5KDLA#issuecomment-549101996,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ABZEXLUIGSGBNNZHFP3FJLDQRZAWJANCNFSM4JH6HNSA
.
steviepubliclab
on 4 Nov 2019
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 notifications@github.com 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 notifications@github.com
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
<
https://github.com/publiclab/plots2/issues/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
https://github.com/publiclab/plots2/issues/6592?email_source=notifications&email_token=AKW5NZYJLZDYTZNDNZE7K4LQSA7TVA5CNFSM4JH6HNSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC7WLGI#issuecomment-549414297,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AKW5NZ63OLXN3VHDDHSUHS3QSA7TVANCNFSM4JH6HNSA
.
nicoleiocana
on 4 Nov 2019
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 notifications@github.com 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 notifications@github.com
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
<,
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
<
https://github.com/publiclab/plots2/issues/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
https://github.com/publiclab/plots2/issues/6592?email_source=notifications&email_token=ABZEXLX5327ZUUCEJG3C33LQSBACNA5CNFSM4JH6HNSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC7WZGQ#issuecomment-549416090,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ABZEXLU7TZPK3ZJ6NYF4GSLQSBACNANCNFSM4JH6HNSA
.
steviepubliclab
on 4 Nov 2019
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 notifications@github.com 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 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 notifications@github.com 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 <[email protected]
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
<,
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
<,
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
<
https://github.com/publiclab/plots2/issues/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
https://github.com/publiclab/plots2/issues/6592?email_source=notifications&email_token=AAAF6JYLKBCHJSX3KSCVDELQSBC3DA5CNFSM4JH6HNSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC7ZONQ#issuecomment-549426998,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAAF6J2BYVPWT4HFGS6LKJDQSBC3DANCNFSM4JH6HNSA
.
jywarren
on 4 Nov 2019
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!
jywarren
on 4 Nov 2019
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.
nicoleiocana
on 4 Nov 2019
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 notifications@github.com
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
https://github.com/publiclab/plots2/issues/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
https://github.com/publiclab/plots2/issues/6592?email_source=notifications&email_token=AAAF6J6OG3AIJEV26POSNZLQSBWPPA5CNFSM4JH6HNSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDAK6ZQ#issuecomment-549498726,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAAF6JZPRPLYJYFPR6OVBHLQSBWPPANCNFSM4JH6HNSA
.
jywarren
on 4 Nov 2019
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:
- It was returning nil; hence, why were seeing
in this issue and - 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
on 4 Nov 2019
@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.
nicoleiocana
on 4 Nov 2019
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 notifications@github.com
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 https://github.com/publiclab/plots2/issues/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
https://github.com/publiclab/plots2/issues/6592?email_source=notifications&email_token=AAAF6J5LBEKHLW565YZUBATQSCAD7A5CNFSM4JH6HNSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDASYYY#issuecomment-549530723,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAAF6J4SBQGLH4DP7XUBP73QSCAD7ANCNFSM4JH6HNSA
.
jywarren
on 4 Nov 2019
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.
nicoleiocana
on 5 Nov 2019
Yes, that's exactly right! Thank you so much for working through this! I also think this applies for https://github.com/publiclab/plots2/pull/6677, what do you think?
jywarren
on 5 Nov 2019
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. =)
nicoleiocana
on 5 Nov 2019
Related issues
milaaraujo
·
3Comments
jywarren
·
3Comments
grvsachdeva
·
3Comments
grvsachdeva
·
3Comments
milaaraujo
·
3Comments
Most helpful comment
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.