-
Notifications
You must be signed in to change notification settings - Fork 153
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
PNI creepy ratings on non-English pages is in the wrong order #7502
Comments
Looking at the stats for respective locales, it appears that voting is storing values "for localized products" independently of the original English product, So that's not great. Solving this one will require:
|
Current code in @property
def total_vote_count(self):
return sum(self.get_or_create_votes())
@property
def creepiness(self):
try:
average = self.creepiness_value / self.total_vote_count
except ZeroDivisionError:
average = 50
return average This needs those Similarly, the POST code currently reads: if request.body and request.method == "POST":
data = json.loads(request.body)
if data.get("value"):
...
try:
product = ProductPage.objects.get(pk=self.id)
... where this product, too, should be resolved to the original English product, not the localized product. |
Looking at the stage and prod db, I think we can get away with code-fixes only, there's a whole bunch of "zero vote" records in the database, but as far as I can they're not actually linked to any products (anymore?). So the PR for this will be code + tests without a cleanup management command. However, I will be filing a cleanup issue for "when we have time for cleanup" to see if we can safely remove those records from the database. |
Also, given that we're going to have to add a secondary locale as part of the test framework before can even write tests this works correctly in an automated fashion, I'm going to file that as separate issue, too, and we can land this first with manual testing, then figure out how to do secondary locales (in both unit tests and load_fake_data) separaterly. |
How to Reproduce:
This was flagged on Twitter https://twitter.com/dtdan03/status/1441837328323031044 and was confirmed by staff
The text was updated successfully, but these errors were encountered: