Plots2: Refactoring profile.html.erb code pt. 3

[sashadev-sky]

The problem

Our code in app/views/users/profile.html.erb is very verbose... we could use some refactoring

We can start by refactoring out a reoccuring line from the code. For now, the 1st line of:

https://github.com/publiclab/plots2/blob/1931439c0c799b3e3420a9dcaace7a54cf895bd4/app/views/users/profile.html.erb#L237-L242

Solution

What to change it to:

Code that we want to refactor and that might be useful to multiple views in the future can go in the app/controllers/application_controller.rb

1)

Try adding a current_profile_user method to the application controller. We can move the conditional to check if there is a current_user and if the current_user is the user who owns the current profile we are viewing with a method like so:

  def current_profile_user
    return @current_user && @profile_user == @current_user
  end

2)

Add your new current_profile_user method to the helper_methods at the top of the page so that it is available to all our views:

https://github.com/publiclab/plots2/blob/1931439c0c799b3e3420a9dcaace7a54cf895bd4/app/controllers/application_controller.rb#L6

3)

Refactor the lines in the 3 code blocks found in the "The Problem" section to use your new helper method instead.

Note that the 1st code block has an additional check for current_user.can_moderate? in it which we need to add on.

For ex., that line can be updated to <% if current_profile_user && current_user.can_moderate? %>

Thanks!!

Steps to Fix

• [ ] Claim this issue with a comment here, below, and ask any clarifying questions you need
• [ ] Fork the repository and set it up locally following the main repo README instructions https://github.com/publiclab/plots2
  
  
  
  • [ ] Create a new feature branch with a unique name descriptive to the issue
  
  
• [ ] Try to fix the issue following the steps above, but even before you're done, you can:
  commit your changes to your branch 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
• [ ] Reference this issue in your pull request body
• [ ] Once you submit your pull request, an additional checklist will be provided for getting it merged

💬 Get help

If you need any help - here are some options:

• Comment below
• Join our gitter chat at https://gitter.im/publiclab/publiclab

Next Steps

When you're done and the code is merged, if you see other places in the code that could use this refactoring, open up another first-timers-only issue and walk a new contributor through it :)

Note related issues #4342 and #4341

[oorjitchowdhary]

@sashadev-sky I don't think this should be a first-timers-only issue... First timers issues are meant to welcome people and get them familiar with the code base... Sorry if I'm wrong

[sashadev-sky]

@oorjitchowdhary I have been getting mixed reviews so I will wait for @jywarren @gauravano to review. I can rewrite it based on the tags they add!

But I partially agree with you it's on the fence. Maybe I can make it even simpler

[SidharthBansal]

@sashadev-sky this is properly formatted but I am afraid whether a newcomer can deal with such huge PR or not. We want the first experience with Public Lab will be friendly and very less time-consuming. Collaborators should not be dishearted with any errors or bugs they would get in the PR. I agree with @oorjitchowdhary.
Thanks @sashadev-sky for your hard work. We really appreciate it. Please try to make first timers which have less amount of work.
Let's create this one as help-wanted issue.

[sashadev-sky]

done! @SidharthBansal @oorjitchowdhary
Should we delete our comments so that other users won't assume this issue has been taken?

[grvsachdeva]

We can also divide this issue in 3 FTO's, corresponding to 3 blocks as explained by @sashadev-sky. What do you think @sashadev-sky, want to open 3 FTO's?

Nice work :balloon: :smiley: !

[sashadev-sky]

@gauravano Sure! What if I leave the first one that has the extra conditional as a help wanted, and make the two simpler ones FTOs?

[grvsachdeva]

Yes, sounds good!

[grvsachdeva]

Also, both FTO's would depend on 1st block, so I think either we can wait for someone to claim this one or we could add a function ourselves. Thoughs?

[sashadev-sky]

@gauravano you are very right. I can just solve this one myself right now, then rewrite the other two.

[grvsachdeva]

Looking for your PR :smiley:

[sashadev-sky]

@gauravano just making sure you saw the PR #4343 I tagged you in

[grvsachdeva]

Sorry, i am away from lappy right now. I would review in some hours.

Gaurav Sachdeva

On Thu 20 Dec, 2018, 5:06 AM Sasha Boginsky <[email protected] wrote:

@gauravano https://github.com/gauravano just making sure you saw the PR

4343 https://github.com/publiclab/plots2/pull/4343 I tagged you in

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/plots2/issues/4339#issuecomment-448790467,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AT6S9ty76Vr2iI9rS3D0lrs5LWpZubk9ks5u6s2WgaJpZM4ZaqHH
.