Skip to content

Commit

Permalink
Allow cancelling a comment vote (#5042)
Browse files Browse the repository at this point in the history
#### 🎩 What? Why?
Allows cancelling a comment vote (up or down) in case the user has already voted the comment. This is important regarding the UX to prevent users from locking down their opinion due to misclicks and producing unnecessary visibility (positive or negative) for the comment.

This is done by allowing the user to to re-click the voting button after the voting. Clicking it for a second time cancels the vote.

#### 📌 Related Issues
- Fixes #5033

#### 📋 Subtasks
- [x] Add `CHANGELOG` entry
- [X] Add tests
  • Loading branch information
ahukkanen authored and mrcasals committed Apr 3, 2019
1 parent 22ef676 commit 2787f79
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 24 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -10,6 +10,8 @@

**Changed**:

- **decidim-comments**: Allow cancelling a vote on a comment. [#5042](https://github.com/decidim/decidim/pull/5042)

**Fixed**:

- **decidim-core**: Fix amendments forms: show error messages and render hashtags. [#4951](https://github.com/decidim/decidim/pull/4951)
Expand Down

Large diffs are not rendered by default.

14 changes: 12 additions & 2 deletions decidim-comments/app/commands/decidim/comments/vote_comment.rb
Expand Up @@ -24,9 +24,19 @@ def initialize(comment, author, options = { weight: 1 })
# Returns nothing.
def call
if @weight == 1
@comment.up_votes.create!(author: @author)
vote = @comment.up_votes.find_by(author: @author)
if vote
vote.destroy!
else
@comment.up_votes.create!(author: @author)
end
elsif @weight == -1
@comment.down_votes.create!(author: @author)
vote = @comment.down_votes.find_by(author: @author)
if vote
vote.destroy!
else
@comment.down_votes.create!(author: @author)
end
else
return broadcast(:invalid)
end
Expand Down
Expand Up @@ -34,16 +34,16 @@ describe("<DownVoteButton />", () => {
expect(wrapper.find(VoteButton).prop("votes")).toEqual(comment.downVotes);
});

it("should pass disabled prop as true if comment downVoted is true", () => {
comment.downVoted = true;
it("should pass disabled prop as false if comment upVoted is true", () => {
comment.upVoted = true;
const wrapper = shallow(<DownVoteButton session={session} comment={comment} downVote={downVote} rootCommentable={rootCommentable} orderBy={orderBy} />);
expect(wrapper.find(VoteButton).prop("disabled")).toBeTruthy();
expect(wrapper.find(VoteButton).prop("disabled")).toBeFalsy();
});

it("should pass disabled prop as true if comment downVoted is true", () => {
it("should pass disabled prop as false if comment downVoted is true", () => {
comment.downVoted = true;
const wrapper = shallow(<DownVoteButton session={session} comment={comment} downVote={downVote} rootCommentable={rootCommentable} orderBy={orderBy} />);
expect(wrapper.find(VoteButton).prop("disabled")).toBeTruthy();
expect(wrapper.find(VoteButton).prop("disabled")).toBeFalsy();
});

describe("when session is not present", () => {
Expand Down
Expand Up @@ -36,7 +36,7 @@ export const DownVoteButton: React.SFC<DownVoteButtonProps> = ({
}

const userLoggedIn = session && session.user;
const disabled = upVoted || downVoted;
const disabled = false;

return (
<VoteButton
Expand Down Expand Up @@ -67,7 +67,7 @@ const DownVoteButtonWithMutation = graphql<DownVoteMutation, DownVoteButtonProps
downVote: {
__typename: "Comment",
...ownProps.comment,
downVotes: ownProps.comment.downVotes + 1,
downVotes: ownProps.comment.downVotes + (ownProps.comment.downVoted ? -1 : 1),
downVoted: true
}
}
Expand Down
Expand Up @@ -34,16 +34,16 @@ describe("<UpVoteButton />", () => {
expect(wrapper.find(VoteButton).prop("votes")).toEqual(comment.upVotes);
});

it("should pass disabled prop as true if comment upVoted is true", () => {
it("should pass disabled prop as false if comment upVoted is true", () => {
comment.upVoted = true;
const wrapper = shallow(<UpVoteButton session={session} comment={comment} upVote={upVote} rootCommentable={rootCommentable} orderBy={orderBy} />);
expect(wrapper.find(VoteButton).prop("disabled")).toBeTruthy();
expect(wrapper.find(VoteButton).prop("disabled")).toBeFalsy();
});

it("should pass disabled prop as true if comment downVoted is true", () => {
it("should pass disabled prop as false if comment downVoted is true", () => {
comment.downVoted = true;
const wrapper = shallow(<UpVoteButton session={session} comment={comment} upVote={upVote} rootCommentable={rootCommentable} orderBy={orderBy} />);
expect(wrapper.find(VoteButton).prop("disabled")).toBeTruthy();
expect(wrapper.find(VoteButton).prop("disabled")).toBeFalsy();
});

describe("when session is not present", () => {
Expand Down
Expand Up @@ -36,7 +36,7 @@ export const UpVoteButton: React.SFC<UpVoteButtonProps> = ({
}

const userLoggedIn = session && session.user;
const disabled = upVoted || downVoted;
const disabled = false;

return (
<VoteButton
Expand Down Expand Up @@ -67,7 +67,7 @@ const UpVoteButtonWithMutation = graphql<UpVoteMutation, UpVoteButtonProps>(upVo
upVote: {
__typename: "Comment",
...ownProps.comment,
upVotes: ownProps.comment.upVotes + 1,
upVotes: ownProps.comment.upVotes + (ownProps.comment.upVoted ? -1 : 1),
upVoted: true
}
}
Expand Down
16 changes: 8 additions & 8 deletions decidim-comments/spec/commands/vote_comment_spec.rb
Expand Up @@ -33,14 +33,14 @@ module Comments
comment.up_votes.create!(author: author)
end

it "broadcasts invalid" do
expect { command.call }.to broadcast(:invalid)
it "broadcasts ok" do
expect { command.call }.to broadcast(:ok)
end

it "doesn't create a comment vote" do
it "removes the comment vote from this user" do
expect do
command.call
end.not_to change(CommentVote, :count)
end.to change(CommentVote, :count).by(-1)
end
end

Expand Down Expand Up @@ -79,14 +79,14 @@ module Comments
comment.down_votes.create!(author: author)
end

it "broadcasts invalid" do
expect { command.call }.to broadcast(:invalid)
it "broadcasts ok" do
expect { command.call }.to broadcast(:ok)
end

it "doesn't create a comment vote" do
it "removes the comment vote from this user" do
expect do
command.call
end.not_to change(CommentVote, :count)
end.to change(CommentVote, :count).by(-1)
end
end

Expand Down

0 comments on commit 2787f79

Please sign in to comment.