votes: fixup various minor issues.

* Add a gap between thumbnails on mobile.
* Adjust CSS for scores and vote buttons.
* Include "Private favorites" as an incentive on the user upgrade page.
* Fix vote buttons not being visible beneath thumbnails on mobile.
* Fix the "Show scores" link not preserving the current page number.
* Fix vote buttons being unintentionally enabled for all thumbnails by default.
* Fix banned and restricted users being able to favorite posts by
  tagging them with `fav:self`.
* Fix search engines being able to crawl /posts?view=score pages.
* Fix broken tests.
This commit is contained in:
evazion
2021-11-19 22:40:34 -06:00
parent 3ae62d08eb
commit eda23c719a
20 changed files with 108 additions and 94 deletions

View File

@@ -6,8 +6,3 @@
max-width: 160px; max-width: 160px;
} }
} }
span.post-favcount a {
color: var(--text-color);
&:hover { text-decoration: underline; }
}

View File

@@ -9,7 +9,7 @@ class PostPreviewComponent < ApplicationComponent
delegate :image_width, :image_height, :file_ext, :file_size, :duration, :is_animated?, to: :media_asset delegate :image_width, :image_height, :file_ext, :file_size, :duration, :is_animated?, to: :media_asset
delegate :media_asset, to: :post delegate :media_asset, to: :post
def initialize(post:, tags: "", show_deleted: false, show_cropped: true, show_votes: true, link_target: post, pool: nil, similarity: nil, recommended: nil, compact: nil, size: nil, current_user: CurrentUser.user, **options) def initialize(post:, tags: "", show_deleted: false, show_cropped: true, show_votes: false, link_target: post, pool: nil, similarity: nil, recommended: nil, compact: nil, size: nil, current_user: CurrentUser.user, **options)
super super
@post = post @post = post
@tags = tags.presence @tags = tags.presence

View File

@@ -55,8 +55,8 @@
<% end %> <% end %>
</p> </p>
<% elsif show_votes -%> <% elsif show_votes -%>
<p class="desc"> <div class="post-preview-score text-sm mt-1">
<%= render_post_votes post, current_user: current_user %> <%= render_post_votes post, current_user: current_user %>
</p> </div>
<% end -%> <% end -%>
<% end -%> <% end -%>

View File

@@ -1,12 +1,10 @@
@import "../../javascript/src/styles/base/000_vars.scss"; @import "../../javascript/src/styles/base/000_vars.scss";
article.post-preview { article.post-preview {
width: 154px;
margin: 0 10px 10px 0;
text-align: center; text-align: center;
display: inline-block; display: inline-block;
position: relative; position: relative;
vertical-align: top; overflow: hidden;
a { a {
display: inline-block; display: inline-block;
@@ -119,38 +117,16 @@ body[data-current-user-can-approve-posts="true"] .post-preview {
} }
} }
@media screen and (max-width: 660px) { @media screen and (min-width: 660px) {
article.post-preview { article.post-preview {
margin: 0; width: 154px;
text-align: center; margin: 0 10px 10px 0;
vertical-align: middle; vertical-align: top;
display: inline-block; }
}
a {
margin: 0 auto; @media screen and (max-width: 660px) {
} article.post-preview img {
border: none !important;
img {
max-width: 33.3vw;
max-height: 33.3vw;
border: none !important;
}
}
.user-disable-cropped-false {
article.post-preview {
width: 33.3%;
height: 33.3vw;
overflow: hidden;
}
img {
width: 33.3vw;
height: 33.3vw;
&.has-cropped-false {
object-fit: cover;
}
}
} }
} }

View File

@@ -1,3 +1,5 @@
@import "../../javascript/src/styles/base/000_vars.scss";
.post-votes { .post-votes {
// Fix it so that the vote buttons don't move when the score changes width. // Fix it so that the vote buttons don't move when the score changes width.
// XXX duplicated from app/components/comment_component/comment_component.scss // XXX duplicated from app/components/comment_component/comment_component.scss
@@ -7,10 +9,11 @@
min-width: 1.25em; min-width: 1.25em;
white-space: nowrap; white-space: nowrap;
vertical-align: middle; vertical-align: middle;
}
a { }
color: var(--text-color);
&:hover { text-decoration: underline; } .posts-container {
} .post-score a {
@include inactive-link;
} }
} }

View File

@@ -1,6 +1,6 @@
<div class="post-votes-tooltip thin-scrollbar"> <div class="post-votes-tooltip thin-scrollbar">
<div class="text-center text-muted"> <div class="text-center text-muted whitespace-nowrap">
+<%= post.up_score %> / <%= post.down_score %> <%= upvote_ratio %> +<%= post.up_score %> / -<%= post.down_score.abs %> <%= upvote_ratio %>
</div> </div>
<div class="post-voters"> <div class="post-voters">

View File

@@ -28,6 +28,8 @@ $spacer: 0.25rem; /* 4px */
white-space: nowrap; white-space: nowrap;
} }
.whitespace-nowrap { white-space: nowrap; }
.leading-none { line-height: 1; } .leading-none { line-height: 1; }
.absolute { position: absolute; } .absolute { position: absolute; }
@@ -45,6 +47,7 @@ $spacer: 0.25rem; /* 4px */
.mx-0\.5 { margin-left: 0.5 * $spacer; margin-right: 0.5 * $spacer; } .mx-0\.5 { margin-left: 0.5 * $spacer; margin-right: 0.5 * $spacer; }
.mx-2 { margin-left: 2 * $spacer; margin-right: 2 * $spacer; } .mx-2 { margin-left: 2 * $spacer; margin-right: 2 * $spacer; }
.mt-1 { margin-top: 1 * $spacer; }
.mt-2 { margin-top: 2 * $spacer; } .mt-2 { margin-top: 2 * $spacer; }
.mt-4 { margin-top: 4 * $spacer; } .mt-4 { margin-top: 4 * $spacer; }
.mt-8 { margin-top: 8 * $spacer; } .mt-8 { margin-top: 8 * $spacer; }

View File

@@ -41,11 +41,13 @@
} }
} }
#posts #posts-container { .posts-container {
width: 100%; display: grid;
display: flex; grid-template-columns: repeat(3, minmax(0, 1fr));
flex-wrap: wrap; gap: 0.25rem;
align-items: center;
justify-content: flex-start; &.user-disable-cropped-false article.post-preview img.has-cropped-true {
object-fit: none;
}
} }
} }

View File

@@ -115,6 +115,7 @@ module PostSets
def hide_from_crawler? def hide_from_crawler?
return true if current_page > 50 return true if current_page > 50
return true if show_votes?
return true if artist.present? && artist.is_banned? return true if artist.present? && artist.is_banned?
return false if query.is_empty_search? || query.is_simple_tag? || query.is_metatag?(:order, :rank) return false if query.is_empty_search? || query.is_simple_tag? || query.is_metatag?(:order, :rank)
true true

View File

@@ -548,9 +548,11 @@ class Post < ApplicationRecord
pool&.add!(self) pool&.add!(self)
when /^fav:(.+)$/i when /^fav:(.+)$/i
raise User::PrivilegeError unless Pundit.policy!(CurrentUser.user, Favorite).create?
Favorite.create(post: self, user: CurrentUser.user) Favorite.create(post: self, user: CurrentUser.user)
when /^-fav:(.+)$/i when /^-fav:(.+)$/i
raise User::PrivilegeError unless Pundit.policy!(CurrentUser.user, Favorite).create?
Favorite.destroy_by(post: self, user: CurrentUser.user) Favorite.destroy_by(post: self, user: CurrentUser.user)
when /^(up|down)vote:(.+)$/i when /^(up|down)vote:(.+)$/i

View File

@@ -35,9 +35,9 @@
<%= render PopupMenuComponent.new do |menu| %> <%= render PopupMenuComponent.new do |menu| %>
<% menu.item do %> <% menu.item do %>
<% if params[:view] == "score" %> <% if params[:view] == "score" %>
<%= link_to "Hide scores", posts_path(tags: params[:tags], view: nil) %> <%= link_to "Hide scores", posts_path(tags: params[:tags], page: params[:page], limit: params[:limit], view: nil), rel: "nofollow" %>
<% else %> <% else %>
<%= link_to "Show scores", posts_path(tags: params[:tags], view: "score") %> <%= link_to "Show scores", posts_path(tags: params[:tags], page: params[:page], limit: params[:limit], view: "score"), rel: "nofollow" %>
<% end %> <% end %>
<% end %> <% end %>
<% end %> <% end %>

View File

@@ -1,5 +1,5 @@
<div id="posts" class="user-disable-cropped-<%= CurrentUser.user.disable_cropped_thumbnails? %>"> <div id="posts">
<div id="posts-container"> <div class="posts-container user-disable-cropped-<%= CurrentUser.user.disable_cropped_thumbnails? %>">
<% if post_set.shown_posts.empty? %> <% if post_set.shown_posts.empty? %>
<%= render "post_sets/blank" %> <%= render "post_sets/blank" %>
<% else %> <% else %>

View File

@@ -12,6 +12,8 @@
<% if params[:tags].blank? && @post_set.current_page == 1 %> <% if params[:tags].blank? && @post_set.current_page == 1 %>
<% canonical_url root_url(host: Danbooru.config.hostname) %> <% canonical_url root_url(host: Danbooru.config.hostname) %>
<% else %>
<% canonical_url posts_url(host: Danbooru.config.hostname, tags: params[:tags], page: params[:page], limit: params[:limit]) %>
<% end %> <% end %>
<% noindex if @post_set.hide_from_crawler? %> <% noindex if @post_set.hide_from_crawler? %>

View File

@@ -70,6 +70,12 @@
<td>2,000</td> <td>2,000</td>
<td>5,000</td> <td>5,000</td>
</tr> </tr>
<tr>
<td>Private Favorites</td>
<td>no</td>
<td>yes</td>
<td>yes</td>
</tr>
<tr> <tr>
<td>Favorite Groups</td> <td>Favorite Groups</td>
<td>3</td> <td>3</td>

View File

@@ -7,7 +7,7 @@ class PostNavbarComponentTest < ViewComponent::TestCase
setup do setup do
@post = create(:post) @post = create(:post)
@user = create(:user) @user = create(:gold_user)
end end
context "The PostNavbarComponent" do context "The PostNavbarComponent" do
@@ -46,9 +46,8 @@ class PostNavbarComponentTest < ViewComponent::TestCase
context "for a post with favgroups" do context "for a post with favgroups" do
setup do setup do
as(@user) do as(@user) do
@favgroup1 = create(:favorite_group, creator: @user, is_public: true) @favgroup1 = create(:favorite_group, creator: @user, post_ids: [@post.id])
@favgroup2 = create(:favorite_group, creator: @user, is_public: false) @favgroup2 = create(:private_favorite_group, creator: @user, post_ids: [@post.id])
@post.update(tag_string: "favgroup:#{@favgroup1.id} favgroup:#{@favgroup2.id}")
end end
end end

View File

@@ -11,12 +11,12 @@ class PostVotesComponentTest < ViewComponent::TestCase
end end
context "for a user who can't vote" do context "for a user who can't vote" do
should "not show the vote buttons" do should "show the vote buttons" do
render_post_votes(@post, current_user: User.anonymous) render_post_votes(@post, current_user: User.anonymous)
assert_css(".post-score") assert_css(".post-score")
assert_no_css(".post-upvote-link") assert_css(".post-upvote-link.inactive-link")
assert_no_css(".post-downvote-link") assert_css(".post-downvote-link.inactive-link")
end end
end end
@@ -34,8 +34,8 @@ class PostVotesComponentTest < ViewComponent::TestCase
context "for a downvoted post" do context "for a downvoted post" do
should "highlight the downvote button as active" do should "highlight the downvote button as active" do
@post.vote!(-1, @user) create(:post_vote, post: @post, user: @user, score: -1)
render_post_votes(@post, current_user: @user) as(@user) { render_post_votes(@post, current_user: @user) }
assert_css(".post-upvote-link.inactive-link") assert_css(".post-upvote-link.inactive-link")
assert_css(".post-downvote-link.active-link") assert_css(".post-downvote-link.active-link")
@@ -44,8 +44,8 @@ class PostVotesComponentTest < ViewComponent::TestCase
context "for an upvoted post" do context "for an upvoted post" do
should "highlight the upvote button as active" do should "highlight the upvote button as active" do
@post.vote!(1, @user) create(:post_vote, post: @post, user: @user, score: 1)
render_post_votes(@post, current_user: @user) as(@user) { render_post_votes(@post, current_user: @user) }
assert_css(".post-upvote-link.active-link") assert_css(".post-upvote-link.active-link")
assert_css(".post-downvote-link.inactive-link") assert_css(".post-downvote-link.inactive-link")

View File

@@ -2,5 +2,9 @@ FactoryBot.define do
factory(:favorite) do factory(:favorite) do
user user
post post
factory(:private_favorite) do
user factory: :gold_user, enable_private_favorites: true
end
end end
end end

View File

@@ -11,10 +11,13 @@ class FavoriteTest < ActiveSupport::TestCase
context "Favorites: " do context "Favorites: " do
context "removing a favorite" do context "removing a favorite" do
should "update the post and user favorite counts" do should "update the post and user favorite counts" do
@user1 = create(:restricted_user)
fav = Favorite.create!(post: @p1, user: @user1) fav = Favorite.create!(post: @p1, user: @user1)
assert_equal(1, @user1.reload.favorite_count) assert_equal(1, @user1.reload.favorite_count)
assert_equal(1, @p1.reload.fav_count) assert_equal(1, @p1.reload.fav_count)
assert_equal(0, @p1.reload.score)
refute(PostVote.positive.exists?(post: @p1, user: @user))
Favorite.destroy_by(post: @p1, user: @user1) Favorite.destroy_by(post: @p1, user: @user1)
@@ -42,14 +45,8 @@ class FavoriteTest < ActiveSupport::TestCase
end end
context "adding a favorite" do context "adding a favorite" do
should "update the post and user favorite counts" do
Favorite.create!(post: @p1, user: @user1)
assert_equal(1, @user1.reload.favorite_count)
assert_equal(1, @p1.reload.fav_count)
end
should "not upvote the post if the user can't vote" do should "not upvote the post if the user can't vote" do
@user1 = create(:restricted_user)
Favorite.create!(post: @p1, user: @user1) Favorite.create!(post: @p1, user: @user1)
assert_equal(1, @user1.reload.favorite_count) assert_equal(1, @user1.reload.favorite_count)
@@ -89,7 +86,7 @@ class FavoriteTest < ActiveSupport::TestCase
assert_equal(["You have already favorited this post"], @f2.errors.full_messages) assert_equal(["You have already favorited this post"], @f2.errors.full_messages)
assert_equal(1, @user1.reload.favorite_count) assert_equal(1, @user1.reload.favorite_count)
assert_equal(1, @p1.reload.fav_count) assert_equal(1, @p1.reload.fav_count)
assert_equal(0, @p1.reload.score) assert_equal(1, @p1.reload.score)
end end
end end
end end

View File

@@ -390,7 +390,7 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
favgroup1 = create(:favorite_group, creator: CurrentUser.user, post_ids: [post1.id]) favgroup1 = create(:favorite_group, creator: CurrentUser.user, post_ids: [post1.id])
favgroup2 = create(:favorite_group, creator: CurrentUser.user, post_ids: [post2.id]) favgroup2 = create(:favorite_group, creator: CurrentUser.user, post_ids: [post2.id])
favgroup3 = create(:favorite_group, creator: create(:user), post_ids: [post3.id], is_public: false) favgroup3 = create(:private_favorite_group, post_ids: [post3.id])
assert_tag_match([post1], "favgroup:#{favgroup1.id}") assert_tag_match([post1], "favgroup:#{favgroup1.id}")
assert_tag_match([post2], "favgroup:#{favgroup2.name}") assert_tag_match([post2], "favgroup:#{favgroup2.name}")
@@ -421,7 +421,7 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
post3 = create(:post) post3 = create(:post)
favgroup1 = create(:favorite_group, creator: CurrentUser.user, post_ids: [post1.id, post2.id]) favgroup1 = create(:favorite_group, creator: CurrentUser.user, post_ids: [post1.id, post2.id])
favgroup2 = create(:favorite_group, creator: create(:user), post_ids: [post2.id, post3.id], is_public: false) favgroup2 = create(:private_favorite_group, post_ids: [post2.id, post3.id])
assert_tag_match([post1, post2], "ordfavgroup:#{favgroup1.id}") assert_tag_match([post1, post2], "ordfavgroup:#{favgroup1.id}")
assert_tag_match([post1, post2], "ordfavgroup:#{favgroup1.name}") assert_tag_match([post1, post2], "ordfavgroup:#{favgroup1.name}")
@@ -975,7 +975,7 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
context "for the upvote:<user> metatag" do context "for the upvote:<user> metatag" do
setup do setup do
@user = create(:user) @user = create(:gold_user)
@upvote = create(:post_vote, user: @user, score: 1) @upvote = create(:post_vote, user: @user, score: 1)
@downvote = create(:post_vote, user: @user, score: -1) @downvote = create(:post_vote, user: @user, score: -1)
end end
@@ -1411,8 +1411,7 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
end end
should "return the correct favorite count for a fav:<name> search for a user with private favorites" do should "return the correct favorite count for a fav:<name> search for a user with private favorites" do
fav = create(:favorite) fav = create(:private_favorite)
fav.user.update!(favorite_count: 1, enable_private_favorites: true)
assert_fast_count(0, "fav:#{fav.user.name}") assert_fast_count(0, "fav:#{fav.user.name}")
assert_fast_count(0, "ordfav:#{fav.user.name}") assert_fast_count(0, "ordfav:#{fav.user.name}")

View File

@@ -699,10 +699,34 @@ class PostTest < ActiveSupport::TestCase
context "for a fav" do context "for a fav" do
should "add/remove the current user to the post's favorite listing" do should "add/remove the current user to the post's favorite listing" do
@post.update(tag_string: "aaa fav:self") @post.update(tag_string: "aaa fav:self")
assert_equal(1, @post.reload.score)
assert_equal(1, @post.favorites.where(user: @user).count) assert_equal(1, @post.favorites.where(user: @user).count)
assert_equal(1, @post.votes.positive.where(user: @user).count)
@post.update(tag_string: "aaa -fav:self") @post.update(tag_string: "aaa -fav:self")
assert_equal(0, @post.reload.score)
assert_equal(0, @post.favorites.count) assert_equal(0, @post.favorites.count)
assert_equal(0, @post.votes.positive.where(user: @user).count)
end
should "not allow banned users to fav" do
assert_raises(User::PrivilegeError) do
as(create(:banned_user)) { @post.update(tag_string: "aaa fav:self") }
end
assert_raises(User::PrivilegeError) do
as(create(:banned_user)) { @post.update(tag_string: "aaa -fav:self") }
end
end
should "not allow restricted users to fav" do
assert_raises(User::PrivilegeError) do
as(create(:restricted_user)) { @post.update(tag_string: "aaa fav:self") }
end
assert_raises(User::PrivilegeError) do
as(create(:restricted_user)) { @post.update(tag_string: "aaa -fav:self") }
end
end end
should "not fail when the fav: metatag is used twice" do should "not fail when the fav: metatag is used twice" do
@@ -870,20 +894,20 @@ class PostTest < ActiveSupport::TestCase
context "upvote:self or downvote:self" do context "upvote:self or downvote:self" do
context "by a member" do context "by a member" do
should "not upvote the post" do should "upvote the post" do
assert_no_difference("PostVote.count") do assert_difference("PostVote.count") do
@post.update(tag_string: "upvote:self") @post.update(tag_string: "upvote:self")
end end
assert_equal(0, @post.reload.score) assert_equal(1, @post.reload.score)
end end
should "not downvote the post" do should "downvote the post" do
assert_no_difference("PostVote.count") do assert_difference("PostVote.count") do
@post.update(tag_string: "downvote:self") @post.update(tag_string: "downvote:self")
end end
assert_equal(0, @post.reload.score) assert_equal(-1, @post.reload.score)
end end
end end
@@ -1476,7 +1500,8 @@ class PostTest < ActiveSupport::TestCase
should "create a vote for each user who can vote" do should "create a vote for each user who can vote" do
assert(@parent.votes.where(user: @gold1).exists?) assert(@parent.votes.where(user: @gold1).exists?)
assert_equal(1, @parent.score) assert(@parent.votes.where(user: @user1).exists?)
assert_equal(2, @parent.score)
end end
end end
end end
@@ -1523,13 +1548,13 @@ class PostTest < ActiveSupport::TestCase
end end
context "Voting:" do context "Voting:" do
should "not allow members to vote" do should "allow members to vote" do
user = create(:user) user = create(:user)
post = create(:post) post = create(:post)
assert_nothing_raised { post.vote!(1, user) } assert_nothing_raised { post.vote!(1, user) }
assert_equal(0, post.votes.count) assert_equal(1, post.votes.count)
assert_equal(0, post.reload.score) assert_equal(1, post.reload.score)
end end
should "not allow duplicate votes" do should "not allow duplicate votes" do