Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

removed test-digest-button for Users other than Admins and Moderators #4293

Merged
merged 12 commits into from Dec 16, 2018
Merged

removed test-digest-button for Users other than Admins and Moderators #4293

Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
7f5481b
removed test-digest-button from production environment
uzorjchibuzor Dec 13, 2018
0e33a8a
added space between conditionals
uzorjchibuzor Dec 13, 2018
08e5c6d
improved code readability
uzorjchibuzor Dec 13, 2018
4d23f98
indented tags
uzorjchibuzor Dec 13, 2018
2f91a6f
corrected error in conditional statement
uzorjchibuzor Dec 13, 2018
b7101ba
restricted the Test-Digest-Button to be viewed by Admins and Mods onl…
uzorjchibuzor Dec 13, 2018
cb8081a
changed the style of the test button to btn-primary like all the othe…
uzorjchibuzor Dec 13, 2018
b7d3c61
added a space between conditionals and tags
uzorjchibuzor Dec 13, 2018
27cc723
removed redundant if statement
uzorjchibuzor Dec 14, 2018
1a97d90
adding && current_user.can_moderate?
uzorjchibuzor Dec 14, 2018
25f53e7
removed extra spaces to make code cleaner
uzorjchibuzor Dec 16, 2018
3ad01a5
Button size changed
grvsachdeva Dec 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 8 additions & 3 deletions app/views/users/profile.html.erb
View file
Open in desktop
Expand Up @@ -233,11 +233,16 @@
</div>
</div>
<% end %>
<% if current_user.can_moderate? || current_user.role == "admin" %>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. can_moderate method checks for both admin and moderator
  2. You are directly calling current_user.mod.. and current_user.role what if there is no user currently logged in, then current_user would be containing nil. This is reason for test failure
  3. This if condition can be removed, how about adding && current_user.can_moderate? at below if where we are checking profile_user and current_user

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification. I am going make changes. I am happy I got to work on this issue

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@uzorjchibuzor let me know if you need any help! I will be available over the next few hours

Also I have one more change I think you should add:

  • You should move the line that checks if there is a current_user above the one that checks if the current_user is a moderator or admin. Otherwise, if there is not a current_user then the 1st method will throw an error


<% if @profile_user && current_user && @profile_user == current_user %>

<%= form_tag "/users/test_digest_email", method: :post do %>
<%= submit_tag "Test Digest Email", class: "btn btn-primary" %>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<%= submit_tag "Test Digest Email", class: "btn btn-primary" %>
<%= submit_tag "Test Digest Email", class: "btn btn-secondary" %>

As secondary would look little lighter. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the btn-secondary bootstrap class is grayish, but the btn-info in a bit lighter than the primary. I have gone with that.

<% end %>

<% if @profile_user && current_user && @profile_user == current_user %>
<%= form_tag "/users/test_digest_email", method: :post do %>
<%= submit_tag "Test Digest Email" %>
<% end %>

<% end %>
sashadev-sky marked this conversation as resolved.
Show resolved Hide resolved

<hr />
Expand Down