-
-
Notifications
You must be signed in to change notification settings - Fork 100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(SharedHubStrategy): exclude Meta and Misc hubs #3131
fix(SharedHubStrategy): exclude Meta and Misc hubs #3131
Conversation
@@ -36,6 +36,8 @@ public function select(): ?Game | |||
->whereHas('games', function ($query) { | |||
$query->where('game_id', '!=', $this->sourceGame->id); // needs other games too! | |||
}) | |||
->where('Title', 'NOT LIKE', "%Meta%") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be NOT LIKE "[Meta%"
or you'll exclude hubs like "[Series - Metal Gear]" and "[Series - Twisted Metal]".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know that we can guarantee hubs will always be encased with square brackets. The problem you're describing definitely makes sense, but I think "%Meta - %" may be a more future-proofed target string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The %Misc. -%
filter will work, but %Meta -%
will miss anything [Meta|QA -
, [Meta|Art -
, or [Meta|DevComp -
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, I forgot about those.
I have reservations about adding more "NOT LIKE" filters on the query. I've combined everything into a single:
->whereRaw("game_sets.title NOT REGEXP '^(Meta[| -]|Misc\\.[ -])'")
which should be a bit more performant. Note that the square brackets in the pattern are not checking for the presence of square brackets in the title. They're defining character classes for conditionals in the pattern.
SQLite doesn't support REGEXP
, so I needed to add some conditional logic for tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless I made a typo, this is still matching the aforementioned series names.
select title from game_sets where title like '%meta%'
and title not regexp '^(Meta[| -]|Misc\\.[ -])';
I think it's the leading ^
. Doesn't that imply that the Meta
must start the string? And we know the strings all start with an open bracket.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No typo on your end, I messed up. I think I have things in a better place now in latest.
This PR causes the game suggestions pages to exclude Meta and Misc hubs from their shared hub strategy. This should hopefully make the hub selection ever-so-slightly more sensical (ie: no more recommendations because the game happens to be in the Noncompliant Writing hub).