AO3-3730 series stats

[anmazz]

Issue

https://otwarchive.atlassian.net/browse/AO3-3730
https://otwarchive.atlassian.net/browse/AO3-6301

Purpose

Refactored with ActiveRecord.

• Series stats that appear/sort in the same listing as work stats
• Series stats include Work Count, Bookmarks, and Subscriptions
• Bookmarks and Subscriptions are sortable among both Series and Works
• Adds new "Types" filter to stats page which groups by Work and Series (retains Fandoms and Flat view)

Also fixes fandom tags on series overview page where fandoms attached only to draft works would appear in listing.

Credit

anna

[Hamham6]

From https://otwarchive.atlassian.net/browse/AO3-5994, 'spam' isn't actually being used yet, and 'approved' is the column for checking spam. Does this need to account for comments on moderated works that aren't approved by the creator ('unreviewed', from the Jira issue) and/or comments hidden by admins that aren't spam?

[anmazz]

(Just dumping a bunch of thoughts here so apologies!) I need to double check some of this/have someone else confirm but when I was testing just locally, I set the hidden_by_admin flag = true manually in the database. So I'm not sure if in the process of doing it with an admin account ALSO sets the approved flag to false in that instance. In that scenario (just setting the hidden_by_admin flag to be true, but approved = false) it wasn't filtering out that from the count.

I can check/test that locally. All that to say, if it doesn't set approved = false when hidden_by_admin = true, then the current version of the code doesn't seem to filter out those comments from the count. Seems like if approved will be renamed to spam then it won't... but this is me saying that without checking.

Anyways, I do think that it should filter those out from the count (hidden_by_admin = true). I'm not super familiar with the comments on moderated works so if anyone could add some more info that'd be great (I'll also dig back into the code/start testing)

[sarken]

You're right: since approved is just for spam, setting hidden_by_admin to true does not automatically set approved = false.

We generally do not include reviewed = false comments in counts. Here's what we usually include when we count comments:

otwarchive/lib/acts_as_commentable/commentable_entity.rb

Lines 26 to 54 in 81cad74

	# The total number of visible comments on this commentable, not including
	# deleted comments, spam comments, unreviewed comments, and comments hidden
	# by an admin.
	#
	# This is the uncached version, and should only be used when calculating an
	# accurate value is important (e.g. when updating the StatCounter in the
	# database).
	def count_visible_comments_uncached
	self.total_comments.where(
	hidden_by_admin: false,
	is_deleted: false,
	unreviewed: false,
	approved: true
	).count
	end
	# The total number of visible comments on this commentable. Cached to reduce
	# computation. The cache is manually expired whenever a comment is added,
	# removed, or changes visibility, but the cache also expires after a fixed
	# amount of time in case of issues with the cache (e.g. stale data when
	# calculating the count).
	def count_visible_comments
	@count_visible_comments ||=
	Rails.cache.fetch(count_visible_comments_key,
	expires_in: ArchiveConfig.SECONDS_UNTIL_COMMENT_COUNTS_EXPIRE.seconds,
	race_condition_ttl: 10.seconds) do
	count_visible_comments_uncached
	end
	end

[anmazz]

Wanted to bring up a few things for discussion:

• I've basically done a comment thread count for the posted chapters in that particular time range, which is different than current implementation (based off total comment threads for the entire work). So I can revert that to the current implementation.
• As to point 1, it would make more sense if the other stats for the works were for that particular range of time (bookmarks in X year, kudos in X year, so on so forth). Hits wouldn't be able to display accurate counts for the year though so that's the issue, along with the potential hit to the db counting the kudos.
• I've had the work count for series reflects total posted in time frame just like word count for the work stats (inclusive only of posted works)

[anmazz]

@redsummernight I’ll definitely fix the internationalization stuff, I just want to make sure there’s discussion about the backend Ruby that I’ve done here as well. Figured that might be easier to fix all in one go if there are general issues/comments

[anmazz]

@redsummernight Sorry for my real poor phrasing. Not trying to call you out or request review from you specifically. If the “Awaiting Review” label COULD be re-added (looking for backend, overall design review) that would be great. If I should address the above comments FIRST, that’s totally fine as well.

really sorry for the bother 😭 I’m getting used to the activerecord so just want to make sure I’m doing things right.

[redsummernight]

No worries, I set the label back to "Awaiting Review".

[Bilka2]

An overall impression without looking deep yet:

Was the blurb draft tag change the only change outside of the original scope of the stats page? If no, please either list the other changes (so we can decide if we want to keep them in this PR) or revert them. For the blurb change, that has since then been changed in another PR, so please merge master to account for that.

Helpers are the wrong place for methods for a controller, they're meant for methods used in views. Methods used only by the controller can just be private methods on the controller itself.

StatItem seems like it may do better being a delegator instead, e.g. similar to CollectionDecorator.

I am confused by the changes in unrelated files like the works and chapter model, they suggest scope creep beyond series. Generally this whole thing seems to scope creep towards a total rework of the stats controller. A rework of the controller isn't a bad idea generally, it does have a lot of cruft. But I think it would be better to do that separately from functional changes and with an overall discussed plan of both what problems it's trying to solve and what design patterns we should use to address those problems, based on what's already in use in this project or other Rails projects. As is, I'm getting lost in the weeds of the implementation details while trying to separate what is a functional change and what is refactoring instead of being able to properly review the functional change or come up with a good design for refactoring. I suspect that this is why this PR lingered in awaiting review for so long: It's trying to do both at once while there's only a plan for one thing. I'd be happy to work with you to figure out that refactoring (e.g. splitting out the param parsing into methods) and organize that via separate PRs/Jira issues, once we have an overall plan.

[Bilka2]

Closing because this PR was adopted at #6010