Refactor user avatar handling and improve user status display

- Updated user avatar creation method to ensure avatar is always created or retrieved.
- Enhanced user status display in views with improved styling and additional information.
- Added missing commas in various places for better code consistency.
- Improved error handling in user avatar job for better logging and state management.
- Updated tests to reflect changes in avatar handling and user model methods.
This commit is contained in:
Dylan Knutson
2024-12-27 18:40:18 +00:00
parent e7a584bc57
commit 50af3d90d8
11 changed files with 258 additions and 175 deletions

View File

@@ -40,10 +40,10 @@ module Domain::Fa::UsersHelper
elements: %w[br img b i span strong],
attributes: {
"span" => %w[style],
"a" => []
"a" => [],
},
css: {
properties: %w[font-size color]
properties: %w[font-size color],
},
transformers:
lambda do |env|
@@ -57,7 +57,7 @@ module Domain::Fa::UsersHelper
url_name = href.path.split("/")[2]&.downcase
Sanitize.node!(
node,
{ elements: %w[a], attributes: { "a" => %w[href] } }
{ elements: %w[a], attributes: { "a" => %w[href] } },
)
node["href"] = domain_fa_user_path(url_name)
node["class"] = "text-slate-200 underline decoration-slate-200 " +
@@ -77,7 +77,7 @@ module Domain::Fa::UsersHelper
end
{ node_allowlist: whitelist }
end
end,
)
end
@@ -87,22 +87,22 @@ module Domain::Fa::UsersHelper
else
ReduxApplicationRecord.connection.execute("SET ivfflat.probes = 32")
user.similar_users_by_followed(
exclude_followed_by: exclude_followed_by
exclude_followed_by: exclude_followed_by,
).limit(limit)
end
end
def fa_user_account_status(user)
log_entry_id = user.log_entry_detail["last_user_page_id"]
return nil if log_entry_id.nil?
return "unknown" if log_entry_id.nil?
log_entry = HttpLogEntry.find_by(id: log_entry_id)
return nil if log_entry.nil?
return "unknown" if log_entry.nil?
parser =
Domain::Fa::Parser::Page.new(
log_entry.response.contents,
require_logged_in: false
require_logged_in: false,
)
return nil unless parser.probably_user_page?
return "unknown" unless parser.probably_user_page?
parser.user_page.account_status
end
end

View File

@@ -3,7 +3,7 @@ module Domain::Fa::Job
DISABLED_PAGE_PATTERNS = [
/User ".+" has voluntarily disabled access/,
/User ".+" was not found in our database./,
/The page you are trying to reach is currently pending deletion/
/The page you are trying to reach is currently pending deletion/,
]
def self.user_disabled_or_not_found?(user, response)
@@ -27,7 +27,7 @@ module Domain::Fa::Job
:fatal,
{
message:
"http #{response.status_code}, log entry #{response.log_entry.id}"
"http #{response.status_code}, log entry #{response.log_entry.id}",
}
]
end
@@ -41,7 +41,7 @@ module Domain::Fa::Job
user.state_detail = {
scan_error:
"(user scan) user has disabled account, see last_user_page_id",
last_user_page_id: response.log_entry.id
last_user_page_id: response.log_entry.id,
}
try_name = /User "(.+)" has voluntarily disabled/.match(response.body)
user.name ||= try_name && try_name[1] || user.url_name
@@ -55,7 +55,7 @@ module Domain::Fa::Job
user.state = :scan_error
user.state_detail = {
scan_error: "(user scan) user was not found, see last_user_page_id",
last_user_page_id: response.log_entry.id
last_user_page_id: response.log_entry.id,
}
user.name ||= user.url_name
user.save!
@@ -82,16 +82,16 @@ module Domain::Fa::Job
user_page.profile_html.encode(
"UTF-8",
invalid: :replace,
undef: :replace
undef: :replace,
)
user.log_entry_detail["last_user_page_id"] = response.log_entry.id
avatar = user.avatar_or_create
avatar = user.ensure_avatar!
user.avatar.file_uri = user_page.profile_thumb_url
if user.avatar.changed?
user.avatar.save!
Domain::Fa::Job::UserAvatarJob.perform_later(
{ user: user, caused_by_entry: response.log_entry }
{ user: user, caused_by_entry: response.log_entry },
)
end
end
@@ -105,7 +105,7 @@ module Domain::Fa::Job
user =
Domain::Fa::User.create!(url_name: name.url_name, name: name.name)
Domain::Fa::Job::UserPageJob.perform_later(
{ user: user, caused_by_entry: caused_by_entry }
{ user: user, caused_by_entry: caused_by_entry },
)
users << user
end

View File

@@ -5,58 +5,44 @@ class Domain::Fa::Job::UserAvatarJob < Domain::Fa::Job::Base
def perform(args)
init_from_args!(args, build_user: false)
@user || raise("user must exist")
@avatar = @user.avatar_or_create
@avatar = @user.ensure_avatar!
logger.prefix =
proc do
"[avatar #{@avatar.id.to_s.bold} / user #{@user.url_name.to_s.bold}]"
end
if @avatar.file_sha256 && !@force_scan
logger.warn(
"downloaded #{time_ago_in_words(@avatar.downloaded_file_at)}, skipping"
)
return
end
if @avatar.state != "ok" && !@force_scan
logger.warn("in state #{@avatar.state.bold}, skipping")
return
end
if @avatar.file_uri.blank?
if @user.due_for_page_scan?
defer_job(
Domain::Fa::Job::UserPageJob,
{ user: @user, caused_by_entry: @caused_by_entry }
)
logger.error("no file uri, scanning user page")
else
logger.error("no file uri")
end
return
end
response =
http_client.get(@avatar.file_uri.to_s, caused_by_entry: @caused_by_entry)
http_client.get(
"https://a.furaffinity.net/0/#{@user.url_name}.gif",
caused_by_entry: @caused_by_entry,
)
@avatar.state_detail["log_entries"] ||= [@avatar&.log_entry&.id].compact
@avatar.state_detail["log_entries"] << response.log_entry.id
@avatar.log_entry = response.log_entry
if [200, 404].include?(response.status_code)
if response.status_code == 404
@avatar.state = :file_not_found
else
@avatar.state = :ok
end
case response.status_code
when 200
@avatar.state = :ok
@avatar.downloaded_file_at = response.log_entry.created_at
@avatar.file = response.log_entry.response
logger.info("downloaded avatar file")
logger.info("downloaded avatar")
when 404
@avatar.state = :file_not_found
if @avatar.file_sha256.blank?
@avatar.downloaded_file_at = response.log_entry.created_at
@avatar.file = response.log_entry.response
logger.info("avatar 404, and no previous file")
else
logger.info("avatar 404, and previous file")
end
else
@avatar.state = :download_error
@avatar.state_detail[
"download_error"
] = "http status #{response.status_code}"
fatal_error(
"http #{response.status_code}, log entry #{response.log_entry.id}"
"http #{response.status_code}, log entry #{response.log_entry.id}",
)
end
ensure

View File

@@ -21,14 +21,14 @@ class Domain::Fa::UserEnqueuer
if already_enqueued <= @low_water_mark
to_enqueue = @high_water_mark - already_enqueued
logger.info(
"enqueuing #{to_enqueue.to_s.bold} more users - #{already_enqueued.to_s.bold} already enqueued"
"enqueuing #{to_enqueue.to_s.bold} more users - #{already_enqueued.to_s.bold} already enqueued",
)
rows =
measure(
proc do |p|
p && "gathered #{p.length.to_s.bold} users to enqueue" ||
"gathering users..."
end
end,
) { to_enqueue.times.map { @user_iterator.next } }
measure("enqueue jobs") do
rows.each do |user|
@@ -59,7 +59,7 @@ class Domain::Fa::UserEnqueuer
# end
end
avatar = user.avatar_or_create
avatar = user.ensure_avatar!
if avatar.file.nil? && avatar.state == "ok"
Domain::Fa::Job::UserAvatarJob.perform_later({ user: user })
types << "avatar"
@@ -73,7 +73,7 @@ class Domain::Fa::UserEnqueuer
else
logger.info(
"#{already_enqueued.to_s.bold} already enqueued (max #{@high_water_mark.to_s.bold}) - " +
"waiting to fall below #{@low_water_mark.to_s.bold}"
"waiting to fall below #{@low_water_mark.to_s.bold}",
)
:sleep
end

View File

@@ -22,7 +22,7 @@ class Domain::Fa::User < ReduxApplicationRecord
enum :state,
[
:ok, # so far so good, user may not yet be scanned
:scan_error # user has been removed or otherwise, see state_detail
:scan_error, # user has been removed or otherwise, see state_detail
]
# Who this user follows (join table)
@@ -69,7 +69,7 @@ class Domain::Fa::User < ReduxApplicationRecord
unless matches
errors.add(
:name,
"name '#{name}' does not match url_name, expected #{expected} but was #{url_name}"
"name '#{name}' does not match url_name, expected #{expected} but was #{url_name}",
)
end
end
@@ -96,7 +96,7 @@ class Domain::Fa::User < ReduxApplicationRecord
gallery: 1.year,
follows: 1.month,
favs: 1.month,
incremental: 1.month
incremental: 1.month,
}
SCAN_FIELD_TYPES = {
@@ -104,7 +104,7 @@ class Domain::Fa::User < ReduxApplicationRecord
gallery: :column,
follows: :column,
favs: :column,
incremental: :state_detail
incremental: :state_detail,
}
SCAN_TYPES.keys.each do |scan_type|
@@ -151,13 +151,13 @@ class Domain::Fa::User < ReduxApplicationRecord
posts.reload
end
def avatar_or_create
def ensure_avatar!
self.class.transaction { avatar || create_avatar! }
end
def self.find_or_build_from_submission_parser(submission_parser)
unless submission_parser.is_a?(
Domain::Fa::Parser::ListedSubmissionParserHelper
Domain::Fa::Parser::ListedSubmissionParserHelper,
) ||
submission_parser.is_a?(Domain::Fa::Parser::SubmissionParserHelper)
raise ArgumentError
@@ -203,7 +203,7 @@ class Domain::Fa::User < ReduxApplicationRecord
.where(
uri_scheme: "https",
uri_host: "www.furaffinity.net",
uri_path: uri_path
uri_path: uri_path,
)
.order(created_at: :desc)
.first
@@ -235,7 +235,7 @@ class Domain::Fa::User < ReduxApplicationRecord
query =
query.where.not(
user_id: exclude_followed_by.follows.select(:followed_id)
user_id: exclude_followed_by.follows.select(:followed_id),
) if exclude_followed_by
users_from_disco_query(query)

View File

@@ -2,13 +2,27 @@
<div class="flex grow items-center gap-4">
<img src="<%= fa_user_avatar_path(user) %>" class="h-12 w-12 rounded-lg" />
<div>
<div class="text-lg font-medium text-slate-900"><%= user.url_name %></div>
<div class="text-sm text-slate-500">
<span><%= fa_user_account_status(user) %></span> •
<span><%= user.state %></span>
</div>
<div class="text-sm text-slate-500">
Registered <%= time_ago_in_words(user.registered_at) %> ago
<div class="text-lg font-bold text-slate-900"><%= user.url_name %></div>
<div class="flex gap-6 text-sm text-slate-400">
<div class="flex flex-col">
<span class="font-medium italic text-slate-500">Status</span>
<span class=""><%= fa_user_account_status(user) %></span>
</div>
<div class="flex flex-col">
<span class="font-medium italic text-slate-500">State</span>
<span class=""><%= user.state %></span>
</div>
<div class="flex flex-col">
<span class="font-medium italic text-slate-500">Registered</span>
<span class="">
<% if user.registered_at %>
<%= time_ago_in_words(user.registered_at) %>
ago
<% else %>
unknown
<% end %>
</span>
</div>
</div>
</div>
</div>
@@ -18,7 +32,7 @@
rel="noopener noreferrer"
class="sky-link flex items-center gap-2"
>
<span class="font-medium">FurAffinity</span>
<span class="font-bold">FurAffinity</span>
<img src="<%= image_path("domain-icons/fa.png") %>" class="h-5 w-5" />
</a>
</section>

View File

@@ -5,7 +5,7 @@ FactoryBot.define do
content_type { "text/plain" }
sha256 { Digest::SHA256.digest(content) }
contents { content }
size { content.size }
size { content.bytesize }
trait :html do
content_type { "text/html" }

View File

@@ -1,107 +1,190 @@
require "rails_helper"
describe Domain::Fa::Job::UserAvatarJob do
let(:user) { create(:domain_fa_user, url_name: "meesh", name: "Meesh") }
let(:http_client_mock) { instance_double("::Scraper::HttpClient") }
before { Scraper::ClientFactory.http_client_mock = http_client_mock }
shared_context "create meesh user" do
let(:user) { Domain::Fa::User.find_by(url_name: "meesh") || raise }
before { Domain::Fa::User.create!({ url_name: "meesh", name: "Meesh" }) }
let(:avatar_fixture_file) do
SpecUtil.read_fixture_file(
"domain/fa/job/meesh_avatar_file.gif",
mode: "rb",
)
end
context "when the user does not exist" do
it "throws" do
perform_now({ url_name: "meesh" }, should_raise: /user must exist/)
let(:avatar_fixture_file_404) do
SpecUtil.read_fixture_file("domain/fa/job/404_avatar_file.gif", mode: "rb")
end
let(:avatar_fixture_file_2) do
SpecUtil.read_fixture_file(
"domain/fa/job/roadkillxing_avatar_file.gif",
mode: "rb",
)
end
shared_context "avatar file found" do
before do
@log_entries =
SpecUtil.init_http_client_mock(
http_client_mock,
[
{
uri: "https://a.furaffinity.net/0/meesh.gif",
status_code: 200,
content_type: "image/gif",
contents: avatar_fixture_file,
},
],
)
end
end
context "when the avatar model does not yet exist" do
include_context "create meesh user"
context "the user has not been page scanned yet" do
it "enqueues a user page scan job" do
perform_now({ user: user })
expect(user.avatar.file_uri).to eq(nil)
expect(SpecUtil.enqueued_jobs(Domain::Fa::Job::UserPageJob)).to match(
[including(args: [{ user: user, caused_by_entry: nil }])]
shared_context "avatar file not found" do
before do
@log_entries =
SpecUtil.init_http_client_mock(
http_client_mock,
[
{
uri: "https://a.furaffinity.net/0/meesh.gif",
status_code: 404,
content_type: "image/gif",
contents: avatar_fixture_file_404,
},
],
)
end
end
shared_context "avatar file is a server error" do
before do
@log_entries =
SpecUtil.init_http_client_mock(
http_client_mock,
[
{
uri: "https://a.furaffinity.net/0/meesh.gif",
status_code: 500,
content_type: "text/html",
contents: "Server Error",
},
],
)
end
end
it "throws when the user does not exist" do
perform_now({ url_name: "meesh" }, should_raise: /user must exist/)
end
context "the user model exists" do
context "the server response is 200" do
include_context "avatar file found"
it "sets the file and the right state" do
perform_now({ user: user })
user.reload
avatar = user.avatar
expect(avatar).not_to be_nil
expect(avatar.state).to eq("ok")
expect(avatar.log_entry).to eq(@log_entries[0])
expect(HexUtil.bin2hex avatar.file_sha256).to eq(
"ebbafc07555df0a0656a9b32ec9b95723c62c5246937dc8434924d9241d1b570",
)
expect(avatar.downloaded_file_at).to be_within(1.seconds).of(Time.now)
end
context "the avatar model has a file already" do
let(:avatar) { user.ensure_avatar! }
it "sets file to the new file" do
avatar.state = :ok
avatar.file =
create(
:blob_entry_p,
content: avatar_fixture_file_2,
content_type: "image/gif",
)
first_log_entry = create(:http_log_entry, response: avatar.file)
avatar.log_entry = first_log_entry
avatar.save!
perform_now({ user: user })
avatar.reload
expect(avatar.state).to eq("ok")
expect(avatar.state_detail["log_entries"]).to eq(
[first_log_entry.id, @log_entries[0].id],
)
expect(avatar.log_entry).to eq(@log_entries[0])
expect(HexUtil.bin2hex avatar.file_sha256).to eq(
"ebbafc07555df0a0656a9b32ec9b95723c62c5246937dc8434924d9241d1b570",
)
expect(avatar.downloaded_file_at).to be_within(1.seconds).of(Time.now)
end
end
end
context "the server response is 404" do
include_context "avatar file not found"
it "sets the file and the right state" do
perform_now({ user: user })
user.reload
avatar = user.avatar
expect(avatar).not_to be_nil
expect(avatar.log_entry).to eq(@log_entries[0])
expect(avatar.state_detail["log_entries"]).to eq([avatar.log_entry.id])
expect(avatar.file).to be_present
expect(HexUtil.bin2hex avatar.file_sha256).to eq(
"9080fd4e7e23920eb2dccfe2d86903fc3e748eebb2e5aa8c657bbf6f3d941cdc",
)
expect(avatar.downloaded_file_at).to be_within(1.seconds).of(Time.now)
expect(avatar.state).to eq("file_not_found")
end
context "a previous successful avatar was downloaded" do
let(:avatar) { user.ensure_avatar! }
it "does not overwrite the file" do
avatar.state = :ok
avatar.file =
create(
:blob_entry_p,
content: avatar_fixture_file,
content_type: "image/gif",
)
log_entry = create(:http_log_entry, response: avatar.file)
avatar.log_entry = log_entry
avatar.save!
perform_now({ user: user })
avatar.reload
expect(avatar.state).to eq("file_not_found")
expect(avatar.file).to eq(log_entry.response)
expect(avatar.log_entry).to eq(@log_entries[0])
expect(avatar.state_detail["log_entries"]).to eq(
[log_entry.id, @log_entries[0].id],
)
end
end
end
context "the server response is 500" do
include_context "avatar file is a server error"
it "has a file and the right state" do
expect { perform_now({ user: user }) }.to raise_error(
/http 500, log entry.+/,
)
avatar = user.avatar.reload
expect(avatar).not_to be_nil
expect(avatar.log_entry).to eq(@log_entries[0])
expect(avatar.state_detail["log_entries"]).to eq([avatar.log_entry.id])
expect(avatar.file).to be_nil
expect(avatar.state).to eq("download_error")
expect(avatar.state_detail["download_error"]).to eq("http status 500")
end
end
end
context "the avatar model exists with a file uri" do
include_context "create meesh user"
before do
avatar = user.avatar_or_create
avatar.file_uri = "https://www.furaffinity.net/a/test/uri.gif"
avatar.save!
@log_entries =
SpecUtil.init_http_client_mock(
http_client_mock,
[
{
uri: "https://www.furaffinity.net/a/test/uri.gif",
status_code: 200,
content_type: "image/gif",
contents:
SpecUtil.read_fixture_file(
"domain/fa/job/meesh_avatar_file.gif",
mode: "rb"
)
}
]
)
end
it "succeeds" do
perform_now({ user: user })
user.reload
avatar = user.avatar
expect(avatar).not_to be_nil
expect(avatar.log_entry).to eq(@log_entries[0])
expect(HexUtil.bin2hex avatar.file_sha256).to eq(
"ebbafc07555df0a0656a9b32ec9b95723c62c5246937dc8434924d9241d1b570"
)
expect(avatar.downloaded_file_at).to be_within(1.seconds).of(Time.now)
end
end
context "the avatar is 404" do
include_context "create meesh user"
before do
avatar = user.avatar_or_create
avatar.file_uri = "https://www.furaffinity.net/a/test/uri.gif"
avatar.save!
@log_entries =
SpecUtil.init_http_client_mock(
http_client_mock,
[
{
uri: "https://www.furaffinity.net/a/test/uri.gif",
status_code: 404,
content_type: "image/gif",
contents:
SpecUtil.read_fixture_file(
"domain/fa/job/meesh_avatar_file.gif",
mode: "rb"
)
}
]
)
end
it "has a file and the right state" do
perform_now({ user: user })
user.reload
avatar = user.avatar
expect(avatar).not_to be_nil
expect(avatar.log_entry).to eq(@log_entries[0])
expect(HexUtil.bin2hex avatar.file_sha256).to eq(
"ebbafc07555df0a0656a9b32ec9b95723c62c5246937dc8434924d9241d1b570"
)
expect(avatar.downloaded_file_at).to be_within(1.seconds).of(Time.now)
expect(avatar.state).to eq("file_not_found")
end
end
end

View File

@@ -8,7 +8,7 @@ describe Domain::Fa::User do
it "user can be destroyed" do
user = SpecUtil.create_domain_fa_user(name: "Foo", url_name: "foo")
user.avatar_or_create
user.ensure_avatar!
user.destroy
end
@@ -18,11 +18,11 @@ describe Domain::Fa::User do
.create(
{
name: "Rails_the_7-tailed_red-orange_",
url_name: "railsthe7-tailedred-orangekits"
}
url_name: "railsthe7-tailedred-orangekits",
},
)
.errors
.full_messages
.full_messages,
).to be_empty
end
@@ -31,7 +31,7 @@ describe Domain::Fa::User do
described_class
.create({ name: "-_fayen_-", url_name: "-fayen-" })
.errors
.full_messages
.full_messages,
).to be_empty
end
@@ -40,7 +40,7 @@ describe Domain::Fa::User do
described_class
.create({ name: "Kammiu", url_name: "rammiu" })
.errors
.full_messages
.full_messages,
).to be_empty
end
@@ -49,7 +49,7 @@ describe Domain::Fa::User do
described_class
.create({ name: "Foo", url_name: "Foo" })
.errors
.full_messages
.full_messages,
).to_not be_empty
end
@@ -58,7 +58,7 @@ describe Domain::Fa::User do
described_class
.create({ name: "Foo", url_name: "foo " })
.errors
.full_messages
.full_messages,
).to_not be_empty
end
@@ -67,7 +67,7 @@ describe Domain::Fa::User do
described_class
.create({ name: "Fur Affinity Staff", url_name: "furaffinitystaff" })
.errors
.full_messages
.full_messages,
).to be_empty
end

Binary file not shown.

After

Width:  |  Height:  |  Size: 3.5 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 8.1 KiB