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:

plots2/app/views/users/profile.html.erb

Lines 237 to 242 in 1931439

	<% if @profile_user && current_user && current_user.can_moderate? && @profile_user == current_user %>
	<br />
	<%= form_tag "/users/test_digest_email", method: :post do %>
	<%= submit_tag "Test Digest Email", class: "btn btn-info", style: "width: 100%;" %>
	<% end %>
	<% end %>

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

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

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:

plots2/app/controllers/application_controller.rb

Line 6 in 1931439

helper_method :current_user_session, :current_user, :prompt_login, :sidebar

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 🎈 😃 !

[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 😃

[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 ***@***.*** wrote: @gauravano <https://github.com/gauravano> just making sure you saw the PR #4343 <#4343> I tagged you in — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4339 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AT6S9ty76Vr2iI9rS3D0lrs5LWpZubk9ks5u6s2WgaJpZM4ZaqHH> .