Filter games version 3 #1199
Filter games version 3 #1199
Conversation
|
Here's a screenshot of "no results found." I didn't get a screenshot of the other one.
|
|
Revised it again.
|
…t's used in more than one place
…at the message can be used on more pages
…other page of results
…rriding the game filter
…'t know if they were.
www/newitems.php
Outdated
| $term = "#reviews:1-"; | ||
| $searchType = "game"; | ||
| $sortby = "lnew"; | ||
| $limit = null; |
There was a problem hiding this comment.
With $limit = null, this returns all 6000+ games that have reviews. The query plan seems to indicate that it's doing a full scan of the games table.
+------+--------------+------------------------+--------+---------------+---------+---------+---------------+-------+----------------------------------------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra |
+------+--------------+------------------------+--------+---------------+---------+---------+---------------+-------+----------------------------------------------+
| 1 | PRIMARY | games | ALL | NULL | NULL | NULL | NULL | 13984 | Using where; Using temporary; Using filesort |
| 1 | PRIMARY | gameRatingsSandbox0_mv | eq_ref | PRIMARY | PRIMARY | 130 | ifdb.games.id | 1 | |
| 2 | MATERIALIZED | gametags | ref | gameid,tag | tag | 1002 | const | 351 | Using where |
+------+--------------+------------------------+--------+---------------+---------+---------+---------------+-------+----------------------------------------------+
It seems to add 0.2 seconds to loading the home page on my laptop, which I guess is fine if hardly anybody uses this feature, but it creeps me out that if this feature gets even a little bit popular, it could take down the site.
I dunno, maybe it's fine. #1203 was fine for a long time, until it wasn't, and it took down the site for a few minutes. But the site came back up again on its own. And then we fixed it, and now it's fine again.
I think the "right way" is to do what I suggested here:
I guess we might have to start by adding a feature to sort games by "Most Recently Reviewed." To do that in a performant way, we'd need to use a materialized view. I… think it might be possible to add a
lastReviewDatecolumn togameRatingsSandbox0_mv…? But that would mean wading in to the nightmarishly complicatedgameRatingsSandbox0query inunscrub-ifarchive.sql.
I'd feel a lot better about merging this PR if we did that. But maybe I just need to get over myself and merge it?
(Sorry I've left this hanging for a month.)
Maybe @salty-horse has opinions?
There was a problem hiding this comment.
@dfabulich can you please tell me more about what I need to do? I added a statement to alter gameRatingsSandbox0_mv to add a new column, but I'm not sure how to get the published date of the latest review into that column....it looks like maybe gameRatingsSandbox0_mv is updated from the gameRatingsSandbox0 view? So do I need to get the published date into the gameRatingsSandbox0 view? (How?) I'm kind of lost. Also, the publishing date would be the greater of either createdate or embargo date, I think, but do we only add it (where?) if the embargo date is past?
|
Oh, also, it's conflicted, but perhaps the deeper problem is that I'm conflicted about this PR. 😖 |
…ter in call to doSearch
…meters in call to doSearch
Filter games merged
…o the home page, too
…iews If we're finding new reviews and we have a custom game filter, limit games to those reviewed in the last year only if the number of reviews we need is home-page-sized. On the allnew page, we want to be able to show more and older reviews.
The code was doing a `left join` (aka a `left outer join`) to `gameRatingsSandbox0_mv` unless `$browse` mode is enabled and you're using one of the specified sort orders, in which case it does a simple `join` (an `inner join`). I wrote this code in PR iftechfoundation#270, specifically in 3bd2c7a. But… why did I write that? Why didn't I just do an inner join in all cases? To be quite honest, I can't remember. I bet the issue was that `gameRatingsSandbox0_mv` didn't have exactly as many rows as the `games` table. (And I bet I was misusing `$browse` … it probably should have been `!$term`.) I believe that `gameRatingsSandbox0_mv` will always have a row for every game in the games table, now that we've merged iftechfoundation#1266, and so it should be safe to do an inner join in all cases.
…ames-for-new-reviews
$game_filter_was_applied (or the equivalent) is actually returned by doSearch anyway--we're just not tracking it. This change will bring back the "games have been filtered" announcement on the allnew page.
Add comment explaining "game_rows_after_filtering"
…iews Always search games for new reviews
Slightly more verbose message. Fixed the spacing before the message (by using
<p>) so that the spacing is about the same whether there is one result or multiple results. Changed some variable names to be clearer (taking out "search" since the game results aren't always the result of obvious searches). Also, the formatting doesn't look so strange anymore (I never figured out the cause). There's now a default value for the $override_game_filter parameter (though I don't know if it'll help much).