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

User page: new following section #1750

Merged
merged 17 commits into from
Jul 24, 2017
Merged

User page: new following section #1750

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
52e195d
Add tab. Add accordion. Add style. Pass test scss-lint.
agileontheweb Jun 27, 2017
7e2e94d
Add following tab to user public page.
taitus Jun 30, 2017
556e259
Remove margin bottom in the table inside accordion. Divided into grou…
agileontheweb Jul 3, 2017
b0c571e
Do not display send notification button when proposal is from another…
taitus Jul 13, 2017
cef67c5
Increased test coverage to Following user public page.
taitus Jul 13, 2017
36f9836
Use inflector methods instead of manual replacements.
Senen Jul 18, 2017
249641f
Simpler code to get followable object icon.
Senen Jul 18, 2017
16987e7
Easier conditional usage.
Senen Jul 18, 2017
57c8887
Add method to follows helper to get followable partial name and argum…
Senen Jul 18, 2017
b7ef755
minor refactoring for follows
voodoorai2000 Jul 19, 2017
3be893b
Merge pull request #12 from consul/follows-refactoring
Senen Jul 19, 2017
cba497b
Create new followables helper to have some methods previously on foll…
Senen Jul 19, 2017
c9a6630
Remove popup on follow buttons and remove uneeded code.
Senen Jul 19, 2017
119a6b3
Remove where clause from followable button partial and rename followa…
Senen Jul 19, 2017
3b13350
Remove uneeded initialization after ajax refresh.
Senen Jul 19, 2017
dec7d2d
Add followable specific notices to follows controller actions and ren…
Senen Jul 19, 2017
b7bce2e
Remove complex methods from follow model. Create instance method foll…
Senen Jul 19, 2017
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
Prev Previous commit
Next Next commit
Do not display send notification button when proposal is from another… 
… author.
  • Loading branch information
@taitus @Senen
taitus authored and Senen committed Jul 13, 2017
commit b0c571e658e6e4c293e6a58e57ae0af1bb2d9062
9 changes: 2 additions & 7 deletions app/controllers/users_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ class UsersController < ApplicationController
load_and_authorize_resource
helper_method :author?
helper_method :valid_interests_access?
helper_method :author_or_admin?
Copy link
Collaborator

Choose a reason for hiding this comment

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

I can't really understand the reason to remove author_or_admin? method and its usage, can you explain a bit further? :)

Copy link
Member

Choose a reason for hiding this comment

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

@bertocq The main reason is that apparently it was not being used anywhere in the application.

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 you're right, I did a project search and no use, although sometimes looking for the method string is not enough if you do compose de method name with a string interpolation like send("#{this}_or_#{that}") but that's not the case I guess based on the specs passing ^_^

Nice catch & cleanup 👏 😄


def show
load_filtered_activity if valid_access?
Expand Down Expand Up @@ -81,12 +80,8 @@ def valid_interests_access?
@user.public_interests || authorized_current_user?
end

def author?
@author ||= current_user && (current_user == @user)
end

def author_or_admin?
@author_or_admin ||= current_user && (author? || current_user.administrator?)
def author?(proposal)
proposal.author_id == current_user.id
end

def authorized_current_user?
Expand Down
2 changes: 1 addition & 1 deletion app/models/abilities/common.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def initialize(user)
can :create, Budget::Investment, budget: { phase: "accepting" }
can :suggest, Budget::Investment, budget: { phase: "accepting" }
can :destroy, Budget::Investment, budget: { phase: ["accepting", "reviewing"] }, author_id: user.id
can :vote, Budget::Investment, budget: { phase: "selecting" }
can :vote, Budget::Investment, budget: { phase: "selecting" }
Copy link
Collaborator

Choose a reason for hiding this comment

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

^_^ /bertocq likes this

can [:show, :create], Budget::Ballot, budget: { phase: "balloting" }
can [:create, :destroy], Budget::Ballot::Line, budget: { phase: "balloting" }

Expand Down
4 changes: 2 additions & 2 deletions app/views/users/_proposal.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@
<%= proposal.summary %>
</td>

<% if can? :retire_form, proposal %>
<% if author?(proposal) %>
<td class="text-center">
<%= link_to t("users.proposals.send_notification"),
new_proposal_notification_path(proposal_id: proposal.id),
class: 'button hollow' %>
</td>
<% end %>

<% if can? :new, ProposalNotification, author_id: proposal.author_id %>
<% if author?(proposal) || proposal.retired? %>
Copy link
Collaborator

@bertocq bertocq Jul 17, 2017 •
edited

Choose a reason for hiding this comment

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

It took me a bit to understand all the conditionals around this block and realize there is only 2 scenarios. Maybe this could be easier to read:

<% if proposal.retired? %>
 <td class="text-center">
  <span class="label alert"><%= t('users.proposals.retired') %></span>
 </td>
<% elsif author?(proposal) %>
 <td class="text-center">
   <%= link_to t('users.proposals.retire'),
               retire_form_proposal_path(proposal),
               class: 'button hollow alert' %>
 </td>
<% end %>

Copy link
Member Author

Choose a reason for hiding this comment

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

Totally agree @bertocq! We will make this change too!

<td class="text-center">
<% if proposal.retired? %>
<span class="label alert"><%= t('users.proposals.retired') %></span>
Expand Down